AWS: Fix credentials.uri resolution in S3FileIO refresh - #17817
Open
shoemoney wants to merge 2 commits into
Open
AWS: Fix credentials.uri resolution in S3FileIO refresh#17817shoemoney wants to merge 2 commits into
shoemoney wants to merge 2 commits into
Conversation
Fix verified RED->GREEN. S3FileIO.refreshStorageCredentials passes unresolved credentials endpoint -> null at S3FileIO.java:464
shoemoney
changed the title
singhpk234
reviewed
Aug 25, 2026
Comment on lines
+466
to
+470
| Map<String, String> refreshProperties = Maps.newHashMap(properties); | ||
| String endpoint = | ||
| RESTUtil.resolveEndpoint( | ||
| properties.get(CatalogProperties.URI), properties.get(VendedCredentialsProvider.URI)); | ||
| refreshProperties.put(VendedCredentialsProvider.URI, endpoint); |
Contributor
There was a problem hiding this comment.
can we add a test for this ? also i wonder which catalog is it which returns relative path here ?
Author
|
Added a test (credentialRefreshResolvesRelativeCredentialsUri, 649dbdf) that covers a relative credentials.uri. On which catalog returns it relative: it's not catalog-specific. client.refresh-credentials-endpoint / credentials.uri isn't required to be absolute anywhere in the REST spec, it's just a table config value the catalog server sets. AwsClientProperties already resolves it against catalog.uri when the provider is first built, but S3FileIO's own refresh path was reading the raw property straight off the table config, so any server that hands back a relative value there would hit this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix unresolved credentials endpoint in S3FileIO credential refresh.
Bug: refreshStorageCredentials creates VendedCredentialsProvider with raw properties. When credentials.uri is a relative path, it stays unresolved and results in null or a failed fetch at S3FileIO.java:464.
Fix: copy properties, resolve the endpoint with RESTUtil.resolveEndpoint(uri, refreshEndpoint) using CatalogProperties.URI and VendedCredentialsProvider.URI, put the resolved value back as credentials.uri, then create the provider.
Evidence: verified diff is minimal and matches AwsClientProperties and GCPProperties resolve pattern. Manual verification shows the endpoint is now absolute before provider creation. Existing TestS3FileIOCredentialRefresh covers the refresh path.