build/alts: fixes needed for .bzl and to bump gRPC to 1.20.#6773
Conversation
This continues envoyproxy#6620. See also envoyproxy#6308 and envoyproxy#5461. Risk level: Low Testing: Existing tests Signed-off-by: Harvey Tuch <htuch@google.com>
|
@JimmyCYJ I think this is the solution, but I'm having issues getting the integration test to pass. I suspect the gRPC handshake fake that we spin up there might also be interacting? Almost all the tests pass except the last one. I'll look into this more later, but wanted to get the ball rolling on this as it's blocking a significant number of devs. |
|
/lgtm Thanks for the fix! |
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
|
I had to add explicit |
| ":terminate_handler_lib", | ||
| ], | ||
| }) + envoy_cc_platform_dep("platform_impl_lib"), | ||
| }) + envoy_cc_platform_dep("platform_impl_lib") + envoy_google_grpc_external_deps(), |
There was a problem hiding this comment.
@jplevyak FYI, I'm adding this macro in this PR that you might also find useful.
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
cmluciano
left a comment
cmluciano
left a comment
There was a problem hiding this comment.
One potential follow-up comment but lgtm, thanks!
| : options_(options), component_factory_(component_factory), thread_factory_(thread_factory), | ||
| file_system_(file_system), stats_allocator_(symbol_table_) { | ||
| #ifdef ENVOY_GOOGLE_GRPC | ||
| grpc_init(); |
There was a problem hiding this comment.
Ahhh this is where I got stuck. For some reason I though one of the c_smt_ptrs was handling this.
| options.get(), target_name, handshaker_service.c_str(), is_upstream, &handshaker); | ||
| tsi_result status = | ||
| alts_tsi_handshaker_create(options.get(), target_name, handshaker_service.c_str(), | ||
| is_upstream, nullptr /* interested_parties */, &handshaker); |
There was a problem hiding this comment.
I had a TODO here to follow up on if Envoy would benefit from allowing this pollset to be configurable. Do you think this would be useful?
…xy#6773) This continues envoyproxy#6620. See also envoyproxy#6308 and envoyproxy#5461. Risk level: Low Testing: Existing tests Signed-off-by: Harvey Tuch <htuch@google.com> Signed-off-by: Jeff Piazza <jeffpiazza@google.com>
5 participants
This continues #6620.
See also #6308 and
#5461.
Risk level: Low
Testing: Existing tests
Signed-off-by: Harvey Tuch htuch@google.com