Support region, endpoint, and cross-region access from Iceberg REST catalog vended credentials#27922
Support region, endpoint, and cross-region access from Iceberg REST catalog vended credentials#27922kaveti wants to merge 1 commit into
Conversation
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
4 similar comments
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
|
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: rkaveti.
|
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
3 similar comments
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
2 similar comments
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
1 similar comment
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
@Praveen2112 i have addressed your review comments. could you please review it again. |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
@krvikash can you please review |
1adbc2a to
08dbc6a
Compare
|
@krvikash can you please review. |
2051ef5 to
a8224b0
Compare
|
@krvikash gentle reminder. |
|
Gentle reminder. @raunaqmorarka , @Praveen2112 could you please review. |
4294e11 to
7f8f75c
Compare
10aa3ab to
6107560
Compare
| * - `iceberg.rest-catalog.vended-credentials-enabled` | ||
| - Use credentials provided by the REST backend for file system access. | ||
| - Use credentials and configuration provided by the REST backend for file system access. | ||
| When enabled, the REST catalog can provide S3 credentials (`s3.access-key-id`, |
There was a problem hiding this comment.
Thanks for the work, agree on motivation.
s3.path-style-access is pretty important for non-AWS S3 setups, and both Trino and Iceberg REST support s3.path-style-access property named in the same way, wonder if we could add support for that being vended too? I think it goes along with region and endpoint
There was a problem hiding this comment.
added support for s3.path-style-access
|
gentle reminder @Praveen2112 @krvikash @findinpath for review. |
|
@smaheshwar-pltr can you please review. |
|
@krvikash @Praveen2112 @findinpath PTAL. |
|
@krvikash @Praveen2112 @findinpath PTAL. |
464755a to
24430fb
Compare
Convert S3SecurityMappingProvider to an interface with extensible credential mapping
|
@krvikash @Praveen2112 @findinpath PTAL. |
Support vended credentials from Iceberg REST catalog
Description
Adds support for
client.region,s3.endpoint,s3.cross-region-access-enabled,and
s3.path-style-accessfrom Iceberg REST catalog vended S3 credentials.Also fixes
ConnectorIdentity.extraCredentialshandling so existing extra credentialsare preserved when vended S3 properties are applied.
Problem
When using vended credentials with Iceberg REST catalog, several S3 client settings
from the catalog response were being ignored by the native S3 filesystem. This meant:
ConnectorIdentity.extraCredentialsinstead of merging themSolution
This extends the existing S3 security mapping mechanism to support vended credentials
and related S3 connectivity properties:
S3SecurityMappingProviderto an interface, withDefaultS3SecurityMappingProviderpreserving the existing file/URI-based security mapping behavior
VendedCredentialsS3SecurityMappingProviderin the Iceberg plugin that extractsvended credentials and properties from
ConnectorIdentityand returns anS3SecurityMappingResultS3SecurityMappingResultto carry optionalpathStyleAccessS3FileSystemModulenow routes throughS3FileSystemLoaderwith an optionalS3SecurityMappingProvider, which defaults to empty when no mapping is configuredConnectorIdentity.extraCredentialswith thevended S3 properties from the Iceberg REST catalog response and merges them with any existing
extra credentials instead of overwriting them
IcebergVendedCredentialsModulebindsVendedCredentialsS3SecurityMappingProviderwheniceberg.rest-catalog.vended-credentials-enabled=trueS3FileSystemLoadernow applies mapping-aware region, endpoint, cross-region access,and path-style access when creating S3 clients
S3FileSystemUtilsnow applies the same mapping-aware settings when creating S3 presigners,so pre-signed URIs use the correct endpoint/addressing mode
s3.cross-region-access-enabledand
s3.path-style-accessS3FileSystemLoaderreusesS3Clientinstances per unique credentialconfiguration, including path-style access
Precedence order:
S3 FileIO properties not currently considered
The Iceberg S3 FileIO specification
defines additional properties that an Iceberg REST catalog could potentially vend, including
but not limited to:
s3.sse.type, s3.sse.key,s3.sse.md5(server-side encryption)s3.acl,s3.access-grants.enabled,s3.access-grants.fallback-to-iams3.access-points.*s3.write.tags.*,s3.delete.tags.*s3.remote-signing-enabledThis PR focuses on the properties that affect S3 client connectivity — credentials,
region, endpoint, cross-region access, and path-style access — as these are the values
required to connect correctly to multi-region or S3-compatible REST-catalog-managed storage.
Support for additional vended properties can be added incrementally via the
S3SecurityMappingProviderextension point introduced here.Example
When the REST catalog returns:
{ "config": { "client.region": "eu-west-1", "s3.endpoint": "https://s3.eu-west-1.amazonaws.com", "s3.cross-region-access-enabled": "true", "s3.path-style-access": "true", "s3.access-key-id": "...", "s3.secret-access-key": "...", "s3.session-token": "..." } }Related
Fixes #27920
Description
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: