GCP: add pluggable GCS token credential provider - #17468
Conversation
| } else if (properties.tokenCredentialProvider().isPresent()) { | ||
| // A custom provider yields a self-refreshing GoogleCredentials (e.g. built from a | ||
| // caller-supplied source credential), addressing static-token expiry for non-vended setups. | ||
| return GcsTokenCredentialProviders.from(properties.properties()).credential(); |
There was a problem hiding this comment.
Please note a silent auth-precedence trap: in credentials(), branches 1–3 check oauth2Token, noAuth, and impersonateServiceAccount before the new tokenCredentialProvider branch (4), so if a user sets the provider alongside gcs.oauth2.token or gcs.impersonate.service-account, the earlier branch silently wins and the provider is never invoked. Only the gcs.no-auth + provider combination is guarded with Preconditions.checkState. The oauth2 coexistence is documented in testTokenCredentialProviderWithOAuth2Token as intentional ("PrefixedStorage resolves precedence"), but the impersonation case has no guard and no documentation. The impersonation trap is sharpest because impersonation may be inherited from shared catalog config. Recommend either (a) adding a Preconditions.checkState guard mirroring the no-auth one for the impersonation case, or (b) adding explicit precedence Javadoc on GCS_TOKEN_CREDENTIAL_PROVIDER ("ignored if gcs.oauth2.token or gcs.impersonate.service-account is also set") plus a LOG.warn when the provider is set but shadowed. Neither option is currently present.
There was a problem hiding this comment.
Going ahead with the second suggestion - precedence Javadoc.
-
Users may have base configs with gcs.token-credential-provider and environment-specific overrides adding gcs.impersonate.service-account. When merged, both properties coexist. Preconditions.checkState fails at Properties construction time, breaking this legitimate config layering pattern. The override should just "win" at credential selection time, not fail the entire config load.
-
Selection vs Validation: This is about which credential to use (selection logic in credentials() method at runtime), not whether the config is valid (validation logic in Properties constructor at startup). Multiple auth properties being present isn't a configuration error - it's a selection decision. Preconditions.checkState in the constructor treats it as validation failure when it should just be a runtime precedence rule. Vended path with both oauth2 and credentials provider already sets this precedent.
| return NoCredentials.getInstance(); | ||
| } else if (properties.impersonateServiceAccount().isPresent()) { | ||
| return buildImpersonatedCredentials(properties); | ||
| } else if (properties.tokenCredentialProvider().isPresent()) { |
There was a problem hiding this comment.
Also, missing PrefixedStorage wiring test: TestGcsTokenCredentialProviders tests the factory in isolation and TestGCPProperties tests property parsing, but no test constructs a PrefixedStorage with gcs.token-credential-provider set and verifies the resulting Storage client receives the provider's GoogleCredentials. TestPrefixedStorage.validParameters already demonstrates the pattern with gcs.oauth2.token (mock credential -> assert setCredentials called). Also missing: a test for the impersonation + provider coexistence behavior (whichever resolution is chosen above). A wiring test is needed to gate confidence in the new branch.
…nd robustness check Adds precedence documentation, robustness checks (null-gaurd, LOG.warn for shadowed provider), and new tests verifying property parsing and credential selection precedence.
2 participants
What
Adds a pluggable storage credential-provider SPI to the GCS FileIO in the
iceberg-gcpmodule:GcsTokenCredentialProvider- interface returning acom.google.auth.oauth2.GoogleCredentials,with an
initialize(Map<String,String>)hook for provider-specific config.GcsTokenCredentialProviders- factory that loads a custom implementation viaDynConstructorsfrom the new
gcs.token-credential-providerproperty, with aDefaultGcsTokenCredentialProviderbacked by Application Default Credentials.
GCPPropertiesconstants (gcs.token-credential-provider,gcs.token-credential-provider.prefix), an accessor, and a precondition preventing it from beingcombined with
gcs.no-auth.PrefixedStorage#credentials(...)that uses the configured provider when present.This is the GCS analogue of the existing Azure
AdlsTokenCredentialProvider(#14136) and AWS'sclient.credentials-provider.Why
The GCS FileIO today supports a static
gcs.oauth2.token,gcs.no-auth, native impersonation, andthe vended refresh endpoint - but there is no pluggable way to supply a caller-provided,
self-refreshing source credential for non-vended setups. None of the existing paths covers this:
gcs.oauth2.tokenis static - it never refreshes, so long-running jobs fail at token expiry.caller-supplied source.
gcs.impersonate.service-account) structurally starts fromGoogleCredentials.getApplicationDefault()as its source credential - there is no property toinject an arbitrary caller-supplied source, so it does not cover the bring-your-own-credentials
case.
This SPI lets integrators plug in a credential source that refreshes, without Iceberg taking on any
specific credential implementation. The default remains Application Default Credentials.
Compatibility
gcs.token-credential-providerset,credential resolution is identical to today (oauth2Token / no-auth / impersonation / ADC).
iceberg-gcp.Scope: this is storage-plane auth
GCP has two independent auth planes, and this PR touches only the first:
GCSFileIO/PrefixedStorageauthenticate to GCS to read/write datafiles. This is where the new
gcs.token-credential-providerSPI lives (properties aregcs.*).GoogleAuthManager(properties are
gcp.auth.*). This PR does not change it.Testing
TestGcsTokenCredentialProviders- default factory, empty/blank provider, custom provider,missing no-arg ctor, non-implementing class, prefixed-property extraction.
TestGCPProperties- provider property is read; mutual-exclusion withgcs.no-authis enforced;provider +
gcs.oauth2.tokenis allowed../gradlew :iceberg-gcp:spotlessCheck :iceberg-gcp:testpasses locally.