Revanth14 commented on code in PR #2163:
URL: https://github.com/apache/iceberg-go/pull/2163#discussion_r4238808667
##########
catalog/rest/vended_creds.go:
##########
@@ -474,6 +486,26 @@ func clearOverriddenCredentialProperties(props,
credentialConfig iceberg.Propert
delete(props, keyAdlsSasExpiresAtMs+"."+suffix)
}
}
+
+ if credentialConfig[iceio.ADLSToken] != "" {
+ // adls.token is evaluated after shared-key, SAS-token, and
connection-string
+ // auth, so any of those left over from the table or catalog
configuration
+ // would shadow a freshly vended token and the read would 403.
props is a
+ // per-location copy, so clearing them cannot affect another
location's IO.
+ //
+ // The value must be nonempty: the Azure bucket factory skips
an empty
+ // adls.token, so clearing on mere key presence would discard
working
+ // credentials and fall through to a weaker auth method.
+ delete(props, iceio.ADLSSharedKeyAccountName)
+ delete(props, iceio.ADLSSharedKeyAccountKey)
+ for key := range props {
+ if strings.HasPrefix(key, iceio.ADLSSasTokenPrefix) ||
+ strings.HasPrefix(key,
keyAdlsSasExpiresAtMs+".") ||
Review Comment:
Fixed, The token path now drops every key `parseCredentialExpiry` reads,
bare form included, through a shared `isCredentialExpiryKey` predicate so the
two can't drift. Covered by `TestVendedCredsRefreshedADLSTokenExpiry`.
--
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]
