Skip to content

s2a fix: don't check transport channel provider endpoint. #3784

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rmehta19
Copy link
Contributor

@rmehta19 rmehta19 commented May 6, 2025

Certain handwritten client libraries set this field to the (TLS) endpoint via GrpcTransportOptions.setUpChannelProvider.

This isn't really a custom endpoint, so we should be able to use S2A when this is set.

Example with Cloud Logging: https://github.com/googleapis/java-logging/blob/644ebd756d44ffc02b58aa30d9a07c8afe700981/google-cloud-logging/src/main/java/com/google/cloud/logging/spi/v2/GrpcLoggingRpc.java#L137

@product-auto-label product-auto-label bot added the size: s Pull request size is small. label May 6, 2025
@rmehta19
Copy link
Contributor Author

rmehta19 commented May 7, 2025

@lqiu96 , would you be able to review this? Thanks!

@lqiu96
Copy link
Contributor

lqiu96 commented May 8, 2025

Certain handwritten client libraries set this field to the (TLS) endpoint via GrpcTransportOptions.setUpChannelProvider.

I see, I wasn't aware of this use case. However, if a user does create their own TransportChannelProvider with a custom endpoint, then it wouldn't skip S2A.

Can you remind me again what the original rationale was for skipping a custom endpoint?

@rmehta19
Copy link
Contributor Author

Can you remind me again what the original rationale was for skipping a custom endpoint?

When a user is setting a custom endpoint, we can't be sure this is a MTLS endpoint, therefore we can't use S2A. We only want to use S2A when we can use the MTLS endpoint.

However, if a user does create their own TransportChannelProvider with a custom endpoint, then it wouldn't skip S2A.

This is true, if a hw client library does this (say they set it to something foo.xyz.com) then S2A wouldn't get skipped, and S2A would be used to connect to MTLS endpoint of api (foo.mtls.googleapis.com) which isn't our desired behavior (we want to respect custom set endpoint and skip S2A in this case). Perhaps we could add some extra field (GAXProvidedEndpoint) to the TransportChannelProvider interface and GrpcTransportOptions.setUpChannelProvider can set that field instead? Then we could keep the S2A logic in EndpointContext as is. WDYT @lqiu96?

@rmehta19
Copy link
Contributor Author

Friendly ping on this @lqiu96. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants