Skip to content

fix(gax): use TLS channel credentials for DirectPath over Interconnect - #14864

Open
kalragauri wants to merge 1 commit into
googleapis:mainfrom
kalragauri:fix/gax-interconnect-tls
Open

kalragauri wants to merge 1 commit into
googleapis:mainfrom
kalragauri:fix/gax-interconnect-tls

Conversation

@kalragauri

Copy link
Copy Markdown

InstantiatingGrpcChannelProvider wrapped DirectPath over Interconnect
channels in GoogleDefaultChannelCredentials, whose negotiator selects
ALTS for every xDS cluster not prefixed "google_cfe_". The regional
Interconnect clusters that RLS returns for data operations are not CFE
clusters, and ALTS is unavailable outside Google Cloud, so those
connections fail once the service routes to them.

The provider now builds CompositeChannelCredentials from
TlsChannelCredentials and the call credentials when Interconnect is
active, so every subchannel negotiates TLS. The call credentials stay on
the channel credentials because those are the only credentials the RLS
control channel inherits from the parent channel. ALTS hard-bound tokens
are not requested on this path; they need an ALTS connection. Standard
DirectPath is unchanged.

DirectPath over Interconnect also requires credentials now, as standard
DirectPath does. Previously a channel built with NoCredentialsProvider
still used DirectPath, and GoogleDefaultChannelCredentials loaded
Application Default Credentials on its own, or failed every RPC with
UNAUTHENTICATED when none were available. Such a channel now falls
back to CloudPath.

The misconfiguration log covers Interconnect as well. It names the
missing credentials instead of asking for ComputeEngineCredentials, it
reports Interconnect being enabled without the attemptDirectPath
option (or with GOOGLE_CLOUD_DISABLE_DIRECT_PATH set), as it already
did for the xDS option, and it logs Interconnect misconfigurations at
WARNING off GCE, where Interconnect is meant to run. Standard
DirectPath keeps logging at FINE off GCE.

The credential selection moves into
createDirectPathChannelCredentials so it can be tested without
dialing. Tests cover the builder option, the environment variable
(including case and whitespace), invalid and "false" values, null
credentials with and without an mTLS client certificate provider, ALTS
hard-bound tokens on both paths, the credentials the channel builder
receives, and the new log messages and levels.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for DirectPath xDS over Interconnect off Google Compute Engine (GCE) by negotiating TLS instead of ALTS and requiring credentials. It updates credential compatibility checks, refactors channel credential creation, improves misconfiguration logging, and adds comprehensive unit tests. The review feedback recommends returning credentials != null in isCredentialDirectPathCompatible() to prevent a potential NullPointerException when credentials are null, and ensuring that CompositeChannelCredentials and TlsChannelCredentials are properly imported to avoid compilation errors.

@kalragauri
kalragauri force-pushed the fix/gax-interconnect-tls branch 2 times, most recently from 00821b0 to 1576303 Compare October 30, 2026 07:50
@kalragauri
kalragauri force-pushed the fix/gax-interconnect-tls branch from 1576303 to 41525ff Compare October 30, 2026 07:58
@kalragauri kalragauri added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 30, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 30, 2026
@kalragauri
kalragauri marked this pull request as ready for review October 30, 2026 09:24
@kalragauri
kalragauri requested review from a team as code owners October 30, 2026 09:24

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants