You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "ganeshsa986 (via GitHub)" <gi...@apache.org> on 2026/10/19 08:41:14 UTC
Re: [PR] feat: Add S3 Tables credential vending support for federated catalogs [polaris]
ganeshsa986 commented on PR #4052:
URL: https://github.com/apache/polaris/pull/4052#issuecomment-6263224583
We have been evaluating Polaris as a governance layer in front of Amazon S3 Tables, hit exactly the
failure this PR describes on 1.7.0, and have since deployed a build of this branch and exercised it
against a real S3 Tables table bucket. Five things from that which may be useful, and an offer at
the end.
### 1. Independent reproduction of the failure mode on 1.7.0
Federating an `EXTERNAL` catalog to the S3 Tables Iceberg REST endpoint works for metadata.
`listNamespaces`, `listTables` and `loadTable` all succeed once `remoteCatalogName` is set to the
table bucket ARN.
Credential vending then appears to succeed and does not work. Polaris returns a well-formed `ASIA`
key with session token and expiry, and the credential authorises nothing — `GetObject` against the
table's warehouse location returns `AccessDenied`.
The cause matches this PR's problem statement. The storage role holds `s3tables:GetTableData` on
`arn:aws:s3tables:REGION:ACCOUNT:bucket/BUCKET/table/*`, while the generated session policy
grants `s3:GetObject` on the `s3://` warehouse path. Effective permission is role ∩ session policy,
which is empty. Worth noting that the role on its own can read the table; it is the scoping step
that removes the access. The failure surfaces as a bare `AccessDenied` at first read, a long way
from the cause.
### 2. The remote endpoint does return `tableId`
`S3TablesUtil.resolveTableLocations` fails closed with *"Ensure the remote S3 Tables endpoint
returns tableId in the loadTable config."* I could not tell from the thread whether that had been
confirmed against real AWS, so we issued a SigV4-signed `loadTable` directly against
`https://s3tables.REGION.amazonaws.com/iceberg`:
```
HTTP 200
top-level keys: ['config', 'metadata', 'metadata-location']
config keys: ['namespaceId', 'tableBucketId', 'tableId']
```
So the key this PR depends on is present. Two further details:
- The `tableId` is the same UUID that prefixes the AWS-generated warehouse bucket name
(`s3://TABLEID-PREFIX...--table-s3/metadata/...`), so a constructed ARN can be cross-checked
against `metadata-location`.
- `namespaceId` and `tableBucketId` are returned as well. Since `tableBucketId` is available, the
table ARN could in principle be assembled without relying on the catalog's
`default-base-location`, if that is ever preferable to the current validation requirement.
### 3. The ARN base-location validation removes a real workaround
On 1.7.0, catalog creation rejects an ARN location:
```
Location prefix not allowed: 'arn:aws:s3tables:...' expected prefixes: 's3://,s3a://'
```
The workaround is to register a placeholder `s3://` location and then add each table's
AWS-generated `--table-s3` bucket to `allowedLocations` individually. Those bucket names are
per-table and not predictable, so it does not scale past a demo. Expressed as an S3 Tables ARN,
`arn:aws:s3tables:REGION:ACCOUNT:bucket/BUCKET/table/*` is a stable prefix covering every
table in the bucket, which is what the validation in this PR enforces.
### 4. End-to-end validation of this branch against a production table bucket
Running this branch's build, with `SUPPORTED_CATALOG_STORAGE_TYPES=["S3","S3_TABLES"]`,
`ENABLE_CATALOG_FEDERATION` and `ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS`, against a table
bucket containing four live tables, with a read-only storage role scoped to those four table ARNs:
- Vended credential assumes the expected role — session name
`PolarisAwsS3TablesCredentialsStorageIntegration`.
- Read through the vended credential succeeds: `format-version 3`, 31 fields, correct `table-uuid`.
- `PutObject` through the same credential is refused by AWS, as the role is read-only.
- `loadTable` on a table in a namespace the principal was not granted is refused by Polaris at
`LOAD_TABLE_WITH_READ_DELEGATION`.
- A credential vended for one namespace gets `AccessDenied` against another namespace's storage,
i.e. the scoping is doing real work rather than vending a bucket-wide credential.
Three principals with deliberately different table grants behaved as expected, including the case
where a principal can list a table and is refused on `loadTable` — so a denial reads as a denial
rather than as absence.
**What this does not cover, and it is the gap flyingImer raised on 2026-03-28 and 2026-04-01:** our
storage role is read-only, so the write and create paths are untested here. We cannot speak to
whether create-time scoping is correct. If it would help, we can stand up a writable role and
exercise `createTable` / `UpdateTableMetadataLocation` against the same bucket.
### 5. `validateAllowedStorageType=false` does not have the intended effect
This substantiates flyingImer's 2026-04-30 point #1 concretely. The branch adds:
```java
S3("org.apache.iceberg.aws.s3.S3FileIO", true),
S3_TABLES("org.apache.iceberg.aws.s3.S3FileIO", true, false),
```
Both entries carry the same FileIO implementation string, and the static initializer in
`StorageTypeFileIO` populates the reverse map unconditionally:
```java
for (var value : StorageTypeFileIO.values()) {
if (value.validateAllowedStorageType()) { ... }
map.put(value.fileIoImplementation, value); // runs for every value
}
```
`values()` iterates in declaration order, so `S3_TABLES` is written last and wins. Confirmed by
running against this branch's compiled classes:
```
enum declaration order:
S3 validateAllowedStorageType=true
S3_TABLES validateAllowedStorageType=false
...
fromFileIoImplementation("org.apache.iceberg.aws.s3.S3FileIO")
resolved to : S3_TABLES
validateAllowedStorageType: false
```
So `fromFileIoImplementation` returns `S3_TABLES` for **every** S3 catalog, not only S3 Tables ones.
Because `S3_TABLES` opts out of `validateAllowedStorageType`, this check in
`IcebergPropertiesValidation` is then skipped for plain S3 catalogs as well:
```java
if (storageType.validateAllowedStorageType()
&& !realmConfig.getConfig(SUPPORTED_CATALOG_STORAGE_TYPES).contains(storageType.name())) {
throw new ValidationException(...);
}
```
So a realm that removes `S3` from `SUPPORTED_CATALOG_STORAGE_TYPES` in order to disable S3 catalogs
would find the restriction silently ineffective. Nothing visible breaks today because both entries
are `safe=true`, which is presumably why it passes tests. On main there is no `S3_TABLES` entry, so
nothing collides and `S3` resolves correctly. Happy to open this as a standalone issue with a
failing test if that is preferred to folding it into the redesign below.
### 6. A shape that fits current main, with one open question
main has moved on from #3699. There is now an `S3CredentialVendingMechanism` SPI — CDI beans
annotated `@Identifier("<mechanism>")`, selected per-catalog by `credentialVendingMechanism`, with
a realm allowlist in `SUPPORTED_S3_CREDENTIAL_VENDING_MECHANISMS` and `STS` as the default.
`StsCredentialVendingMechanism` is ~80 lines and does nothing but return an integration from
`integrationFor(...)`.
S3 Tables looks like a natural second mechanism:
- `S3TablesCredentialVendingMechanism` with `@Identifier("S3_TABLES")`, returning an integration
that extends `CachingStorageIntegration` and implements `getStorageAccessConfig(List<LocationGrant>,
...)`, `buildCacheKey` and `addExtraProperties`.
- The `s3tables:` policy builder in `AwsS3TablesCredentialsStorageIntegration` carries over
essentially unchanged; read and write ARN sets come from partitioning the grants by action, the
same way `AwsCredentialsStorageIntegration` already derives `readLocations` / `writeLocations`.
- `S3TablesUtil`, `ConfigCapturingHTTPClient` and `CapturedConfigHolder` carry over as-is, with the
existing TODO pointing at apache/iceberg#16399.
- `AwsStsUtil`, the `StorageType.S3_TABLES` entry, the `StorageTypeFileIO` opt-out and
`validateAccessToLocations` all drop out — STS handling now lives in the mechanism plus the
caching base class, and `validateAccessToLocations` no longer exists on main.
On the contract question from 2026-08-25 — whether `LocationGrant.locations` can represent a
backend-native resource such as an S3 Tables table ARN — it appears to need no change.
`LocationGrant` is `(Set<String> locations, Set<PolarisStorageActions> actions)`, the core does not
parse those strings, and every consumer of `locations()` is inside a provider's own integration. A
dedicated S3 Tables integration can read them as ARNs. So the abstraction does not need
generalizing for this case.
**The open question is the location prefix, not the grant.** `AwsStorageConfigurationInfo`
hardcodes `StorageType.S3`, whose prefixes are `s3://` and `s3a://`, and
`validatePrefixForStorageType` runs when the storage config is constructed — before
`validateS3CredentialVendingMechanism` in `PolarisAdminService` — so a mechanism's `validate()`
hook cannot accept an ARN location that the prefix check has already refused.
Two ways to resolve it, and we would rather be told which is acceptable than guess:
- **(a)** Add `arn:aws:s3tables:` to the prefixes accepted for `StorageType.S3`, and enforce the
correspondence in the mechanism hooks — `STS.validate()` refuses ARN locations, and
`S3_TABLES.validate()` requires them. No new storage type, so no FileIO-string collision and no
opt-out. The cost is that location-shape enforcement moves from the config class into the
mechanisms.
- **(b)** Introduce `StorageType.S3_TABLES` properly, which then requires fixing the
`FileIO impl -> storage type` reverse lookup rather than opting out of it — i.e. the structural
fix tracked in #4486, since two storage types legitimately share `S3FileIO`.
(a) is smaller and avoids #4486 entirely; (b) is the more honest modelling if a storage type is
considered the right unit. Either is fine by us.
### Status, and an offer
@aritragster — are you still carrying this forward? The 2026-08-25 question has been open for a
while, and we did not want to duplicate work.
If you are, we are glad to help with validation: we have a live S3 Tables table bucket, a working
federated catalog, and the harness described in section 4.
If you are not, we are willing to do the port described in section 6 and open it as a follow-up PR,
keeping you as co-author on the commits that carry your code. The S3 Tables knowledge in this PR —
the `s3tables:` action set, the ARN construction, and the `tableId` capture — is the hard part and
it is all still correct; what has gone stale is the surrounding vending shape, which main has since
replaced.
Before starting we would want a steer on the (a)/(b) question above, so the port lands in a shape
reviewers are willing to take.
cc @flyingImer @singhpk234 @dimas-b
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org