Skip to content

Revert STS from 1.4 with cherry picks of integration and stress test fixes. #180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
istio-testing merged 11 commits into from
Mar 4, 2020
Merged

Revert STS from 1.4 with cherry picks of integration and stress test fixes. #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8d3fb7c
Revert "fix opencensus tracer (#155)"
duderino Mar 3, 2020
7cf7f62
Revert "Add x-goog-user-proj header for sts credential (#152)"
duderino Mar 3, 2020
c665445
Revert "Update GrpcService to add StsService. (#411)"
duderino Mar 3, 2020
7a48ec8
Revert "fix tracer ssl credential (#151)"
duderino Mar 3, 2020
f0c8b8c
Revert "remove url validation as it is not implemented"
duderino Mar 3, 2020
4e1e29f
Revert "Use gRPC Security Token Service (STS) to get call credentials…
duderino Mar 3, 2020
6bdbf7a
Revert "[release-1.4] Use sts for call credential when STS_PORT is pr…
duderino Mar 3, 2020
bd68bad
Revert "Upgrade gRPC to 1.25 which has gRPC STS feature (#145)"
duderino Mar 3, 2020
037cab7
ci: mark //test/integration:protocol_integration_test as flaky. (#162)
PiotrSikora Feb 24, 2020
6a4f4fe
Remove wasm filter stress test
gargnupur Feb 20, 2020
63302fd
Remove wasm stress test framework
gargnupur Feb 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 1 addition & 46 deletions api/envoy/api/v2/core/grpc_service.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ message GrpcService {
}
}

// [#next-free-field: 8]
// [#next-free-field: 7]
message CallCredentials {
message ServiceAccountJWTAccessCredentials {
string json_key = 1;
Expand All @@ -86,46 +86,6 @@ message GrpcService {
}
}

// Security token service configuration that allows Google gRPC to
// fetch security token from an OAuth 2.0 authorization server.
// See https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-16 and
// https://github.com/grpc/grpc/pull/19587.
// [#next-free-field: 10]
message StsService {
// URI of the token exchange service that handles token exchange requests.
string token_exchange_service_uri = 1;

// Location of the target service or resource where the client
// intends to use the requested security token.
string resource = 2;

// Logical name of the target service where the client intends to
// use the requested security token.
string audience = 3;

// The desired scope of the requested security token in the
// context of the service or resource where the token will be used.
string scope = 4;

// Type of the requested security token.
string requested_token_type = 5;

// The path of subject token, a security token that represents the
// identity of the party on behalf of whom the request is being made.
string subject_token_path = 6 [(validate.rules).string = {min_bytes: 1}];

// Type of the subject token.
string subject_token_type = 7 [(validate.rules).string = {min_bytes: 1}];

// The path of actor token, a security token that represents the identity
// of the acting party. The acting party is authorized to use the
// requested security token and act on behalf of the subject.
string actor_token_path = 8;

// Type of the actor token.
string actor_token_type = 9;
}

oneof credential_specifier {
option (validate.required) = true;

Expand Down Expand Up @@ -153,11 +113,6 @@ message GrpcService {
// https://grpc.io/grpc/cpp/namespacegrpc.html#a823c6a4b19ffc71fb33e90154ee2ad07.
// https://grpc.io/docs/guides/auth.html#extending-grpc-to-support-other-authentication-mechanisms.
MetadataCredentialsFromPlugin from_plugin = 6;

// Custom security token service which implements OAuth 2.0 token exchange.
// https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-16
// See https://github.com/grpc/grpc/pull/19587.
StsService sts_service = 7;
}
}

Expand Down
50 changes: 1 addition & 49 deletions api/envoy/api/v3alpha/core/grpc_service.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ message GrpcService {
}
}

// [#next-free-field: 8]
// [#next-free-field: 7]
message CallCredentials {
message ServiceAccountJWTAccessCredentials {
string json_key = 1;
Expand All @@ -88,49 +88,6 @@ message GrpcService {
}
}

// Security token service configuration that allows Google gRPC to
// fetch security token from an OAuth 2.0 authorization server.
// See https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-16 and
// https://github.com/grpc/grpc/pull/19587.
// [#next-free-field: 10]
message StsService {
option (udpa.api.annotations.versioning).previous_message_type =
"envoy.api.v2.core.GrpcService.GoogleGrpc.CallCredentials.StsService";

// URI of the token exchange service that handles token exchange requests.
string token_exchange_service_uri = 1;

// Location of the target service or resource where the client
// intends to use the requested security token.
string resource = 2;

// Logical name of the target service where the client intends to
// use the requested security token.
string audience = 3;

// The desired scope of the requested security token in the
// context of the service or resource where the token will be used.
string scope = 4;

// Type of the requested security token.
string requested_token_type = 5;

// The path of subject token, a security token that represents the
// identity of the party on behalf of whom the request is being made.
string subject_token_path = 6 [(validate.rules).string = {min_bytes: 1}];

// Type of the subject token.
string subject_token_type = 7 [(validate.rules).string = {min_bytes: 1}];

// The path of actor token, a security token that represents the identity
// of the acting party. The acting party is authorized to use the
// requested security token and act on behalf of the subject.
string actor_token_path = 8;

// Type of the actor token.
string actor_token_type = 9;
}

oneof credential_specifier {
option (validate.required) = true;

Expand Down Expand Up @@ -158,11 +115,6 @@ message GrpcService {
// https://grpc.io/grpc/cpp/namespacegrpc.html#a823c6a4b19ffc71fb33e90154ee2ad07.
// https://grpc.io/docs/guides/auth.html#extending-grpc-to-support-other-authentication-mechanisms.
MetadataCredentialsFromPlugin from_plugin = 6;

// Custom security token service which implements OAuth 2.0 token exchange.
// https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-16
// See https://github.com/grpc/grpc/pull/19587.
StsService sts_service = 7;
}
}

Expand Down
33 changes: 14 additions & 19 deletions api/wasm/cpp/proxy_wasm_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,7 @@ class RootContext : public ContextBase {
const std::string root_id_;
std::unordered_map<uint32_t, HttpCallCallback> http_calls_;
std::unordered_map<uint32_t, GrpcSimpleCallCallback> simple_grpc_calls_;
std::unique_ptr<GrpcCallHandlerBase> cur_grpc_call_;
std::unordered_map<uint32_t, std::unique_ptr<GrpcCallHandlerBase>> grpc_calls_;
std::unique_ptr<GrpcStreamHandlerBase> cur_grpc_stream_;
Copy link
Copy Markdown

@PiotrSikora PiotrSikora Mar 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't be reverting those changes, see: #175 (comment)

Copy link
Copy Markdown
Author

@duderino duderino Mar 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we didn't ship that in 1.4.6. I think instead we should get release-1.4 into a state that looks like the 1.4.6 release we just shipped, tag it, then reapply any fixes which can go out in 1.4.7

@howardjohn CC

Copy link
Copy Markdown

@PiotrSikora PiotrSikora Mar 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a valid point. OK, fair enough, no one should be using Wasm in 1.4 anyway...

cc @mandarjog @kyessenov @bianpengyuan for visibility.

Copy link
Copy Markdown

@kyessenov kyessenov Mar 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree that we need to get release-1.4 branch into a state that looks like 1.4.6. We did not ship from 1.4 branch, as in reality we created a private 1.4.6 branch that was developed independently from 1.4 branch. Now that 1.4.6 branch is done, we should simply merge 1.4.6 branch into 1.4 branch (and that was exceptional due to secrecy around the patches).

std::unordered_map<uint32_t, std::unique_ptr<GrpcStreamHandlerBase>> grpc_streams_;
};

Expand Down Expand Up @@ -1236,16 +1234,19 @@ inline void GrpcStreamHandlerBase::send(StringView message, bool end_of_stream)
}
}

inline void RootContext::onGrpcCreateInitialMetadata(uint32_t) {
inline void RootContext::onGrpcCreateInitialMetadata(uint32_t token) {
{
if (cur_grpc_call_ != nullptr) {
cur_grpc_call_->onCreateInitialMetadata();
auto it = grpc_calls_.find(token);
if (it != grpc_calls_.end()) {
it->second->onCreateInitialMetadata();
return;
}
}
{
if (cur_grpc_stream_ != nullptr) {
cur_grpc_stream_->onCreateInitialMetadata();
auto it = grpc_streams_.find(token);
if (it != grpc_streams_.end()) {
it->second->onCreateInitialMetadata();
return;
}
}
}
Expand Down Expand Up @@ -1344,32 +1345,26 @@ inline bool RootContext::grpcCallHandler(StringView service, StringView service_
const google::protobuf::MessageLite& request,
uint32_t timeout_milliseconds,
std::unique_ptr<GrpcCallHandlerBase> handler) {
cur_grpc_call_ = std::move(handler);
auto token = grpcCall(service, service_name, method_name, request, timeout_milliseconds);
if (token) {
cur_grpc_call_->token_ = token;
cur_grpc_call_->context_ = this;
grpc_calls_[token] = std::move(cur_grpc_call_);
cur_grpc_call_ = nullptr;
handler->token_ = token;
handler->context_ = this;
grpc_calls_[token] = std::move(handler);
return true;
}
cur_grpc_call_ = nullptr;
return false;
}

inline bool RootContext::grpcStreamHandler(StringView service, StringView service_name,
StringView method_name,
std::unique_ptr<GrpcStreamHandlerBase> handler) {
cur_grpc_stream_ = std::move(handler);
auto token = grpcStream(service, service_name, method_name);
if (token) {
cur_grpc_stream_->token_ = token;
cur_grpc_stream_->context_ = this;
grpc_streams_[token] = std::move(cur_grpc_stream_);
cur_grpc_stream_ = nullptr;
handler->token_ = token;
handler->context_ = this;
grpc_streams_[token] = std::move(handler);
return true;
}
cur_grpc_stream_ = nullptr;
return false;
}

Expand Down
Loading