aaasif04-github opened a new pull request, #18757: URL: https://github.com/apache/iceberg/pull/18757
When a catalog returns more than one S3 credential, `VendedCredentialsProvider` rejects the refresh response with "only one S3 credential should exist", and long-running jobs fail when the first credentials expire. `VendedAdlsCredentialProvider` does the same for more than one ADLS credential per storage account. The REST spec allows more than one credential of the same type and says clients should choose the longest matching prefix. - **S3:** `S3FileIO` already builds one S3 client per credential prefix, but the refresh provider does not know which prefix its client serves. `PrefixedS3Client` now passes its storage prefix to the client factory in `credentials.storage-prefix` (next to the existing internal `credentials.uri`), and `VendedCredentialsProvider` uses the credential with the longest prefix that the storage prefix starts with. With a single credential, it behaves as before. - **ADLS:** `ADLSFileIO` keeps one SAS token per storage account and has no per-prefix clients, so there is no storage prefix to select by. `VendedAdlsCredentialProvider` now accepts several credentials for a storage account when they carry the same SAS token. Different tokens for one storage account are still rejected. This follows #18716, which does the same for GCS. Nessie (projectnessie/nessie#13128 for GCS, projectnessie/nessie#13269 for S3 and ADLS) sends the same credential once for each of the table's locations, so its refresh responses contain more than one credential. Test plan: - `TestVendedCredentialsProvider`: with more than one S3 credential and a storage prefix, the provider picks the longest prefix that covers the storage prefix and skips a longer one that does not cover it; the refresh fails when none covers it; one S3 credential is used even when its prefix does not cover the storage prefix. The existing test without a storage prefix still expects "only one S3 credential should exist". - `TestS3FileIOCredentialRefresh.prefixedClientsRefreshWithTheirOwnCredential`: `S3FileIO` with two storage credentials refreshes each prefix client with its own credential. Without the `PrefixedS3Client` change it fails with "only one S3 credential should exist". - `TestVendedAdlsCredentialProvider.sameSasTokenForMultiplePrefixesOfStorageAccount`: the same SAS token for two prefixes of one storage account is accepted. The existing test with different tokens still fails. - `./gradlew :iceberg-aws:test :iceberg-azure:test` for the classes above, and checkstyle and spotless for both modules. - Not tested against real S3 or ADLS. Points for review: - `credentials.storage-prefix` is passed through the client factory properties, because `AwsClientProperties` creates the provider by reflection from those properties. The root client (prefix `s3`) also passes it; with several credentials, none of which covers `s3`, its refresh fails as before, but with a clearer message. - For ADLS I chose "same token" over "longest prefix", because a single token per storage account serves every path. Happy to change it if reviewers prefer another rule. --- **AI Disclosure** - Model: Claude Opus 5.5 - Platform/Tool: Claude Code - Human Oversight: fully reviewed - Prompt Summary: Let S3 and ADLS refresh vended credentials when a catalog returns more than one credential, following #18716 for GCS. Claude generated the code, the tests and this description; the author reviewed the approach and the changes. -- 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]
