aritragster commented on code in PR #4052:
URL: https://github.com/apache/polaris/pull/4052#discussion_r3047372379
##########
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;
+
+ if (isS3Tables && capturedTableId.isPresent()) {
+ String tableArn = constructS3TablesArn(catalogEntity,
capturedTableId.get());
+ validateS3TablesArn(tableIdentifier, tableArn, catalogEntity);
+ tableLocations = Set.of(tableArn);
+ LOGGER
+ .atDebug()
+ .addKeyValue("tableIdentifier", tableIdentifier)
Review Comment:
Agreed. I changed it from warn-and-continue to BadRequestException. If the
catalog is S3_TABLES and no tableId was captured, we now refuse to vend
credentials rather than proceeding with unscoped permissions.
--
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]
