Revanth14 commented on code in PR #2163:
URL: https://github.com/apache/iceberg-go/pull/2163#discussion_r4238796564
##########
catalog/rest/vended_creds_test.go:
##########
@@ -692,6 +692,96 @@ func TestPrefixScopedIOReplacesS3CredentialAtomically(t
*testing.T) {
assert.NotContains(t, props, keyS3TokenExpiresAtMs)
}
+func TestPrefixScopedIOClearsStaleADLSAuthForVendedToken(t *testing.T) {
+ t.Parallel()
+
+ const host = "acct.dfs.core.windows.net"
+
+ p := newPrefixScopedIO(context.Background(), iceberg.Properties{
+ iceio.ADLSEndpoint:
"dfs.core.windows.net",
+ iceio.ADLSSharedKeyAccountName: "acct",
+ iceio.ADLSSharedKeyAccountKey: "stale-key",
+ iceio.ADLSSasTokenPrefix + host: "stale-sas",
+ keyAdlsSasExpiresAtMs + "." + host: "1000",
+ iceio.ADLSConnectionStringPrefix + "acct":
"stale-connection-string",
+ }, []StorageCredential{{
+ Prefix: "abfss://container@" + host + "/",
+ Config: iceberg.Properties{iceio.ADLSToken: "vended-token"},
+ }})
+
+ props := p.propertiesForLocation("abfss://container@" + host +
"/table/metadata.json")
+
+ // The vended token and unrelated tuning properties survive.
+ assert.Equal(t, "vended-token", props[iceio.ADLSToken])
+ assert.Equal(t, "dfs.core.windows.net", props[iceio.ADLSEndpoint])
+
+ // Everything that outranks adls.token in the Azure auth chain is gone,
so the
+ // vended token is what actually authenticates the read.
+ assert.NotContains(t, props, iceio.ADLSSharedKeyAccountName)
+ assert.NotContains(t, props, iceio.ADLSSharedKeyAccountKey)
+ assert.NotContains(t, props, iceio.ADLSSasTokenPrefix+host)
+ assert.NotContains(t, props, keyAdlsSasExpiresAtMs+"."+host)
+ assert.NotContains(t, props, iceio.ADLSConnectionStringPrefix+"acct")
+}
+
+func TestPrefixScopedIOKeepsCredentialsWhenVendedADLSTokenIsEmpty(t
*testing.T) {
+ t.Parallel()
+
+ const host = "acct.dfs.core.windows.net"
+
+ p := newPrefixScopedIO(context.Background(), iceberg.Properties{
+ iceio.ADLSSharedKeyAccountName: "acct",
+ iceio.ADLSSharedKeyAccountKey: "working-key",
+ iceio.ADLSSasTokenPrefix + host: "working-sas",
+ }, []StorageCredential{{
+ Prefix: "abfss://container@" + host + "/",
+ Config: iceberg.Properties{iceio.ADLSToken: ""},
+ }})
+
+ props := p.propertiesForLocation("abfss://container@" + host +
"/table/metadata.json")
+
+ // The Azure bucket factory skips an empty adls.token, so clearing on
key
+ // presence alone would discard these working credentials and fall
through to
+ // a weaker auth method.
+ assert.Equal(t, "acct", props[iceio.ADLSSharedKeyAccountName])
+ assert.Equal(t, "working-key", props[iceio.ADLSSharedKeyAccountKey])
+ assert.Equal(t, "working-sas", props[iceio.ADLSSasTokenPrefix+host])
+}
+
+func TestVendedCredsRefreshClearsStaleADLSAuth(t *testing.T) {
+ const host = "acct.dfs.core.windows.net"
+
+ var captured iceberg.Properties
+ iceio.Register("vended-adls-refresh-test", func(_ context.Context, _
*url.URL, props map[string]string) (iceio.IO, error) {
Review Comment:
Fixed: it unregisters in `t.Cleanup` now. `ioPropsRecorder` lives in
`package rest_test`, and this test needs `package rest` internals
(`newTestRefresher`, `loadFS`), so I mirrored its cleanup instead of reusing it.
--
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]
