uros-b commented on code in PR #17468:
URL: https://github.com/apache/iceberg/pull/17468#discussion_r3699773067
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/PrefixedStorage.java:
##########
@@ -158,6 +158,10 @@ private Credentials credentials(GCPProperties properties) {
return NoCredentials.getInstance();
} else if (properties.impersonateServiceAccount().isPresent()) {
return buildImpersonatedCredentials(properties);
+ } 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();
Review 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 al
so set") plus a LOG.warn when the provider is set but shadowed. Neither option
is currently present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
