review

Author: adutra

CHANGELOG.md

@@ -29,6 +29,10 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 
 ### Highlights
 
+- Support for S3 request signing has been added, allowing Polaris to work with S3-compatible object storage systems.
+  *Remote signing is currently experimental and not enabled by default*. To enable it, either set the system-wide property
+  `REMOTE_SIGNING_ENABLED` to `true`, or the catalog-level `polaris.request-signing.enabled` property to `true`.
+
 ### Upgrade notes
 
 ### Breaking changes

api/iceberg-aws-sign-service/src/main/java/org/apache/polaris/service/aws/sign/model/PolarisS3SignRequest.java

@@ -37,7 +37,7 @@
 public interface PolarisS3SignRequest extends S3SignRequest {
 
   @Value.Default
-  @Nullable // Replace javax.annotation.Nullable with jakarta.annotation.Nullable
+  @Nullable // Replace javax.annotation.Nullable from S3SignRequest with jakarta.annotation.Nullable
   @Override
   default String body() {
     return null;

polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java

@@ -355,7 +355,8 @@ public static void enforceFeatureEnabledOrThrow(
           .key("REMOTE_SIGNING_ENABLED")
           .catalogConfig("polaris.config.remote-signing.enabled")
           .description(
-              "If true, the remote signing endpoints are enabled either globally, or for a specific catalog.")
+              "If true, the remote signing endpoints are enabled either globally, or for a specific catalog. "
+                  + "This feature is currently experimental and may change in future releases.")
           .defaultValue(false)
           .buildFeatureConfiguration();
 

polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java

@@ -26,7 +26,6 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import org.apache.iceberg.exceptions.BadRequestException;
@@ -207,7 +206,7 @@ public Catalog.TypeEnum getCatalogType() {
   }
 
   public boolean isExternal() {
-    return Objects.equals(getCatalogType(), Catalog.TypeEnum.EXTERNAL);
+    return getCatalogType() == Catalog.TypeEnum.EXTERNAL;
   }
 
   public boolean isPassthroughFacade() {

polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisEndpoints.java

@@ -106,11 +106,9 @@ public static Set<Endpoint> getSupportedPolicyEndpoints(RealmConfig realmConfig)
    * remote signing; otherwise, returns an empty set.
    */
   public static Set<Endpoint> getSupportedRemoteSigningEndpoints(
-      CallContext callContext, CatalogEntity catalogEntity) {
+      RealmConfig realmConfig, CatalogEntity catalogEntity) {
     boolean remoteSigningEnabled =
-        callContext
-            .getRealmConfig()
-            .getConfig(FeatureConfiguration.REMOTE_SIGNING_ENABLED, catalogEntity);
+        realmConfig.getConfig(FeatureConfiguration.REMOTE_SIGNING_ENABLED, catalogEntity);
     return remoteSigningEnabled ? REMOTE_SIGNING_ENDPOINTS : ImmutableSet.of();
   }
 }

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

@@ -18,7 +18,6 @@
  */
 package org.apache.polaris.service.catalog.iceberg;
 
-import static org.apache.polaris.core.storage.PolarisStorageConfigurationInfo.deserialize;
 import static org.apache.polaris.service.exception.IcebergExceptionMapper.isStorageProviderRetryableException;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -130,9 +129,9 @@
 import org.apache.polaris.core.storage.PolarisStorageIntegration;
 import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider;
 import org.apache.polaris.core.storage.StorageLocation;
+import org.apache.polaris.core.storage.StorageUtil;
 import org.apache.polaris.core.storage.aws.AwsCredentialsStorageIntegration;
 import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
-import org.apache.polaris.core.storage.StorageUtil;
 import org.apache.polaris.core.storage.cache.StorageCredentialCache;
 import org.apache.polaris.service.catalog.CatalogPrefixParser;
 import org.apache.polaris.service.catalog.SupportsNotifications;
@@ -877,11 +876,9 @@ public AccessConfig getAccessConfigForRemoteSigning(TableIdentifier tableIdentif
 
     Optional<PolarisStorageConfigurationInfo> configurationInfo =
         findStorageInfo(tableIdentifier)
-            .map(
-                info ->
-                    deserialize(
-                        info.getInternalPropertiesAsMap()
-                            .get(PolarisEntityConstants.getStorageConfigInfoPropertyName())));
+            .map(PolarisEntity::getInternalPropertiesAsMap)
+            .map(info -> info.get(PolarisEntityConstants.getStorageConfigInfoPropertyName()))
+            .map(PolarisStorageConfigurationInfo::deserialize);
 
     if (configurationInfo.isEmpty()) {
       LOGGER

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java

@@ -803,7 +803,7 @@ public Response getConfig(
                         .addAll(PolarisEndpoints.getSupportedPolicyEndpoints(realmConfig))
                         .addAll(
                             PolarisEndpoints.getSupportedRemoteSigningEndpoints(
-                                callContext, catalogEntity))
+                                callContext.getRealmConfig(), catalogEntity))
                         .build())
                 .build())
         .build();

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java

@@ -701,17 +701,16 @@ public Optional<LoadTableResponse> loadTableWithAccessDelegationIfStale(
 
     if (delegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) {
 
-      LOGGER.info(
-          "allow external catalog credential vending: {}",
+      boolean allowExternalCatalogCredentialVending =
           callContext
               .getRealmConfig()
               .getConfig(
-                  FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, catalogEntity));
-      if (catalogEntity.isExternal()
-          && !callContext
-              .getRealmConfig()
-              .getConfig(
-                  FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, catalogEntity)) {
+                  FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING, catalogEntity);
+
+      LOGGER.info(
+          "allow external catalog credential vending: {}", allowExternalCatalogCredentialVending);
+
+      if (catalogEntity.isExternal() && !allowExternalCatalogCredentialVending) {
         throw new ForbiddenException(
             "Access Delegation is not enabled for this catalog. Please consult applicable "
                 + "documentation for the catalog config property '%s' to enable this feature",
@@ -776,26 +775,17 @@ private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredential
       CatalogEntity catalogEntity) {
     LoadTableResponse.Builder responseBuilder =
         LoadTableResponse.builder().withTableMetadata(tableMetadata);
+    AccessConfig accessConfig = null;
     if (baseCatalog instanceof SupportsCredentialDelegation credentialDelegation
         && delegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS)) {
       LOGGER
           .atDebug()
           .addKeyValue("tableIdentifier", tableIdentifier)
           .addKeyValue("tableLocation", tableMetadata.location())
           .log("Fetching client credentials for table");
-      AccessConfig accessConfig =
+      accessConfig =
           credentialDelegation.getAccessConfigForCredentialDelegation(
               tableIdentifier, tableMetadata, actions);
-      Map<String, String> credentialConfig = accessConfig.credentials();
-      responseBuilder.addAllConfig(credentialConfig);
-      responseBuilder.addAllConfig(accessConfig.extraProperties());
-      if (!credentialConfig.isEmpty()) {
-        responseBuilder.addCredential(
-            ImmutableCredential.builder()
-                .prefix(tableMetadata.location())
-                .config(credentialConfig)
-                .build());
-      }
     } else if (baseCatalog instanceof SupportsRemoteSigning remoteSigning
         && delegationModes.contains(AccessDelegationMode.REMOTE_SIGNING)) {
       S3RemoteSigningCatalogHandler.throwIfRemoteSigningNotEnabled(
@@ -805,7 +795,9 @@ private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredential
           .addKeyValue("tableIdentifier", tableIdentifier)
           .addKeyValue("tableLocation", tableMetadata.location())
           .log("Enabling remote signing for table");
-      AccessConfig accessConfig = remoteSigning.getAccessConfigForRemoteSigning(tableIdentifier);
+      accessConfig = remoteSigning.getAccessConfigForRemoteSigning(tableIdentifier);
+    }
+    if (accessConfig != null) {
       Map<String, String> credentialConfig = accessConfig.credentials();
       responseBuilder.addAllConfig(credentialConfig);
       responseBuilder.addAllConfig(accessConfig.extraProperties());

runtime/service/src/main/java/org/apache/polaris/service/storage/aws/signer/S3RemoteSigningCatalogHandler.java

@@ -70,6 +70,8 @@ public PolarisS3SignResponse signS3Request(
 
     LOGGER.debug("Requesting s3 signing for {}: {}", tableIdentifier, s3SignRequest);
 
+    throwIfRemoteSigningNotEnabled(callContext.getRealmConfig(), catalogEntity);
+
     // TODO authorize based on the request's method?
 
     try {
@@ -83,8 +85,6 @@ public PolarisS3SignResponse signS3Request(
           PolarisAuthorizableOperation.SIGN_S3_REQUEST, tableIdentifier);
     }
 
-    throwIfRemoteSigningNotEnabled(callContext.getRealmConfig(), catalogEntity);
-
     PolarisS3SignResponse s3SignResponse = s3RequestSigner.signRequest(s3SignRequest);
     LOGGER.debug("S3 signing response: {}", s3SignResponse);