Skip to content

fix(storage): use TLS for default credentials with DirectPath over Interconnect - #16599

Open
kalragauri wants to merge 4 commits into
googleapis:mainfrom
kalragauri:feat/grpc-tls-creds
Open

kalragauri wants to merge 4 commits into
googleapis:mainfrom
kalragauri:feat/grpc-tls-creds

Conversation

@kalragauri

@kalragauri kalragauri commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

With DirectPathXdsOverInterconnectOption (or GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS_OVER_INTERCONNECT=true) and gRPC >= 1.85, DefaultOptionsGrpc() targets google-c2p:///storage-direct.googleapis.com?force-xds (default_options.cc#L178-L181), but the default credentials still map to grpc::GoogleDefaultCredentials() (unified_grpc_credentials.cc#L109-L112). Those credentials select ALTS for any xDS cluster not named google_cfe_* (google_default_credentials.cc#L136-L145), which is what Traffic Director returns for DirectPath backends. Interconnect clients run outside Google Cloud, where ALTS is unavailable, so the channel never connects.

This PR adds an internal GrpcGoogleDefaultTlsOnlyOption. When set, the GoogleDefaultCredentialsConfig branch of CreateAuthenticationStrategy() returns grpc::CompositeChannelCredentials(grpc::SslCredentials(), call_creds), with call_creds derived from Application Default Credentials the same way gRPC's own Google Default credentials derive them (google_default_credentials.cc#L263). Tokens stay channel-level, so RLS control channels inherit them; only the transport changes from ALTS to TLS. The ADC lookup mirrors the REST loader (oauth2_google_credentials.cc#L161-L180) rather than reusing it, because grpc_utils does not link rest_internal (libcurl). Differences from grpc::GoogleDefaultCredentials()'s own lookup, all matching the REST
credentials and pinned by tests: a GOOGLE_APPLICATION_CREDENTIALS file that cannot be opened or
parsed is an error rather than a fall-through to the gcloud file and the metadata server;
GOOGLE_GCLOUD_ADC_PATH_OVERRIDE is honored; with no file at all, Compute Engine call credentials
are used without probing the metadata server; and CARootsFilePathOption applies to the TLS roots.

DefaultOptionsGrpc() sets the option whenever gRPC supports force-xds and the effective endpoint is an Interconnect target: keyed on the endpoint like the AuthorityOption default from #16584, but for any force-xds host, since ALTS is unavailable regardless of host.

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Oct 28, 2026

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

Copy link
Copy Markdown

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 the GrpcGoogleDefaultTlsOnlyOption option and implements a custom Application Default Credentials (ADC) lookup and classification mechanism in the gRPC unified credentials implementation. This allows the SDK to compose TLS channel credentials with call credentials derived from ADC when running in environments where ALTS is unavailable, such as DirectPath over Interconnect. The changes also include comprehensive unit tests and updates to the Cloud Storage gRPC plugin options and documentation to enable this option for Interconnect endpoints. I have no feedback to provide as there are no review comments.

@kalragauri
kalragauri marked this pull request as ready for review October 28, 2026 06:38
@kalragauri
kalragauri requested review from a team as code owners October 28, 2026 06:38
@kalragauri
kalragauri requested a review from v-pratap October 28, 2026 06:38

@v-pratap v-pratap 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.

Looks good overall, and the option wiring and error paths are well covered. Main ask: add one test that can tell the TLS strategy apart from grpc::GoogleDefaultCredentials(); right now swapping them keeps the suite green. A few nits inline.

Comment thread google/cloud/internal/unified_grpc_credentials_test.cc
Comment thread google/cloud/internal/unified_grpc_credentials.cc Outdated
Comment thread google/cloud/internal/unified_grpc_credentials.cc
Comment thread google/cloud/internal/unified_grpc_credentials.h Outdated
@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.81785% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.42%. Comparing base (608aeef) to head (3a945e8).

Files with missing lines Patch % Lines
...le/cloud/internal/unified_grpc_credentials_test.cc 99.74% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16599      +/-   ##
==========================================
+ Coverage   92.40%   92.42%   +0.02%     
==========================================
  Files        2266     2266              
  Lines      220810   221345     +535     
==========================================
+ Hits       204036   204588     +552     
+ Misses      16774    16757      -17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread google/cloud/internal/unified_grpc_credentials.h Outdated

This branch was successfully deployed

1 active deployment
false — 3a945e8e Deployed Oct 29, 2026 by kalragauri via Save PR ref #12702
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants