Repository navigation
fix(gax): use TLS channel credentials for DirectPath over Interconnect - #14864
kalragauri wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
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.
1576303 to
41525ff
Compare
41525ff to
5305add
Compare
| // Off GCE, DirectPath is normally unavailable, so misconfigurations are only logged at FINE. | ||
| // DirectPath over Interconnect is meant to run off GCE, so when the client asked for it, its | ||
| // misconfigurations are warnings. | ||
| Level level = isOnComputeEngine() || interconnectViaBuilderOption ? Level.WARNING : Level.FINE; |
There was a problem hiding this comment.
I think this only sets log level to FINE for non-GCE scenarios? Other client libraries would still get WARNING on GCE?
| Level level = isOnComputeEngine() || interconnectViaBuilderOption ? Level.WARNING : Level.FINE; | ||
|
|
||
| if (!isDirectPathEnabled()) { | ||
| // Checked before the xDS cases: clients such as Storage set the xDS option together with |
There was a problem hiding this comment.
If Storage always sets it, do we even need this log? I feel we are adding complex error-prone logics for unlikely scenarios.
InstantiatingGrpcChannelProviderwrapped DirectPath over Interconnectchannels in
GoogleDefaultChannelCredentials, whose negotiator selectsALTS for every xDS cluster not prefixed
"google_cfe_". The regionalInterconnect 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
CompositeChannelCredentialsfromTlsChannelCredentialsand the call credentials when Interconnect isactive, 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
NoCredentialsProviderstill used DirectPath, and
GoogleDefaultChannelCredentialsloadedApplication Default Credentials on its own, or failed every RPC with
UNAUTHENTICATEDwhen none were available. Such a channel now fallsback to CloudPath.
The misconfiguration log covers Interconnect as well. It names the
missing credentials instead of asking for
ComputeEngineCredentials, itreports Interconnect being enabled without the
attemptDirectPathoption (or with
GOOGLE_CLOUD_DISABLE_DIRECT_PATHset), as it alreadydid for the xDS option, and it logs Interconnect misconfigurations at
WARNINGoff GCE, where Interconnect is meant to run, when the clientasked for Interconnect through the builder option. The
GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS_OVER_INTERCONNECTenvironmentvariable is process-wide, so a client that did not enable DirectPath
treats it like
GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS: a note that it maybe a misconfiguration, at
WARNINGon GCE andFINEelsewhere. StandardDirectPath keeps logging at
FINEoff GCE.The credential selection moves into
createDirectPathChannelCredentialsso it can be tested withoutdialing. Tests cover the builder option, the environment variable
(including case and whitespace), invalid and
"false"values, nullcredentials 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. In java-storage,
StorageOptionsBuilderTestnow attaches the client's credentials beforeexpecting DirectPath over Interconnect, and checks that
NoCredentialsfalls back to CloudPath.