aritragster commented on code in PR #4052:
URL: https://github.com/apache/polaris/pull/4052#discussion_r2985603851
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -812,6 +816,13 @@ public Optional<LoadTableResponse> loadTable(
// when data-access is specified but access delegation grants are not
found.
Table table = baseCatalog.loadTable(tableIdentifier);
+ // Capture tableId from remote catalog response for S3 Tables ARN
construction
+ Optional<String> capturedTableId = capturedConfigHolder().getTableId();
+ List<String> resourceArns = List.of();
+ if (capturedTableId.isPresent()) {
+ resourceArns = List.of(constructS3TablesArn(capturedTableId.get()));
Review Comment:
I took a look at #3699 and the direction there looks like a natural fit for
what S3 Tables needs. The key difference with S3 Tables is that the resource
identifier is an ARN (not an s3:// path) and the IAM actions use the s3tables:
namespace, but the overall credential vending flow is the same. If #3699's
refactored abstractions can accommodate ARN-based resources alongside
path-based ones, S3 Tables support should fit in cleanly.
Happy to rebase this PR on top of #3699 once it lands, or collaborate with
@tokoko to make sure the refactoring accounts for the S3 Tables case. In the
meantime I'll keep this draft up as a reference for the S3 Tables-specific
logic (policy generation, signingName detection, tableId capture).
Let me know what you'd prefer — happy to help either way.
--
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]
