build/alts: fixes needed for .bzl and to bump gRPC to 1.20.#6773
build/alts: fixes needed for .bzl and to bump gRPC to 1.20.#6773htuch merged 7 commits intoenvoyproxy:masterfrom
Conversation
This continues envoyproxy#6620. See also envoyproxy#6308 and envoyproxy#5461. Risk level: Low Testing: Existing tests Signed-off-by: Harvey Tuch <[email protected]>
|
@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 <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
|
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 <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
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 <[email protected]> Signed-off-by: Jeff Piazza <[email protected]>
5 participants
This continues #6620.
See also #6308 and
#5461.
Risk level: Low
Testing: Existing tests
Signed-off-by: Harvey Tuch [email protected]