Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AWS, Core, GCP: Support relative credential endpoint / pass OAuth2 token to credential provider #11954

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

nastra
Copy link
Contributor

@nastra nastra commented Jan 13, 2025

The goal of this PR is to improve the credential URI / token handling when a server sends back properties to a client in order to indicate that refreshing vended credentials is supported. In detail, the following things are improved here:

  • currently only a full URI can be passed as the credentials.uri. However, the catalog URI itself is already passed via the properties and so having support for a relative credential path can make things easier for servers. This is being changed for S3 + GCP in this PR.
  • The VendedCredentialsProvider (specific to S3) currently requires the OAuth2 token to be passed via client.credentials-provider.token in order to be properly passed down from AwsClientProperties to the VendedCredentialsProvider. This is being simplified in this PR so that the same OAuth2 token is passed down to VendedCredentialsProvider that already exists in the properties under token. Note that client.credentials-provider.token still takes precedence in case that property exists

@nastra nastra force-pushed the refresh-credentials-improvements branch from 90f7152 to 71cde42 Compare January 13, 2025 10:32
@nastra nastra requested a review from danielcweeks January 13, 2025 10:34
@nastra nastra force-pushed the refresh-credentials-improvements branch from 71cde42 to e555a96 Compare January 13, 2025 10:49
@nastra nastra closed this Jan 13, 2025
@nastra nastra reopened this Jan 13, 2025
@danielcweeks
Copy link
Contributor

Minor comments and we should add a description, but LGTM.

@nastra nastra force-pushed the refresh-credentials-improvements branch from e555a96 to 1212bc0 Compare January 14, 2025 15:37
@nastra nastra requested a review from danielcweeks January 14, 2025 15:45
@nastra nastra force-pushed the refresh-credentials-improvements branch from 1212bc0 to 362bf01 Compare January 14, 2025 15:48
@nastra nastra closed this Jan 15, 2025
@nastra nastra reopened this Jan 15, 2025
@nastra nastra added this to the Iceberg 1.7.2 milestone Jan 15, 2025
@nastra nastra closed this Jan 15, 2025
@nastra nastra reopened this Jan 15, 2025
@nastra nastra force-pushed the refresh-credentials-improvements branch 2 times, most recently from a2dccd1 to 3d42850 Compare January 15, 2025 07:33
@github-actions github-actions bot added the build label Jan 15, 2025
@nastra nastra force-pushed the refresh-credentials-improvements branch from 3d42850 to ad6b255 Compare January 15, 2025 08:43
@nastra nastra merged commit 978189c into apache:main Jan 15, 2025
46 checks passed
@nastra nastra deleted the refresh-credentials-improvements branch January 15, 2025 10:07
@jbonofre jbonofre modified the milestones: Iceberg 1.7.2, Iceberg 1.8.0 Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants