aritragster commented on code in PR #4052:
URL: https://github.com/apache/polaris/pull/4052#discussion_r3047380956
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -855,8 +865,35 @@ private LoadTableResponse.Builder
buildLoadTableResponseWithDelegationCredential
Set<String> tableLocations =
StorageUtil.getLocationsUsedByTable(tableMetadata);
- // For non polaris' catalog, validate that table locations are within
allowed locations
- if (!(baseCatalog instanceof IcebergCatalog)) {
+ // For S3 Tables catalogs, replace s3:// table locations with the
constructed table ARN.
+ // s3tables:* IAM actions require ARN resources, not s3:// paths.
+ CatalogEntity catalogEntity =
CatalogEntity.of(getResolvedCatalogEntity());
+ boolean isS3Tables =
+ catalogEntity.getStorageConfigurationInfo() != null
+ && catalogEntity.getStorageConfigurationInfo().getStorageType()
+ == PolarisStorageConfigurationInfo.StorageType.S3_TABLES;
Review Comment:
Done on the private function - extracted to isS3TablesCatalog() with a
comment documenting the assumption that the federated catalog loadTable call
has already succeeded.
On the CapturedConfigHolder: the problem it solves is that the S3 Tables
tableId only exists in the config section of the remote loadTable REST
response. When Polaris calls baseCatalog.loadTable(), the Iceberg RESTCatalog
internally deserializes the HTTP response, consumes the config map (for token
refresh, endpoint config, etc.), and returns a BaseTable object. The
BaseTable/TableMetadata doesn't expose the config section - by the time our
handler sees the table, the tableId is gone.
The CapturedConfigHolder is a request-scoped bean that bridges this gap. The
ConfigCapturingHTTPClient wraps the RESTClient at the HTTP layer (the only
point where the config is visible), extracts the tableId, and stashes it in the
holder. After baseCatalog.loadTable() returns, the handler reads the tableId
from the holder to construct the table ARN.
Let me know what you think of this.
--
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]
