From d6066faa2dc4f1850420b3abf39b47a1be0585d5 Mon Sep 17 00:00:00 2001 From: Jason Date: Wed, 13 Aug 2025 11:11:00 +0300 Subject: [PATCH 01/23] add refresh credentials property to loadTableResult --- .../core/rest/PolarisResourcePaths.java | 11 ++++++ .../iceberg/IcebergCatalogAdapter.java | 34 ++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java b/polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java index 8a30d79624..16eea08da2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/rest/PolarisResourcePaths.java @@ -57,6 +57,17 @@ public String genericTables(Namespace ns) { "polaris", "v1", prefix, "namespaces", RESTUtil.encodeNamespace(ns), "generic-tables"); } + public String credentialsPath(TableIdentifier ident) { + return SLASH.join( + "v1", + prefix, + "namespaces", + RESTUtil.encodeNamespace(ident.namespace()), + "tables", + RESTUtil.encodeString(ident.name()), + "credentials"); + } + public String genericTable(TableIdentifier ident) { return SLASH.join( "polaris", diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 76401582a9..970dfc1325 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -40,6 +40,7 @@ import java.util.Set; import java.util.function.Function; import org.apache.iceberg.MetadataUpdate; +import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.BadRequestException; @@ -75,7 +76,9 @@ import org.apache.polaris.core.persistence.resolver.ResolverFactory; import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.rest.PolarisEndpoints; +import org.apache.polaris.core.rest.PolarisResourcePaths; import org.apache.polaris.core.secrets.UserSecretsManager; +import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.service.catalog.AccessDelegationMode; import org.apache.polaris.service.catalog.CatalogPrefixParser; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; @@ -430,16 +433,45 @@ public Response loadTable( .loadTableIfStale(tableIdentifier, ifNoneMatch, snapshots) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } else { - response = + LoadTableResponse originalResponse = catalog .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); + + if (delegationModes.contains(VENDED_CREDENTIALS)) { + response = + injectRefreshVendedCredentialProperties( + originalResponse, + new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier)); + } else { + response = originalResponse; + } } return tryInsertETagHeader(Response.ok(response), response, namespace, table).build(); }); } + private LoadTableResponse injectRefreshVendedCredentialProperties( + LoadTableResponse originalResponse, String credentialsEndpoint) { + LoadTableResponse.Builder loadResponseBuilder = + LoadTableResponse.builder().withTableMetadata(originalResponse.tableMetadata()); + loadResponseBuilder.addAllConfig(originalResponse.config()); + loadResponseBuilder.addAllCredentials(originalResponse.credentials()); + loadResponseBuilder.addConfig( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); + // Only enable credential refresh for currently supported credential types + if (originalResponse.credentials().stream() + .anyMatch( + credential -> + credential + .config() + .containsKey(StorageAccessProperty.AWS_SECRET_KEY.getPropertyName()))) { + loadResponseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + } + return loadResponseBuilder.build(); + } + @Override public Response tableExists( String prefix, From 5c05a0dd397351521707b9c2d0130ca110c94855 Mon Sep 17 00:00:00 2001 From: Jason Date: Wed, 13 Aug 2025 15:07:28 +0300 Subject: [PATCH 02/23] IcebergCatalogAdapterTest: Added test to ensure refresh credentials endpoint is included --- .../iceberg/IcebergCatalogAdapterTest.java | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java index f8f948a66d..7204911e41 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java @@ -19,17 +19,23 @@ package org.apache.polaris.service.catalog.iceberg; +import static org.assertj.core.api.Assertions.assertThat; + import java.io.IOException; import java.lang.reflect.Field; import java.util.List; import java.util.Map; import java.util.stream.Stream; import org.apache.iceberg.Schema; +import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ListNamespacesResponse; import org.apache.iceberg.rest.responses.ListTablesResponse; +import org.apache.iceberg.rest.responses.LoadTableResponse; +import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AuthenticationParameters; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.BearerAuthenticationParameters; @@ -43,6 +49,7 @@ import org.assertj.core.api.Assertions; import org.assertj.core.util.Strings; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -242,4 +249,60 @@ private static Stream paginationTestCases() { Arguments.of("5", 5), Arguments.of("5", 10)); } + + @Test + void testLoadTableReturnsCredentialsRefreshEndpoint() throws IOException { + try (InMemoryCatalog inMemoryCatalog = new InMemoryCatalog()) { + // Initialize and replace the default handler with one backed by in-memory catalog + inMemoryCatalog.initialize("inMemory", Map.of()); + mockCatalogAdapter(inMemoryCatalog); + + // Create a namespace and table + String namespace = "test_ns"; + String tableName = "test_table"; + inMemoryCatalog.createNamespace(Namespace.of(namespace)); + + Schema schema = + new Schema( + Types.NestedField.required(1, "id", Types.LongType.get()), + Types.NestedField.optional(2, "name", Types.StringType.get())); + + CreateTableRequest createTableRequest = + CreateTableRequest.builder().withName(tableName).withSchema(schema).build(); + + // Create the table first + catalogAdapter.createTable( + FEDERATED_CATALOG_NAME, + namespace, + createTableRequest, + "vended-credentials", + testServices.realmContext(), + testServices.securityContext()); + + // Load the table with vended credentials access delegation mode + LoadTableResponse response = + (LoadTableResponse) + catalogAdapter + .loadTable( + FEDERATED_CATALOG_NAME, + namespace, + tableName, + "vended-credentials", + null, + null, + testServices.realmContext(), + testServices.securityContext()) + .getEntity(); + + // Verify that the response contains the credentials refresh endpoint configuration + assertThat(response.config()).containsKey(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT); + + String expectedEndpoint = + String.format( + "v1/%s/namespaces/%s/tables/%s/credentials", + FEDERATED_CATALOG_NAME, namespace, tableName); + assertThat(response.config().get(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT)) + .isEqualTo(expectedEndpoint); + } + } } From b9386a2df69dcc534a02beb41cd789dba6869883 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 18 Aug 2025 12:52:20 +0300 Subject: [PATCH 03/23] fixup! add refresh credentials property to loadTableResult --- .../iceberg/IcebergCatalogAdapter.java | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 970dfc1325..65a98a476c 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -62,6 +62,7 @@ import org.apache.iceberg.rest.responses.ConfigResponse; import org.apache.iceberg.rest.responses.ImmutableLoadCredentialsResponse; import org.apache.iceberg.rest.responses.LoadTableResponse; +import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.catalog.ExternalCatalogFactory; @@ -78,7 +79,6 @@ import org.apache.polaris.core.rest.PolarisEndpoints; import org.apache.polaris.core.rest.PolarisResourcePaths; import org.apache.polaris.core.secrets.UserSecretsManager; -import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.service.catalog.AccessDelegationMode; import org.apache.polaris.service.catalog.CatalogPrefixParser; import org.apache.polaris.service.catalog.api.IcebergRestCatalogApiService; @@ -454,22 +454,25 @@ public Response loadTable( private LoadTableResponse injectRefreshVendedCredentialProperties( LoadTableResponse originalResponse, String credentialsEndpoint) { - LoadTableResponse.Builder loadResponseBuilder = - LoadTableResponse.builder().withTableMetadata(originalResponse.tableMetadata()); - loadResponseBuilder.addAllConfig(originalResponse.config()); - loadResponseBuilder.addAllCredentials(originalResponse.credentials()); - loadResponseBuilder.addConfig( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); // Only enable credential refresh for currently supported credential types if (originalResponse.credentials().stream() .anyMatch( credential -> credential - .config() - .containsKey(StorageAccessProperty.AWS_SECRET_KEY.getPropertyName()))) { + .prefix() + .toLowerCase() + .startsWith(StorageConfigInfo.StorageTypeEnum.S3.name().toLowerCase()))) { + LoadTableResponse.Builder loadResponseBuilder = + LoadTableResponse.builder().withTableMetadata(originalResponse.tableMetadata()); + loadResponseBuilder.addAllConfig(originalResponse.config()); + loadResponseBuilder.addAllCredentials(originalResponse.credentials()); + loadResponseBuilder.addConfig( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); loadResponseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); + return loadResponseBuilder.build(); + } else { + return originalResponse; } - return loadResponseBuilder.build(); } @Override From 0e388b55399b1881bfd61734d6e330f87f769abb Mon Sep 17 00:00:00 2001 From: Jason Date: Wed, 20 Aug 2025 10:57:33 +0300 Subject: [PATCH 04/23] fixed lint error --- .../service/catalog/iceberg/IcebergCatalogAdapter.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 65a98a476c..cff8e69be4 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -35,6 +35,7 @@ import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; import java.util.EnumSet; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -460,8 +461,9 @@ private LoadTableResponse injectRefreshVendedCredentialProperties( credential -> credential .prefix() - .toLowerCase() - .startsWith(StorageConfigInfo.StorageTypeEnum.S3.name().toLowerCase()))) { + .toLowerCase(Locale.ROOT) + .startsWith( + StorageConfigInfo.StorageTypeEnum.S3.name().toLowerCase(Locale.ROOT)))) { LoadTableResponse.Builder loadResponseBuilder = LoadTableResponse.builder().withTableMetadata(originalResponse.tableMetadata()); loadResponseBuilder.addAllConfig(originalResponse.config()); From 149b1fc2d28c6f13010035eeb7dda223008f7386 Mon Sep 17 00:00:00 2001 From: Jason Date: Wed, 20 Aug 2025 12:44:39 +0300 Subject: [PATCH 05/23] delegate refresh credential endpoint configuration to storage integration --- .../AtomicOperationMetaStoreManager.java | 6 +- .../TransactionWorkspaceMetaStoreManager.java | 6 +- .../TransactionalMetaStoreManagerImpl.java | 6 +- .../core/storage/PolarisCredentialVendor.java | 3 +- .../storage/PolarisStorageIntegration.java | 3 +- .../core/storage/StorageAccessProperty.java | 26 +++++++ .../aws/AwsCredentialsStorageIntegration.java | 9 ++- .../AzureCredentialsStorageIntegration.java | 17 ++++- .../storage/cache/StorageCredentialCache.java | 6 +- .../gcp/GcpCredentialsStorageIntegration.java | 3 +- .../InMemoryStorageIntegrationTest.java | 3 +- ...zureCredentialsStorageIntegrationTest.java | 6 +- .../cache/StorageCredentialCacheTest.java | 69 ++++++++++++------- .../AwsCredentialsStorageIntegrationTest.java | 39 ++++++++--- ...AzureCredentialStorageIntegrationTest.java | 3 +- .../GcpCredentialsStorageIntegrationTest.java | 3 +- .../catalog/iceberg/IcebergCatalog.java | 6 +- .../iceberg/IcebergCatalogAdapter.java | 61 ++++++---------- .../iceberg/IcebergCatalogHandler.java | 36 +++++++--- .../iceberg/SupportsCredentialDelegation.java | 3 +- .../catalog/io/DefaultFileIOFactory.java | 3 +- .../service/catalog/io/FileIOUtil.java | 6 +- ...PolarisStorageIntegrationProviderImpl.java | 3 +- .../catalog/AbstractIcebergCatalogTest.java | 3 +- .../IcebergCatalogHandlerAuthzTest.java | 28 ++++---- .../iceberg/IcebergCatalogAdapterTest.java | 63 ----------------- 26 files changed, 232 insertions(+), 188 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 7d7a26c486..5763060780 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1600,7 +1600,8 @@ private void revokeGrantRecord( PolarisEntityType entityType, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { // get meta store session we should be using BasePersistence ms = callCtx.getMetaStore(); @@ -1642,7 +1643,8 @@ private void revokeGrantRecord( callCtx.getRealmConfig(), allowListOperation, allowedReadLocations, - allowedWriteLocations); + allowedWriteLocations, + refreshCredentialsEndpoint); return new ScopedCredentialsResult(accessConfig); } catch (Exception ex) { return new ScopedCredentialsResult( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 2671ad98b9..ab1de5339b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -327,7 +327,8 @@ public ScopedCredentialsResult getSubscopedCredsForEntity( PolarisEntityType entityType, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { return delegate.getSubscopedCredsForEntity( callCtx, catalogId, @@ -335,7 +336,8 @@ public ScopedCredentialsResult getSubscopedCredsForEntity( entityType, allowListOperation, allowedReadLocations, - allowedWriteLocations); + allowedWriteLocations, + refreshCredentialsEndpoint); } @Override diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index c187a6375a..5c58933730 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -2022,7 +2022,8 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( PolarisEntityType entityType, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { // get meta store session we should be using TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); @@ -2055,7 +2056,8 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( callCtx.getRealmConfig(), allowListOperation, allowedReadLocations, - allowedWriteLocations); + allowedWriteLocations, + refreshCredentialsEndpoint); return new ScopedCredentialsResult(accessConfig); } catch (Exception ex) { return new ScopedCredentialsResult( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java index 04022d233c..53f2e93c05 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java @@ -47,5 +47,6 @@ ScopedCredentialsResult getSubscopedCredsForEntity( PolarisEntityType entityType, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations); + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java index c989820913..81b9cfe4bf 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java @@ -61,7 +61,8 @@ public abstract AccessConfig getSubscopedCreds( @Nonnull RealmConfig realmConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations); + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint); /** * Validate access for the provided operation actions and locations. diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java index 33526d2e29..80c63c347a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java @@ -18,6 +18,8 @@ */ package org.apache.polaris.core.storage; +import org.apache.iceberg.aws.AwsClientProperties; + /** * A subset of Iceberg catalog properties recognized by Polaris. * @@ -39,6 +41,18 @@ public enum StorageAccessProperty { Boolean.class, "s3.path-style-access", "whether to use S3 path style access", false), CLIENT_REGION( String.class, "client.region", "region to configure client for making requests to AWS"), + AWS_REFRESH_CREDENTIALS_ENABLED( + Boolean.class, + AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, + "whether to enable automatic refresh of credentials", + true, + false), + AWS_REFRESH_CREDENTIALS_ENDPOINT( + String.class, + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, + "the endpoint to use for refreshing credentials", + true, + false), GCS_ACCESS_TOKEN(String.class, "gcs.oauth2.token", "the gcs scoped access token"), GCS_ACCESS_TOKEN_EXPIRES_AT( @@ -52,6 +66,18 @@ public enum StorageAccessProperty { // it expects for SAS AZURE_ACCESS_TOKEN(String.class, "", "the azure scoped access token"), AZURE_SAS_TOKEN(String.class, "adls.sas-token.", "an azure shared access signature token"), + AZURE_REFRESH_CREDENTIALS_ENABLED( + Boolean.class, + "adls.refresh-credentials-enabled", + "whether to enable automatic refresh of credentials", + true, + false), + AZURE_REFRESH_CREDENTIALS_ENDPOINT( + String.class, + "adls.refresh-credentials-endpoint", + "the endpoint to use for refreshing credentials", + true, + false), EXPIRATION_TIME( Long.class, "expiration-time", diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java index 616fb1f4d5..333c667837 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java @@ -74,7 +74,8 @@ public AccessConfig getSubscopedCreds( @Nonnull RealmConfig realmConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { int storageCredentialDurationSeconds = realmConfig.getConfig(STORAGE_CREDENTIAL_DURATION_SECONDS); AwsStorageConfigurationInfo storageConfig = config(); @@ -120,6 +121,12 @@ public AccessConfig getSubscopedCreds( accessConfig.put(StorageAccessProperty.CLIENT_REGION, region); } + if (refreshCredentialsEndpoint != null) { + accessConfig.put(StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENABLED, "true"); + accessConfig.put( + StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENDPOINT, refreshCredentialsEndpoint); + } + URI endpointUri = storageConfig.getEndpointUri(); if (endpointUri != null) { accessConfig.put(StorageAccessProperty.AWS_ENDPOINT, endpointUri.toString()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java index 50dd8c4143..562cd79721 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java @@ -76,7 +76,8 @@ public AccessConfig getSubscopedCreds( @Nonnull RealmConfig realmConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { String loc = !allowedWriteLocations.isEmpty() ? allowedWriteLocations.stream().findAny().orElse(null) @@ -169,15 +170,25 @@ public AccessConfig getSubscopedCreds( String.format("Endpoint %s not supported", location.getEndpoint())); } - return toAccessConfig(sasToken, storageDnsName, sanitizedEndTime.toInstant()); + return toAccessConfig( + sasToken, storageDnsName, sanitizedEndTime.toInstant(), refreshCredentialsEndpoint); } @VisibleForTesting - static AccessConfig toAccessConfig(String sasToken, String storageDnsName, Instant expiresAt) { + static AccessConfig toAccessConfig( + String sasToken, + String storageDnsName, + Instant expiresAt, + String refreshCredentialsEndpoint) { AccessConfig.Builder accessConfig = AccessConfig.builder(); handleAzureCredential(accessConfig, sasToken, storageDnsName); accessConfig.put( StorageAccessProperty.EXPIRATION_TIME, String.valueOf(expiresAt.toEpochMilli())); + if (refreshCredentialsEndpoint != null) { + accessConfig.put( + StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT, refreshCredentialsEndpoint); + accessConfig.put(StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENABLED, "true"); + } return accessConfig.build(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java index d8d88edc64..916a267810 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java @@ -105,7 +105,8 @@ public AccessConfig getOrGenerateSubScopeCreds( @Nonnull PolarisEntity polarisEntity, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { if (!isTypeSupported(polarisEntity.getType())) { callCtx .getDiagServices() @@ -130,7 +131,8 @@ public AccessConfig getOrGenerateSubScopeCreds( polarisEntity.getType(), k.allowedListAction(), k.allowedReadLocations(), - k.allowedWriteLocations()); + k.allowedWriteLocations(), + refreshCredentialsEndpoint); if (scopedCredentialsResult.isSuccess()) { long maxCacheDurationMs = maxCacheDurationMs(callCtx.getRealmConfig()); return new StorageCredentialCacheEntry( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java index 0120df2b11..60785ab1a4 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java @@ -75,7 +75,8 @@ public AccessConfig getSubscopedCreds( @Nonnull RealmConfig realmConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { try { sourceCredentials.refresh(); } catch (IOException e) { diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java index fa77778144..74c7229fcf 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java @@ -197,7 +197,8 @@ public AccessConfig getSubscopedCreds( @Nonnull RealmConfig realmConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { return null; } } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java index 89b60dba57..f1f3d99569 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java @@ -32,12 +32,12 @@ public class AzureCredentialsStorageIntegrationTest { public void testAzureCredentialFormatting() { Instant expiresAt = Instant.ofEpochMilli(Long.MAX_VALUE); - AccessConfig noSuffixResult = toAccessConfig("sasToken", "some_account", expiresAt); + AccessConfig noSuffixResult = toAccessConfig("sasToken", "some_account", expiresAt, null); Assertions.assertThat(noSuffixResult.credentials()).hasSize(2); Assertions.assertThat(noSuffixResult.credentials()).containsKey("adls.sas-token.some_account"); AccessConfig adlsSuffixResult = - toAccessConfig("sasToken", "some_account." + AzureLocation.ADLS_ENDPOINT, expiresAt); + toAccessConfig("sasToken", "some_account." + AzureLocation.ADLS_ENDPOINT, expiresAt, null); Assertions.assertThat(adlsSuffixResult.credentials()).hasSize(3); Assertions.assertThat(adlsSuffixResult.credentials()) .containsKey("adls.sas-token.some_account"); @@ -45,7 +45,7 @@ public void testAzureCredentialFormatting() { .containsKey("adls.sas-token.some_account." + AzureLocation.ADLS_ENDPOINT); AccessConfig blobSuffixResult = - toAccessConfig("sasToken", "some_account." + AzureLocation.BLOB_ENDPOINT, expiresAt); + toAccessConfig("sasToken", "some_account." + AzureLocation.BLOB_ENDPOINT, expiresAt, null); Assertions.assertThat(blobSuffixResult.credentials()).hasSize(3); Assertions.assertThat(blobSuffixResult.credentials()) .containsKey("adls.sas-token.some_account"); diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java index a8e97133bb..d68a0f5cc6 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java @@ -89,7 +89,8 @@ public void testBadResult() { Mockito.any(), Mockito.anyBoolean(), Mockito.anySet(), - Mockito.anySet())) + Mockito.anySet(), + Mockito.any())) .thenReturn(badResult); PolarisEntity polarisEntity = new PolarisEntity( @@ -103,7 +104,8 @@ public void testBadResult() { polarisEntity, true, Set.of("s3://bucket1/path"), - Set.of("s3://bucket3/path"))) + Set.of("s3://bucket3/path"), + null)) .isInstanceOf(UnprocessableEntityException.class) .hasMessage("Failed to get subscoped credentials: extra_error_info"); } @@ -121,7 +123,8 @@ public void testCacheHit() { Mockito.any(), Mockito.anyBoolean(), Mockito.anySet(), - Mockito.anySet())) + Mockito.anySet(), + Mockito.any())) .thenReturn(mockedScopedCreds.get(0)) .thenReturn(mockedScopedCreds.get(1)) .thenReturn(mockedScopedCreds.get(1)); @@ -137,7 +140,8 @@ public void testCacheHit() { polarisEntity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket3/path", "s3://bucket4/path")); + Set.of("s3://bucket3/path", "s3://bucket4/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(1); // subscope for the same entity and same allowed locations, will hit the cache @@ -147,7 +151,8 @@ public void testCacheHit() { polarisEntity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket3/path", "s3://bucket4/path")); + Set.of("s3://bucket3/path", "s3://bucket4/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(1); } @@ -164,7 +169,8 @@ public void testCacheEvict() throws InterruptedException { Mockito.any(), Mockito.anyBoolean(), Mockito.anySet(), - Mockito.anySet())) + Mockito.anySet(), + Mockito.any())) .thenReturn(mockedScopedCreds.get(0)) .thenReturn(mockedScopedCreds.get(1)) .thenReturn(mockedScopedCreds.get(2)); @@ -187,7 +193,8 @@ public void testCacheEvict() throws InterruptedException { polarisEntity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + null); Assertions.assertThat(storageCredentialCache.getIfPresent(cacheKey)).isNull(); storageCredentialCache.getOrGenerateSubScopeCreds( @@ -196,7 +203,8 @@ public void testCacheEvict() throws InterruptedException { polarisEntity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + null); Assertions.assertThat(storageCredentialCache.getIfPresent(cacheKey)).isNull(); storageCredentialCache.getOrGenerateSubScopeCreds( @@ -205,7 +213,8 @@ public void testCacheEvict() throws InterruptedException { polarisEntity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + null); Assertions.assertThat(storageCredentialCache.getIfPresent(cacheKey)).isNull(); } @@ -222,7 +231,8 @@ public void testCacheGenerateNewEntries() { Mockito.any(), Mockito.anyBoolean(), Mockito.anySet(), - Mockito.anySet())) + Mockito.anySet(), + Mockito.any())) .thenReturn(mockedScopedCreds.get(0)) .thenReturn(mockedScopedCreds.get(1)) .thenReturn(mockedScopedCreds.get(2)); @@ -236,7 +246,8 @@ public void testCacheGenerateNewEntries() { entity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // update the entity's storage config, since StorageConfig changed, cache will generate new @@ -253,7 +264,8 @@ public void testCacheGenerateNewEntries() { PolarisEntity.of(updateEntity), /* allowedListAction= */ true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // allowedListAction changed to different value FALSE, will generate new entry @@ -264,7 +276,8 @@ public void testCacheGenerateNewEntries() { entity, /* allowedListAction= */ false, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // different allowedWriteLocations, will generate new entry @@ -275,7 +288,8 @@ public void testCacheGenerateNewEntries() { entity, /* allowedListAction= */ false, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://differentbucket/path")); + Set.of("s3://differentbucket/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // different allowedReadLocations, will generate new try @@ -291,7 +305,8 @@ public void testCacheGenerateNewEntries() { PolarisEntity.of(updateEntity), /* allowedListAction= */ false, Set.of("s3://differentbucket/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } } @@ -310,7 +325,8 @@ public void testCacheNotAffectedBy() { Mockito.any(), Mockito.anyBoolean(), Mockito.anySet(), - Mockito.anySet())) + Mockito.anySet(), + Mockito.any())) .thenReturn(mockedScopedCreds.get(0)) .thenReturn(mockedScopedCreds.get(1)) .thenReturn(mockedScopedCreds.get(2)); @@ -322,7 +338,8 @@ public void testCacheNotAffectedBy() { entity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket3/path", "s3://bucket4/path")); + Set.of("s3://bucket3/path", "s3://bucket4/path"), + null); } Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); @@ -334,7 +351,8 @@ public void testCacheNotAffectedBy() { new PolarisEntity(new PolarisBaseEntity.Builder(entity).id(1234).build()), true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket3/path", "s3://bucket4/path")); + Set.of("s3://bucket3/path", "s3://bucket4/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } @@ -346,7 +364,8 @@ public void testCacheNotAffectedBy() { new PolarisEntity(new PolarisBaseEntity.Builder(entity).entityVersion(5).build()), true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket3/path", "s3://bucket4/path")); + Set.of("s3://bucket3/path", "s3://bucket4/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } // order of the allowedReadLocations does not affect the cache @@ -357,7 +376,8 @@ public void testCacheNotAffectedBy() { new PolarisEntity(new PolarisBaseEntity.Builder(entity).entityVersion(5).build()), true, Set.of("s3://bucket2/path", "s3://bucket1/path"), - Set.of("s3://bucket3/path", "s3://bucket4/path")); + Set.of("s3://bucket3/path", "s3://bucket4/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } @@ -369,7 +389,8 @@ public void testCacheNotAffectedBy() { new PolarisEntity(new PolarisBaseEntity.Builder(entity).entityVersion(5).build()), true, Set.of("s3://bucket2/path", "s3://bucket1/path"), - Set.of("s3://bucket4/path", "s3://bucket3/path")); + Set.of("s3://bucket4/path", "s3://bucket3/path"), + null); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } } @@ -451,7 +472,8 @@ public void testExtraProperties() { Mockito.any(), Mockito.anyBoolean(), Mockito.anySet(), - Mockito.anySet())) + Mockito.anySet(), + Mockito.any())) .thenReturn(properties); List entityList = getPolarisEntities(); @@ -462,7 +484,8 @@ public void testExtraProperties() { entityList.get(0), true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket3/path", "s3://bucket4/path")); + Set.of("s3://bucket3/path", "s3://bucket4/path"), + null); Assertions.assertThat(config.credentials()) .containsExactly(Map.entry("s3.secret-access-key", "super-secret-123")); Assertions.assertThat(config.extraProperties()) diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java index 10ba4b9088..0fd732c0e4 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java @@ -95,12 +95,18 @@ public void testGetSubscopedCreds(String scheme) { EMPTY_REALM_CONFIG, true, Set.of(warehouseDir + "/namespace/table"), - Set.of(warehouseDir + "/namespace/table")); + Set.of(warehouseDir + "/namespace/table"), + "/namespace/table/credentials"); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") .containsEntry(StorageAccessProperty.AWS_KEY_ID.getPropertyName(), "accessKey") .containsEntry(StorageAccessProperty.AWS_SECRET_KEY.getPropertyName(), "secretKey") + .containsEntry( + StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENABLED.getPropertyName(), "true") + .containsEntry( + StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENDPOINT.getPropertyName(), + "/namespace/table/credentials") .containsEntry( StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS.getPropertyName(), String.valueOf(EXPIRE_TIME.toEpochMilli())); @@ -242,7 +248,8 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { EMPTY_REALM_CONFIG, true, Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), - Set.of(s3Path(bucket, firstPath)))) + Set.of(s3Path(bucket, firstPath)), + null)) .isInstanceOf(IllegalArgumentException.class); break; case AWS_PARTITION: @@ -260,7 +267,8 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { EMPTY_REALM_CONFIG, true, Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), - Set.of(s3Path(bucket, firstPath))); + Set.of(s3Path(bucket, firstPath)), + null); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -360,7 +368,8 @@ public void testGetSubscopedCredsInlinePolicyWithoutList() { EMPTY_REALM_CONFIG, false, /* allowList = false*/ Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), - Set.of(s3Path(bucket, firstPath))); + Set.of(s3Path(bucket, firstPath)), + null); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -454,7 +463,8 @@ public void testGetSubscopedCredsInlinePolicyWithoutWrites() { EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), - Set.of()); + Set.of(), + null); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -516,7 +526,8 @@ public void testGetSubscopedCredsInlinePolicyWithEmptyReadAndWrite() { .region("us-east-2") .build(), stsClient) - .getSubscopedCreds(EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of()); + .getSubscopedCreds( + EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of(), null); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -554,7 +565,11 @@ public void testClientRegion(String awsPartition) { .build(), stsClient) .getSubscopedCreds( - EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of())) + EMPTY_REALM_CONFIG, + true, /* allowList = true */ + Set.of(), + Set.of(), + null)) .isInstanceOf(IllegalArgumentException.class); break; case AWS_PARTITION: @@ -569,7 +584,7 @@ public void testClientRegion(String awsPartition) { .build(), stsClient) .getSubscopedCreds( - EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of()); + EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of(), null); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.CLIENT_REGION.getPropertyName(), clientRegion); @@ -604,7 +619,7 @@ public void testNoClientRegion(String awsPartition) { .build(), stsClient) .getSubscopedCreds( - EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of()); + EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of(), null); assertThat(accessConfig.credentials()) .isNotEmpty() .doesNotContainKey(StorageAccessProperty.CLIENT_REGION.getPropertyName()); @@ -621,7 +636,11 @@ public void testNoClientRegion(String awsPartition) { .build(), stsClient) .getSubscopedCreds( - EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of())) + EMPTY_REALM_CONFIG, + true, /* allowList = true */ + Set.of(), + Set.of(), + null)) .isInstanceOf(IllegalArgumentException.class); break; default: diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java index 768f4c330c..d79d6be393 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java @@ -352,7 +352,8 @@ private AccessConfig subscopedCredsForOperations( EMPTY_REALM_CONFIG, allowListAction, new HashSet<>(allowedReadLoc), - new HashSet<>(allowedWriteLoc)); + new HashSet<>(allowedWriteLoc), + null); } private BlobContainerClient createContainerClient( diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java index da890627ea..b003abc180 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java @@ -170,7 +170,8 @@ private AccessConfig subscopedCredsForOperations( EMPTY_REALM_CONFIG, allowListAction, new HashSet<>(allowedReadLoc), - new HashSet<>(allowedWriteLoc)); + new HashSet<>(allowedWriteLoc), + null); } @Test diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 4c0f7af2e0..9e311fdbd3 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -830,7 +830,8 @@ public boolean sendNotification( public AccessConfig getAccessConfig( TableIdentifier tableIdentifier, TableMetadata tableMetadata, - Set storageActions) { + Set storageActions, + String refreshCredentialsEndpoint) { Optional storageInfo = findStorageInfo(tableIdentifier); if (storageInfo.isEmpty()) { LOGGER @@ -846,7 +847,8 @@ public AccessConfig getAccessConfig( tableIdentifier, StorageUtil.getLocationsAllowedToBeAccessed(tableMetadata), storageActions, - storageInfo.get()); + storageInfo.get(), + refreshCredentialsEndpoint); } private String buildPrefixedLocation(TableIdentifier tableIdentifier) { diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index cff8e69be4..517bccb510 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -35,13 +35,11 @@ import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; import java.util.EnumSet; -import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Function; import org.apache.iceberg.MetadataUpdate; -import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.BadRequestException; @@ -63,7 +61,6 @@ import org.apache.iceberg.rest.responses.ConfigResponse; import org.apache.iceberg.rest.responses.ImmutableLoadCredentialsResponse; import org.apache.iceberg.rest.responses.LoadTableResponse; -import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.auth.PolarisAuthorizer; import org.apache.polaris.core.auth.PolarisPrincipal; import org.apache.polaris.core.catalog.ExternalCatalogFactory; @@ -363,12 +360,18 @@ public Response createTable( securityContext, prefix, catalog -> { + String refreshCredentialsEndpoint = + delegationModes.contains(VENDED_CREDENTIALS) + ? new PolarisResourcePaths(prefix) + .credentialsPath(TableIdentifier.of(namespace, createTableRequest.name())) + : null; if (createTableRequest.stageCreate()) { if (delegationModes.isEmpty()) { return Response.ok(catalog.createTableStaged(ns, createTableRequest)).build(); } else { return Response.ok( - catalog.createTableStagedWithWriteDelegation(ns, createTableRequest)) + catalog.createTableStagedWithWriteDelegation( + ns, createTableRequest, refreshCredentialsEndpoint)) .build(); } } else if (delegationModes.isEmpty()) { @@ -378,7 +381,8 @@ public Response createTable( .build(); } else { LoadTableResponse response = - catalog.createTableDirectWithWriteDelegation(ns, createTableRequest); + catalog.createTableDirectWithWriteDelegation( + ns, createTableRequest, refreshCredentialsEndpoint); return tryInsertETagHeader( Response.ok(response), response, namespace, createTableRequest.name()) .build(); @@ -434,49 +438,21 @@ public Response loadTable( .loadTableIfStale(tableIdentifier, ifNoneMatch, snapshots) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } else { - LoadTableResponse originalResponse = + String refreshCredentialsEndpoint = + (delegationModes.contains(VENDED_CREDENTIALS)) + ? new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier) + : null; + response = catalog - .loadTableWithAccessDelegationIfStale(tableIdentifier, ifNoneMatch, snapshots) + .loadTableWithAccessDelegationIfStale( + tableIdentifier, ifNoneMatch, snapshots, refreshCredentialsEndpoint) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); - - if (delegationModes.contains(VENDED_CREDENTIALS)) { - response = - injectRefreshVendedCredentialProperties( - originalResponse, - new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier)); - } else { - response = originalResponse; - } } return tryInsertETagHeader(Response.ok(response), response, namespace, table).build(); }); } - private LoadTableResponse injectRefreshVendedCredentialProperties( - LoadTableResponse originalResponse, String credentialsEndpoint) { - // Only enable credential refresh for currently supported credential types - if (originalResponse.credentials().stream() - .anyMatch( - credential -> - credential - .prefix() - .toLowerCase(Locale.ROOT) - .startsWith( - StorageConfigInfo.StorageTypeEnum.S3.name().toLowerCase(Locale.ROOT)))) { - LoadTableResponse.Builder loadResponseBuilder = - LoadTableResponse.builder().withTableMetadata(originalResponse.tableMetadata()); - loadResponseBuilder.addAllConfig(originalResponse.config()); - loadResponseBuilder.addAllCredentials(originalResponse.credentials()); - loadResponseBuilder.addConfig( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, credentialsEndpoint); - loadResponseBuilder.addConfig(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "true"); - return loadResponseBuilder.build(); - } else { - return originalResponse; - } - } - @Override public Response tableExists( String prefix, @@ -636,7 +612,10 @@ public Response loadCredentials( prefix, catalog -> { LoadTableResponse loadTableResponse = - catalog.loadTableWithAccessDelegation(tableIdentifier, "all"); + catalog.loadTableWithAccessDelegation( + tableIdentifier, + "all", + new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier)); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index 17cdd7af36..f2beb2493b 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -393,7 +393,7 @@ public LoadTableResponse createTableDirect(Namespace namespace, CreateTableReque * @return ETagged {@link LoadTableResponse} to uniquely identify the table metadata */ public LoadTableResponse createTableDirectWithWriteDelegation( - Namespace namespace, CreateTableRequest request) { + Namespace namespace, CreateTableRequest request, String refreshCredentialsEndpoint) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_DIRECT_WITH_WRITE_DELEGATION; authorizeCreateTableLikeUnderNamespaceOperationOrThrow( @@ -432,7 +432,8 @@ public LoadTableResponse createTableDirectWithWriteDelegation( PolarisStorageActions.READ, PolarisStorageActions.WRITE, PolarisStorageActions.LIST), - SNAPSHOTS_ALL) + SNAPSHOTS_ALL, + refreshCredentialsEndpoint) .build(); } else if (table instanceof BaseMetadataTable) { // metadata tables are loaded on the client side, return NoSuchTableException for now @@ -500,7 +501,7 @@ public LoadTableResponse createTableStaged(Namespace namespace, CreateTableReque } public LoadTableResponse createTableStagedWithWriteDelegation( - Namespace namespace, CreateTableRequest request) { + Namespace namespace, CreateTableRequest request, String refreshCredentialsEndpoint) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_STAGED_WITH_WRITE_DELEGATION; authorizeCreateTableLikeUnderNamespaceOperationOrThrow( @@ -514,7 +515,11 @@ public LoadTableResponse createTableStagedWithWriteDelegation( TableMetadata metadata = stageTableCreateHelper(namespace, request); return buildLoadTableResponseWithDelegationCredentials( - ident, metadata, Set.of(PolarisStorageActions.ALL), SNAPSHOTS_ALL) + ident, + metadata, + Set.of(PolarisStorageActions.ALL), + SNAPSHOTS_ALL, + refreshCredentialsEndpoint) .build(); } @@ -623,8 +628,10 @@ public Optional loadTableIfStale( } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, String snapshots) { - return loadTableWithAccessDelegationIfStale(tableIdentifier, null, snapshots).get(); + TableIdentifier tableIdentifier, String snapshots, String refreshCredentialsEndpoint) { + return loadTableWithAccessDelegationIfStale( + tableIdentifier, null, snapshots, refreshCredentialsEndpoint) + .get(); } /** @@ -638,7 +645,10 @@ public LoadTableResponse loadTableWithAccessDelegation( * load table response, otherwise */ public Optional loadTableWithAccessDelegationIfStale( - TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots) { + TableIdentifier tableIdentifier, + IfNoneMatch ifNoneMatch, + String snapshots, + String refreshCredentialsEndpoint) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -708,7 +718,11 @@ public Optional loadTableWithAccessDelegationIfStale( TableMetadata tableMetadata = baseTable.operations().current(); return Optional.of( buildLoadTableResponseWithDelegationCredentials( - tableIdentifier, tableMetadata, actionsRequested, snapshots) + tableIdentifier, + tableMetadata, + actionsRequested, + snapshots, + refreshCredentialsEndpoint) .build()); } else if (table instanceof BaseMetadataTable) { // metadata tables are loaded on the client side, return NoSuchTableException for now @@ -722,7 +736,8 @@ private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredential TableIdentifier tableIdentifier, TableMetadata tableMetadata, Set actions, - String snapshots) { + String snapshots, + String refreshCredentialsEndpoint) { LoadTableResponse.Builder responseBuilder = LoadTableResponse.builder().withTableMetadata(tableMetadata); if (baseCatalog instanceof SupportsCredentialDelegation credentialDelegation) { @@ -732,7 +747,8 @@ private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredential .addKeyValue("tableLocation", tableMetadata.location()) .log("Fetching client credentials for table"); AccessConfig accessConfig = - credentialDelegation.getAccessConfig(tableIdentifier, tableMetadata, actions); + credentialDelegation.getAccessConfig( + tableIdentifier, tableMetadata, actions, refreshCredentialsEndpoint); Map credentialConfig = accessConfig.credentials(); responseBuilder.addAllConfig(credentialConfig); responseBuilder.addAllConfig(accessConfig.extraProperties()); diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java index 21ec380eb0..354e4e7d1b 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java @@ -35,5 +35,6 @@ public interface SupportsCredentialDelegation { AccessConfig getAccessConfig( TableIdentifier tableIdentifier, TableMetadata tableMetadata, - Set storageActions); + Set storageActions, + String refreshCredentialsEndpoint); } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index d2c73e2684..a62824ce3a 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -91,7 +91,8 @@ public FileIO loadFileIO( identifier, tableLocations, storageActions, - storageInfo)); + storageInfo, + null)); // Update the FileIO with the subscoped credentials // Update with properties in case there are table-level overrides the credentials should diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java index c5ef12d784..f11ba26208 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java @@ -81,7 +81,8 @@ public static AccessConfig refreshAccessConfig( TableIdentifier tableIdentifier, Set tableLocations, Set storageActions, - PolarisEntity entity) { + PolarisEntity entity, + String refreshCredentialsEndpoint) { boolean skipCredentialSubscopingIndirection = callContext @@ -111,7 +112,8 @@ public static AccessConfig refreshAccessConfig( entity, allowList, tableLocations, - writeLocations); + writeLocations, + refreshCredentialsEndpoint); LOGGER .atDebug() .addKeyValue("tableIdentifier", tableIdentifier) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java b/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java index e07bdd0823..1971b11ccb 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java @@ -113,7 +113,8 @@ public AccessConfig getSubscopedCreds( @Nonnull RealmConfig realmConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, - @Nonnull Set allowedWriteLocations) { + @Nonnull Set allowedWriteLocations, + String refreshCredentialsEndpoint) { return AccessConfig.builder().build(); } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java index 696bca4328..d993051fca 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java @@ -1828,7 +1828,8 @@ public void testDropTableWithPurge() { taskEntity.getType(), true, Set.of(tableMetadata.location()), - Set.of(tableMetadata.location())) + Set.of(tableMetadata.location()), + null) .getAccessConfig() .credentials(); Assertions.assertThat(credentials) diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java index a3ab18e3f4..22f5aa9c43 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java @@ -616,7 +616,8 @@ public void testCreateTableDirectWithWriteDelegationAllSufficientPrivileges() { Set.of(PolarisPrivilege.CATALOG_MANAGE_CONTENT)), () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) - .createTableDirectWithWriteDelegation(NS2, createDirectWithWriteDelegationRequest); + .createTableDirectWithWriteDelegation( + NS2, createDirectWithWriteDelegationRequest, null); }, () -> { newWrapper(Set.of(PRINCIPAL_ROLE2)).dropTableWithPurge(newtable); @@ -646,7 +647,8 @@ public void testCreateTableDirectWithWriteDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_LIST), () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) - .createTableDirectWithWriteDelegation(NS2, createDirectWithWriteDelegationRequest); + .createTableDirectWithWriteDelegation( + NS2, createDirectWithWriteDelegationRequest, null); }); } @@ -719,7 +721,8 @@ public void testCreateTableStagedWithWriteDelegationAllSufficientPrivileges() { Set.of(PolarisPrivilege.CATALOG_MANAGE_CONTENT)), () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) - .createTableStagedWithWriteDelegation(NS2, createStagedWithWriteDelegationRequest); + .createTableStagedWithWriteDelegation( + NS2, createStagedWithWriteDelegationRequest, null); }, // createTableStagedWithWriteDelegation doesn't actually commit any metadata null, @@ -748,7 +751,8 @@ public void testCreateTableStagedWithWriteDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_LIST), () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) - .createTableStagedWithWriteDelegation(NS2, createStagedWithWriteDelegationRequest); + .createTableStagedWithWriteDelegation( + NS2, createStagedWithWriteDelegationRequest, null); }); } @@ -892,7 +896,7 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all"), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null), null /* cleanupAction */); } @@ -908,7 +912,7 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all")); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null)); } @Test @@ -921,7 +925,7 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all"), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null), null /* cleanupAction */); } @@ -937,7 +941,7 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all")); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null)); } @Test @@ -950,7 +954,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all"), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null), null /* cleanupAction */); } @@ -969,7 +973,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleInsufficientPermissions( () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all")); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null)); } @Test @@ -985,7 +989,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleSufficientPrivileges() () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all"), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null), null /* cleanupAction */); } @@ -1004,7 +1008,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleInsufficientPermissions () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all")); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null)); } @Test diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java index 7204911e41..f8f948a66d 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapterTest.java @@ -19,23 +19,17 @@ package org.apache.polaris.service.catalog.iceberg; -import static org.assertj.core.api.Assertions.assertThat; - import java.io.IOException; import java.lang.reflect.Field; import java.util.List; import java.util.Map; import java.util.stream.Stream; import org.apache.iceberg.Schema; -import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.inmemory.InMemoryCatalog; -import org.apache.iceberg.rest.requests.CreateTableRequest; import org.apache.iceberg.rest.responses.ListNamespacesResponse; import org.apache.iceberg.rest.responses.ListTablesResponse; -import org.apache.iceberg.rest.responses.LoadTableResponse; -import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AuthenticationParameters; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.BearerAuthenticationParameters; @@ -49,7 +43,6 @@ import org.assertj.core.api.Assertions; import org.assertj.core.util.Strings; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -249,60 +242,4 @@ private static Stream paginationTestCases() { Arguments.of("5", 5), Arguments.of("5", 10)); } - - @Test - void testLoadTableReturnsCredentialsRefreshEndpoint() throws IOException { - try (InMemoryCatalog inMemoryCatalog = new InMemoryCatalog()) { - // Initialize and replace the default handler with one backed by in-memory catalog - inMemoryCatalog.initialize("inMemory", Map.of()); - mockCatalogAdapter(inMemoryCatalog); - - // Create a namespace and table - String namespace = "test_ns"; - String tableName = "test_table"; - inMemoryCatalog.createNamespace(Namespace.of(namespace)); - - Schema schema = - new Schema( - Types.NestedField.required(1, "id", Types.LongType.get()), - Types.NestedField.optional(2, "name", Types.StringType.get())); - - CreateTableRequest createTableRequest = - CreateTableRequest.builder().withName(tableName).withSchema(schema).build(); - - // Create the table first - catalogAdapter.createTable( - FEDERATED_CATALOG_NAME, - namespace, - createTableRequest, - "vended-credentials", - testServices.realmContext(), - testServices.securityContext()); - - // Load the table with vended credentials access delegation mode - LoadTableResponse response = - (LoadTableResponse) - catalogAdapter - .loadTable( - FEDERATED_CATALOG_NAME, - namespace, - tableName, - "vended-credentials", - null, - null, - testServices.realmContext(), - testServices.securityContext()) - .getEntity(); - - // Verify that the response contains the credentials refresh endpoint configuration - assertThat(response.config()).containsKey(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT); - - String expectedEndpoint = - String.format( - "v1/%s/namespaces/%s/tables/%s/credentials", - FEDERATED_CATALOG_NAME, namespace, tableName); - assertThat(response.config().get(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT)) - .isEqualTo(expectedEndpoint); - } - } } From 32ca5c2ba08763f733c179ed981501474c32d95f Mon Sep 17 00:00:00 2001 From: Jason Date: Thu, 21 Aug 2025 11:01:32 +0300 Subject: [PATCH 06/23] change stirng to optional --- .../AtomicOperationMetaStoreManager.java | 2 +- .../TransactionWorkspaceMetaStoreManager.java | 2 +- .../TransactionalMetaStoreManagerImpl.java | 2 +- .../core/storage/PolarisCredentialVendor.java | 3 +- .../storage/PolarisStorageIntegration.java | 3 +- .../aws/AwsCredentialsStorageIntegration.java | 12 +++---- .../AzureCredentialsStorageIntegration.java | 15 ++++---- .../storage/cache/StorageCredentialCache.java | 2 +- .../gcp/GcpCredentialsStorageIntegration.java | 3 +- .../InMemoryStorageIntegrationTest.java | 3 +- ...zureCredentialsStorageIntegrationTest.java | 10 ++++-- .../cache/StorageCredentialCacheTest.java | 35 ++++++++++--------- .../AwsCredentialsStorageIntegrationTest.java | 31 +++++++++++----- ...AzureCredentialStorageIntegrationTest.java | 3 +- .../GcpCredentialsStorageIntegrationTest.java | 3 +- .../catalog/iceberg/IcebergCatalog.java | 2 +- .../iceberg/IcebergCatalogAdapter.java | 27 ++++++++------ .../iceberg/IcebergCatalogHandler.java | 16 ++++++--- .../iceberg/SupportsCredentialDelegation.java | 3 +- .../catalog/io/DefaultFileIOFactory.java | 2 +- .../service/catalog/io/FileIOUtil.java | 2 +- ...PolarisStorageIntegrationProviderImpl.java | 2 +- .../catalog/AbstractIcebergCatalogTest.java | 3 +- .../IcebergCatalogHandlerAuthzTest.java | 25 ++++++------- 24 files changed, 126 insertions(+), 85 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 5763060780..e5942a8291 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1601,7 +1601,7 @@ private void revokeGrantRecord( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { // get meta store session we should be using BasePersistence ms = callCtx.getMetaStore(); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index ab1de5339b..3d9f3c0523 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -328,7 +328,7 @@ public ScopedCredentialsResult getSubscopedCredsForEntity( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { return delegate.getSubscopedCredsForEntity( callCtx, catalogId, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 5c58933730..3fa70af71b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -2023,7 +2023,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { // get meta store session we should be using TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java index 53f2e93c05..a0060d43ac 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java @@ -19,6 +19,7 @@ package org.apache.polaris.core.storage; import jakarta.annotation.Nonnull; +import java.util.Optional; import java.util.Set; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.entity.PolarisEntityType; @@ -48,5 +49,5 @@ ScopedCredentialsResult getSubscopedCredsForEntity( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint); + Optional refreshCredentialsEndpoint); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java index 81b9cfe4bf..28df510ab2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java @@ -20,6 +20,7 @@ import jakarta.annotation.Nonnull; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.apache.polaris.core.config.RealmConfig; @@ -62,7 +63,7 @@ public abstract AccessConfig getSubscopedCreds( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint); + Optional refreshCredentialsEndpoint); /** * Validate access for the provided operation actions and locations. diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java index 333c667837..7c73b17c38 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java @@ -75,7 +75,7 @@ public AccessConfig getSubscopedCreds( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { int storageCredentialDurationSeconds = realmConfig.getConfig(STORAGE_CREDENTIAL_DURATION_SECONDS); AwsStorageConfigurationInfo storageConfig = config(); @@ -121,11 +121,11 @@ public AccessConfig getSubscopedCreds( accessConfig.put(StorageAccessProperty.CLIENT_REGION, region); } - if (refreshCredentialsEndpoint != null) { - accessConfig.put(StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENABLED, "true"); - accessConfig.put( - StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENDPOINT, refreshCredentialsEndpoint); - } + refreshCredentialsEndpoint.ifPresent( + endpoint -> { + accessConfig.put(StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENABLED, "true"); + accessConfig.put(StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENDPOINT, endpoint); + }); URI endpointUri = storageConfig.getEndpointUri(); if (endpointUri != null) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java index 562cd79721..9c9469b2c7 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java @@ -46,6 +46,7 @@ import java.time.temporal.ChronoUnit; import java.util.HashSet; import java.util.Objects; +import java.util.Optional; import java.util.Set; import org.apache.polaris.core.config.RealmConfig; import org.apache.polaris.core.storage.AccessConfig; @@ -77,7 +78,7 @@ public AccessConfig getSubscopedCreds( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { String loc = !allowedWriteLocations.isEmpty() ? allowedWriteLocations.stream().findAny().orElse(null) @@ -179,16 +180,16 @@ static AccessConfig toAccessConfig( String sasToken, String storageDnsName, Instant expiresAt, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { AccessConfig.Builder accessConfig = AccessConfig.builder(); handleAzureCredential(accessConfig, sasToken, storageDnsName); accessConfig.put( StorageAccessProperty.EXPIRATION_TIME, String.valueOf(expiresAt.toEpochMilli())); - if (refreshCredentialsEndpoint != null) { - accessConfig.put( - StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT, refreshCredentialsEndpoint); - accessConfig.put(StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENABLED, "true"); - } + refreshCredentialsEndpoint.ifPresent( + endpoint -> { + accessConfig.put(StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT, endpoint); + accessConfig.put(StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENABLED, "true"); + }); return accessConfig.build(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java index 916a267810..c26ca67cd1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java @@ -106,7 +106,7 @@ public AccessConfig getOrGenerateSubScopeCreds( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { if (!isTypeSupported(polarisEntity.getType())) { callCtx .getDiagServices() diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java index 60785ab1a4..27b1581c7a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Stream; import org.apache.polaris.core.config.RealmConfig; @@ -76,7 +77,7 @@ public AccessConfig getSubscopedCreds( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { try { sourceCredentials.refresh(); } catch (IOException e) { diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java index 74c7229fcf..9ba5271ab4 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java @@ -21,6 +21,7 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.config.RealmConfig; @@ -198,7 +199,7 @@ public AccessConfig getSubscopedCreds( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { return null; } } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java index f1f3d99569..6241be21ee 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java @@ -22,6 +22,7 @@ import static org.apache.polaris.core.storage.azure.AzureCredentialsStorageIntegration.toAccessConfig; import java.time.Instant; +import java.util.Optional; import org.apache.polaris.core.storage.AccessConfig; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @@ -32,12 +33,14 @@ public class AzureCredentialsStorageIntegrationTest { public void testAzureCredentialFormatting() { Instant expiresAt = Instant.ofEpochMilli(Long.MAX_VALUE); - AccessConfig noSuffixResult = toAccessConfig("sasToken", "some_account", expiresAt, null); + AccessConfig noSuffixResult = + toAccessConfig("sasToken", "some_account", expiresAt, Optional.empty()); Assertions.assertThat(noSuffixResult.credentials()).hasSize(2); Assertions.assertThat(noSuffixResult.credentials()).containsKey("adls.sas-token.some_account"); AccessConfig adlsSuffixResult = - toAccessConfig("sasToken", "some_account." + AzureLocation.ADLS_ENDPOINT, expiresAt, null); + toAccessConfig( + "sasToken", "some_account." + AzureLocation.ADLS_ENDPOINT, expiresAt, Optional.empty()); Assertions.assertThat(adlsSuffixResult.credentials()).hasSize(3); Assertions.assertThat(adlsSuffixResult.credentials()) .containsKey("adls.sas-token.some_account"); @@ -45,7 +48,8 @@ public void testAzureCredentialFormatting() { .containsKey("adls.sas-token.some_account." + AzureLocation.ADLS_ENDPOINT); AccessConfig blobSuffixResult = - toAccessConfig("sasToken", "some_account." + AzureLocation.BLOB_ENDPOINT, expiresAt, null); + toAccessConfig( + "sasToken", "some_account." + AzureLocation.BLOB_ENDPOINT, expiresAt, Optional.empty()); Assertions.assertThat(blobSuffixResult.credentials()).hasSize(3); Assertions.assertThat(blobSuffixResult.credentials()) .containsKey("adls.sas-token.some_account"); diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java index d68a0f5cc6..74e66cf878 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java @@ -25,6 +25,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.apache.iceberg.exceptions.UnprocessableEntityException; import org.apache.polaris.core.PolarisCallContext; @@ -105,7 +106,7 @@ public void testBadResult() { true, Set.of("s3://bucket1/path"), Set.of("s3://bucket3/path"), - null)) + Optional.empty())) .isInstanceOf(UnprocessableEntityException.class) .hasMessage("Failed to get subscoped credentials: extra_error_info"); } @@ -141,7 +142,7 @@ public void testCacheHit() { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket3/path", "s3://bucket4/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(1); // subscope for the same entity and same allowed locations, will hit the cache @@ -152,7 +153,7 @@ public void testCacheHit() { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket3/path", "s3://bucket4/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(1); } @@ -194,7 +195,7 @@ public void testCacheEvict() throws InterruptedException { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getIfPresent(cacheKey)).isNull(); storageCredentialCache.getOrGenerateSubScopeCreds( @@ -204,7 +205,7 @@ public void testCacheEvict() throws InterruptedException { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getIfPresent(cacheKey)).isNull(); storageCredentialCache.getOrGenerateSubScopeCreds( @@ -214,7 +215,7 @@ public void testCacheEvict() throws InterruptedException { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getIfPresent(cacheKey)).isNull(); } @@ -247,7 +248,7 @@ public void testCacheGenerateNewEntries() { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // update the entity's storage config, since StorageConfig changed, cache will generate new @@ -265,7 +266,7 @@ public void testCacheGenerateNewEntries() { /* allowedListAction= */ true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // allowedListAction changed to different value FALSE, will generate new entry @@ -277,7 +278,7 @@ public void testCacheGenerateNewEntries() { /* allowedListAction= */ false, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // different allowedWriteLocations, will generate new entry @@ -289,7 +290,7 @@ public void testCacheGenerateNewEntries() { /* allowedListAction= */ false, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://differentbucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } // different allowedReadLocations, will generate new try @@ -306,7 +307,7 @@ public void testCacheGenerateNewEntries() { /* allowedListAction= */ false, Set.of("s3://differentbucket/path", "s3://bucket2/path"), Set.of("s3://bucket/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(++cacheSize); } } @@ -339,7 +340,7 @@ public void testCacheNotAffectedBy() { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket3/path", "s3://bucket4/path"), - null); + Optional.empty()); } Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); @@ -352,7 +353,7 @@ public void testCacheNotAffectedBy() { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket3/path", "s3://bucket4/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } @@ -365,7 +366,7 @@ public void testCacheNotAffectedBy() { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket3/path", "s3://bucket4/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } // order of the allowedReadLocations does not affect the cache @@ -377,7 +378,7 @@ public void testCacheNotAffectedBy() { true, Set.of("s3://bucket2/path", "s3://bucket1/path"), Set.of("s3://bucket3/path", "s3://bucket4/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } @@ -390,7 +391,7 @@ public void testCacheNotAffectedBy() { true, Set.of("s3://bucket2/path", "s3://bucket1/path"), Set.of("s3://bucket4/path", "s3://bucket3/path"), - null); + Optional.empty()); Assertions.assertThat(storageCredentialCache.getEstimatedSize()).isEqualTo(entityList.size()); } } @@ -485,7 +486,7 @@ public void testExtraProperties() { true, Set.of("s3://bucket1/path", "s3://bucket2/path"), Set.of("s3://bucket3/path", "s3://bucket4/path"), - null); + Optional.empty()); Assertions.assertThat(config.credentials()) .containsExactly(Map.entry("s3.secret-access-key", "super-secret-123")); Assertions.assertThat(config.extraProperties()) diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java index 0fd732c0e4..44f3b86c5b 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java @@ -23,6 +23,7 @@ import jakarta.annotation.Nonnull; import java.time.Instant; import java.util.List; +import java.util.Optional; import java.util.Set; import org.apache.polaris.core.storage.AccessConfig; import org.apache.polaris.core.storage.BaseStorageIntegrationTest; @@ -96,7 +97,7 @@ public void testGetSubscopedCreds(String scheme) { true, Set.of(warehouseDir + "/namespace/table"), Set.of(warehouseDir + "/namespace/table"), - "/namespace/table/credentials"); + Optional.of("/namespace/table/credentials")); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -268,7 +269,7 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { true, Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), Set.of(s3Path(bucket, firstPath)), - null); + Optional.empty()); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -369,7 +370,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutList() { false, /* allowList = false*/ Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), Set.of(s3Path(bucket, firstPath)), - null); + Optional.empty()); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -464,7 +465,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutWrites() { true, /* allowList = true */ Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), Set.of(), - null); + Optional.empty()); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -527,7 +528,11 @@ public void testGetSubscopedCredsInlinePolicyWithEmptyReadAndWrite() { .build(), stsClient) .getSubscopedCreds( - EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of(), null); + EMPTY_REALM_CONFIG, + true, /* allowList = true */ + Set.of(), + Set.of(), + Optional.empty()); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") @@ -569,7 +574,7 @@ public void testClientRegion(String awsPartition) { true, /* allowList = true */ Set.of(), Set.of(), - null)) + Optional.empty())) .isInstanceOf(IllegalArgumentException.class); break; case AWS_PARTITION: @@ -584,7 +589,11 @@ public void testClientRegion(String awsPartition) { .build(), stsClient) .getSubscopedCreds( - EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of(), null); + EMPTY_REALM_CONFIG, + true, /* allowList = true */ + Set.of(), + Set.of(), + Optional.empty()); assertThat(accessConfig.credentials()) .isNotEmpty() .containsEntry(StorageAccessProperty.CLIENT_REGION.getPropertyName(), clientRegion); @@ -619,7 +628,11 @@ public void testNoClientRegion(String awsPartition) { .build(), stsClient) .getSubscopedCreds( - EMPTY_REALM_CONFIG, true, /* allowList = true */ Set.of(), Set.of(), null); + EMPTY_REALM_CONFIG, + true, /* allowList = true */ + Set.of(), + Set.of(), + Optional.empty()); assertThat(accessConfig.credentials()) .isNotEmpty() .doesNotContainKey(StorageAccessProperty.CLIENT_REGION.getPropertyName()); @@ -640,7 +653,7 @@ public void testNoClientRegion(String awsPartition) { true, /* allowList = true */ Set.of(), Set.of(), - null)) + Optional.empty())) .isInstanceOf(IllegalArgumentException.class); break; default: diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java index d79d6be393..f2818738fc 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java @@ -44,6 +44,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.stream.Stream; import org.apache.polaris.core.storage.AccessConfig; import org.apache.polaris.core.storage.BaseStorageIntegrationTest; @@ -353,7 +354,7 @@ private AccessConfig subscopedCredsForOperations( allowListAction, new HashSet<>(allowedReadLoc), new HashSet<>(allowedWriteLoc), - null); + Optional.empty()); } private BlobContainerClient createContainerClient( diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java index b003abc180..9df6ab9268 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java @@ -41,6 +41,7 @@ import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import org.apache.polaris.core.storage.AccessConfig; import org.apache.polaris.core.storage.BaseStorageIntegrationTest; @@ -171,7 +172,7 @@ private AccessConfig subscopedCredsForOperations( allowListAction, new HashSet<>(allowedReadLoc), new HashSet<>(allowedWriteLoc), - null); + Optional.empty()); } @Test diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 9e311fdbd3..2e0d3f5475 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -831,7 +831,7 @@ public AccessConfig getAccessConfig( TableIdentifier tableIdentifier, TableMetadata tableMetadata, Set storageActions, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { Optional storageInfo = findStorageInfo(tableIdentifier); if (storageInfo.isEmpty()) { LOGGER diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java index 517bccb510..860476cf81 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogAdapter.java @@ -360,11 +360,11 @@ public Response createTable( securityContext, prefix, catalog -> { - String refreshCredentialsEndpoint = - delegationModes.contains(VENDED_CREDENTIALS) - ? new PolarisResourcePaths(prefix) - .credentialsPath(TableIdentifier.of(namespace, createTableRequest.name())) - : null; + Optional refreshCredentialsEndpoint = + getRefreshCredentialsEndpoint( + delegationModes, + prefix, + TableIdentifier.of(namespace, createTableRequest.name())); if (createTableRequest.stageCreate()) { if (delegationModes.isEmpty()) { return Response.ok(catalog.createTableStaged(ns, createTableRequest)).build(); @@ -438,10 +438,8 @@ public Response loadTable( .loadTableIfStale(tableIdentifier, ifNoneMatch, snapshots) .orElseThrow(() -> new WebApplicationException(Response.Status.NOT_MODIFIED)); } else { - String refreshCredentialsEndpoint = - (delegationModes.contains(VENDED_CREDENTIALS)) - ? new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier) - : null; + Optional refreshCredentialsEndpoint = + getRefreshCredentialsEndpoint(delegationModes, prefix, tableIdentifier); response = catalog .loadTableWithAccessDelegationIfStale( @@ -453,6 +451,15 @@ public Response loadTable( }); } + private static Optional getRefreshCredentialsEndpoint( + EnumSet delegationModes, + String prefix, + TableIdentifier tableIdentifier) { + return delegationModes.contains(AccessDelegationMode.VENDED_CREDENTIALS) + ? Optional.of(new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier)) + : Optional.empty(); + } + @Override public Response tableExists( String prefix, @@ -615,7 +622,7 @@ public Response loadCredentials( catalog.loadTableWithAccessDelegation( tableIdentifier, "all", - new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier)); + Optional.of(new PolarisResourcePaths(prefix).credentialsPath(tableIdentifier))); return Response.ok( ImmutableLoadCredentialsResponse.builder() .credentials(loadTableResponse.credentials()) diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java index f2beb2493b..57b8a990a4 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java @@ -393,7 +393,9 @@ public LoadTableResponse createTableDirect(Namespace namespace, CreateTableReque * @return ETagged {@link LoadTableResponse} to uniquely identify the table metadata */ public LoadTableResponse createTableDirectWithWriteDelegation( - Namespace namespace, CreateTableRequest request, String refreshCredentialsEndpoint) { + Namespace namespace, + CreateTableRequest request, + Optional refreshCredentialsEndpoint) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_DIRECT_WITH_WRITE_DELEGATION; authorizeCreateTableLikeUnderNamespaceOperationOrThrow( @@ -501,7 +503,9 @@ public LoadTableResponse createTableStaged(Namespace namespace, CreateTableReque } public LoadTableResponse createTableStagedWithWriteDelegation( - Namespace namespace, CreateTableRequest request, String refreshCredentialsEndpoint) { + Namespace namespace, + CreateTableRequest request, + Optional refreshCredentialsEndpoint) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_TABLE_STAGED_WITH_WRITE_DELEGATION; authorizeCreateTableLikeUnderNamespaceOperationOrThrow( @@ -628,7 +632,9 @@ public Optional loadTableIfStale( } public LoadTableResponse loadTableWithAccessDelegation( - TableIdentifier tableIdentifier, String snapshots, String refreshCredentialsEndpoint) { + TableIdentifier tableIdentifier, + String snapshots, + Optional refreshCredentialsEndpoint) { return loadTableWithAccessDelegationIfStale( tableIdentifier, null, snapshots, refreshCredentialsEndpoint) .get(); @@ -648,7 +654,7 @@ public Optional loadTableWithAccessDelegationIfStale( TableIdentifier tableIdentifier, IfNoneMatch ifNoneMatch, String snapshots, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { // Here we have a single method that falls through multiple candidate // PolarisAuthorizableOperations because instead of identifying the desired operation up-front // and @@ -737,7 +743,7 @@ private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredential TableMetadata tableMetadata, Set actions, String snapshots, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { LoadTableResponse.Builder responseBuilder = LoadTableResponse.builder().withTableMetadata(tableMetadata); if (baseCatalog instanceof SupportsCredentialDelegation credentialDelegation) { diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java index 354e4e7d1b..b85973ed8e 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/SupportsCredentialDelegation.java @@ -18,6 +18,7 @@ */ package org.apache.polaris.service.catalog.iceberg; +import java.util.Optional; import java.util.Set; import org.apache.iceberg.TableMetadata; import org.apache.iceberg.catalog.TableIdentifier; @@ -36,5 +37,5 @@ AccessConfig getAccessConfig( TableIdentifier tableIdentifier, TableMetadata tableMetadata, Set storageActions, - String refreshCredentialsEndpoint); + Optional refreshCredentialsEndpoint); } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java index a62824ce3a..81f6e7c8ff 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/DefaultFileIOFactory.java @@ -92,7 +92,7 @@ public FileIO loadFileIO( tableLocations, storageActions, storageInfo, - null)); + Optional.empty())); // Update the FileIO with the subscoped credentials // Update with properties in case there are table-level overrides the credentials should diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java index f11ba26208..f4a6320d67 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java @@ -82,7 +82,7 @@ public static AccessConfig refreshAccessConfig( Set tableLocations, Set storageActions, PolarisEntity entity, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { boolean skipCredentialSubscopingIndirection = callContext diff --git a/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java b/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java index 1971b11ccb..e04a9525ba 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java @@ -114,7 +114,7 @@ public AccessConfig getSubscopedCreds( boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations, - String refreshCredentialsEndpoint) { + Optional refreshCredentialsEndpoint) { return AccessConfig.builder().build(); } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java index d993051fca..d66d25cd5c 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/AbstractIcebergCatalogTest.java @@ -46,6 +46,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.function.Function; @@ -1829,7 +1830,7 @@ public void testDropTableWithPurge() { true, Set.of(tableMetadata.location()), Set.of(tableMetadata.location()), - null) + Optional.empty()) .getAccessConfig() .credentials(); Assertions.assertThat(credentials) diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java index 22f5aa9c43..a8090f038b 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java @@ -26,6 +26,7 @@ import java.time.Instant; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.UUID; import org.apache.hadoop.conf.Configuration; @@ -617,7 +618,7 @@ public void testCreateTableDirectWithWriteDelegationAllSufficientPrivileges() { () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) .createTableDirectWithWriteDelegation( - NS2, createDirectWithWriteDelegationRequest, null); + NS2, createDirectWithWriteDelegationRequest, Optional.empty()); }, () -> { newWrapper(Set.of(PRINCIPAL_ROLE2)).dropTableWithPurge(newtable); @@ -648,7 +649,7 @@ public void testCreateTableDirectWithWriteDelegationInsufficientPermissions() { () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) .createTableDirectWithWriteDelegation( - NS2, createDirectWithWriteDelegationRequest, null); + NS2, createDirectWithWriteDelegationRequest, Optional.empty()); }); } @@ -722,7 +723,7 @@ public void testCreateTableStagedWithWriteDelegationAllSufficientPrivileges() { () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) .createTableStagedWithWriteDelegation( - NS2, createStagedWithWriteDelegationRequest, null); + NS2, createStagedWithWriteDelegationRequest, Optional.empty()); }, // createTableStagedWithWriteDelegation doesn't actually commit any metadata null, @@ -752,7 +753,7 @@ public void testCreateTableStagedWithWriteDelegationInsufficientPermissions() { () -> { newWrapper(Set.of(PRINCIPAL_ROLE1)) .createTableStagedWithWriteDelegation( - NS2, createStagedWithWriteDelegationRequest, null); + NS2, createStagedWithWriteDelegationRequest, Optional.empty()); }); } @@ -896,7 +897,7 @@ public void testLoadTableWithReadAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", Optional.empty()), null /* cleanupAction */); } @@ -912,7 +913,7 @@ public void testLoadTableWithReadAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null)); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", Optional.empty())); } @Test @@ -925,7 +926,7 @@ public void testLoadTableWithWriteAccessDelegationSufficientPrivileges() { PolarisPrivilege.TABLE_READ_DATA, PolarisPrivilege.TABLE_WRITE_DATA, PolarisPrivilege.CATALOG_MANAGE_CONTENT), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null), + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", Optional.empty()), null /* cleanupAction */); } @@ -941,7 +942,7 @@ public void testLoadTableWithWriteAccessDelegationInsufficientPermissions() { PolarisPrivilege.TABLE_CREATE, PolarisPrivilege.TABLE_LIST, PolarisPrivilege.TABLE_DROP), - () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", null)); + () -> newWrapper().loadTableWithAccessDelegation(TABLE_NS1A_2, "all", Optional.empty())); } @Test @@ -954,7 +955,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleSufficientPrivileges() { () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", Optional.empty()), null /* cleanupAction */); } @@ -973,7 +974,7 @@ public void testLoadTableWithReadAccessDelegationIfStaleInsufficientPermissions( () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null)); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", Optional.empty())); } @Test @@ -989,7 +990,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleSufficientPrivileges() () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null), + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", Optional.empty()), null /* cleanupAction */); } @@ -1008,7 +1009,7 @@ public void testLoadTableWithWriteAccessDelegationIfStaleInsufficientPermissions () -> newWrapper() .loadTableWithAccessDelegationIfStale( - TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", null)); + TABLE_NS1A_2, IfNoneMatch.fromHeader("W/\"0:0\""), "all", Optional.empty())); } @Test From d8d11b3df1b570362a4baa68e390794faa62db81 Mon Sep 17 00:00:00 2001 From: Jason Date: Thu, 21 Aug 2025 11:13:04 +0300 Subject: [PATCH 07/23] added unit test for azure --- ...AzureCredentialsStorageIntegrationTest.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java index 6241be21ee..fab57532ee 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java @@ -24,6 +24,7 @@ import java.time.Instant; import java.util.Optional; import org.apache.polaris.core.storage.AccessConfig; +import org.apache.polaris.core.storage.StorageAccessProperty; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @@ -37,15 +38,28 @@ public void testAzureCredentialFormatting() { toAccessConfig("sasToken", "some_account", expiresAt, Optional.empty()); Assertions.assertThat(noSuffixResult.credentials()).hasSize(2); Assertions.assertThat(noSuffixResult.credentials()).containsKey("adls.sas-token.some_account"); + Assertions.assertThat(noSuffixResult.credentials()) + .doesNotContainKey( + StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT.getPropertyName()); AccessConfig adlsSuffixResult = toAccessConfig( - "sasToken", "some_account." + AzureLocation.ADLS_ENDPOINT, expiresAt, Optional.empty()); - Assertions.assertThat(adlsSuffixResult.credentials()).hasSize(3); + "sasToken", + "some_account." + AzureLocation.ADLS_ENDPOINT, + expiresAt, + Optional.of("endpoint/credentials")); + Assertions.assertThat(adlsSuffixResult.credentials()).hasSize(5); Assertions.assertThat(adlsSuffixResult.credentials()) .containsKey("adls.sas-token.some_account"); Assertions.assertThat(adlsSuffixResult.credentials()) .containsKey("adls.sas-token.some_account." + AzureLocation.ADLS_ENDPOINT); + Assertions.assertThat(adlsSuffixResult.credentials()) + .containsEntry( + StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT.getPropertyName(), + "endpoint/credentials"); + Assertions.assertThat(adlsSuffixResult.credentials()) + .containsEntry( + StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENABLED.getPropertyName(), "true"); AccessConfig blobSuffixResult = toAccessConfig( From 7259b61fd4ff183030d713fe7c6adfe7cf09a1e0 Mon Sep 17 00:00:00 2001 From: Jason Date: Thu, 21 Aug 2025 12:00:39 +0300 Subject: [PATCH 08/23] Add endpoint to cache key --- .../core/storage/cache/StorageCredentialCache.java | 3 ++- .../core/storage/cache/StorageCredentialCacheKey.java | 10 ++++++++-- .../core/storage/cache/StorageCredentialCacheTest.java | 3 ++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java index c26ca67cd1..ab3b3cc4fd 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java @@ -118,7 +118,8 @@ public AccessConfig getOrGenerateSubScopeCreds( polarisEntity, allowListOperation, allowedReadLocations, - allowedWriteLocations); + allowedWriteLocations, + refreshCredentialsEndpoint); LOGGER.atDebug().addKeyValue("key", key).log("subscopedCredsCache"); Function loader = k -> { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheKey.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheKey.java index 79eba7d1dc..8b9d0542d3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheKey.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheKey.java @@ -19,6 +19,7 @@ package org.apache.polaris.core.storage.cache; import jakarta.annotation.Nullable; +import java.util.Optional; import java.util.Set; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntityConstants; @@ -47,12 +48,16 @@ public interface StorageCredentialCacheKey { @Value.Parameter(order = 6) Set allowedWriteLocations(); + @Value.Parameter(order = 7) + Optional refreshCredentialsEndpoint(); + static StorageCredentialCacheKey of( String realmId, PolarisEntity entity, boolean allowedListAction, Set allowedReadLocations, - Set allowedWriteLocations) { + Set allowedWriteLocations, + Optional refreshCredentialsEndpoint) { String storageConfigSerializedStr = entity .getInternalPropertiesAsMap() @@ -63,6 +68,7 @@ static StorageCredentialCacheKey of( storageConfigSerializedStr, allowedListAction, allowedReadLocations, - allowedWriteLocations); + allowedWriteLocations, + refreshCredentialsEndpoint); } } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java index 74e66cf878..becc220a63 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java @@ -185,7 +185,8 @@ public void testCacheEvict() throws InterruptedException { polarisEntity, true, Set.of("s3://bucket1/path", "s3://bucket2/path"), - Set.of("s3://bucket/path")); + Set.of("s3://bucket/path"), + Optional.empty()); // the entry will be evicted immediately because the token is expired storageCredentialCache.getOrGenerateSubScopeCreds( From d449ccf1a6d9e0f394937c997d7e92f001c9fe1e Mon Sep 17 00:00:00 2001 From: Jason Date: Thu, 21 Aug 2025 12:03:41 +0300 Subject: [PATCH 09/23] Added integration test --- .../polaris/service/it/RestCatalogMinIOSpecialIT.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java index 6a93da886e..ef09478f8d 100644 --- a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java +++ b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java @@ -42,12 +42,14 @@ import org.apache.iceberg.Schema; import org.apache.iceberg.Table; import org.apache.iceberg.TableOperations; +import org.apache.iceberg.aws.AwsClientProperties; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.OutputFile; import org.apache.iceberg.io.PositionOutputStream; import org.apache.iceberg.rest.RESTCatalog; import org.apache.iceberg.rest.auth.OAuth2Properties; +import org.apache.iceberg.rest.credentials.Credential; import org.apache.iceberg.rest.responses.LoadTableResponse; import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; @@ -267,6 +269,15 @@ public LoadTableResponse doTestCreateTable(RESTCatalog restCatalog) throws IOExc LoadTableResponse loadTableResponse = catalogApi.loadTableWithAccessDelegation(catalogName, id, "ALL"); assertThat(loadTableResponse.config()).containsKey("s3.endpoint"); + assertThat(loadTableResponse.credentials().stream().map(Credential::config)) + .allSatisfy( + c -> + assertThat(c) + .containsEntry( + AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, Boolean.TRUE.toString()) + .containsEntry( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, + "v1/" + catalogName + "/namespaces/test-ns/tables/t1/credentials")); restCatalog.dropTable(id); assertThat(restCatalog.tableExists(id)).isFalse(); From 211d3155a4b098569712754ba7fdf71f185b083b Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 11:06:52 +0300 Subject: [PATCH 10/23] GCP: Add refresh credential properties' --- .../core/storage/StorageAccessProperty.java | 13 ++++++++++++ .../gcp/GcpCredentialsStorageIntegration.java | 7 +++++++ .../GcpCredentialsStorageIntegrationTest.java | 20 ++++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java index 80c63c347a..8514d3e6bc 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java @@ -19,6 +19,7 @@ package org.apache.polaris.core.storage; import org.apache.iceberg.aws.AwsClientProperties; +import org.apache.iceberg.gcp.GCPProperties; /** * A subset of Iceberg catalog properties recognized by Polaris. @@ -61,6 +62,18 @@ public enum StorageAccessProperty { "the time the gcs access token expires, in milliseconds", true, true), + GCS_REFRESH_CREDENTIALS_ENABLED( + Boolean.class, + GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENABLED, + "whether to enable automatic refresh of credentials", + true, + false), + GCS_REFRESH_CREDENTIALS_ENDPOINT( + String.class, + GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT, + "the endpoint to use for refreshing credentials", + true, + false), // Currently not using ACCESS TOKEN as the ResolvingFileIO is using ADLSFileIO for azure case and // it expects for SAS diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java index 27b1581c7a..966332f02d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java @@ -114,6 +114,13 @@ public AccessConfig getSubscopedCreds( accessConfig.put( StorageAccessProperty.GCS_ACCESS_TOKEN_EXPIRES_AT, String.valueOf(token.getExpirationTime().getTime())); + + refreshCredentialsEndpoint.ifPresent( + endpoint -> { + accessConfig.put(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENDPOINT, endpoint); + accessConfig.put(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENABLED, "true"); + }); + return accessConfig.build(); } diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java index 9df6ab9268..671f57ccee 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java @@ -60,6 +60,8 @@ class GcpCredentialsStorageIntegrationTest extends BaseStorageIntegrationTest { private final String gcsServiceKeyJsonFileLocation = System.getenv("GOOGLE_APPLICATION_CREDENTIALS"); + private static final String REFRESH_ENDPOINT = "get/credentials"; + @ParameterizedTest @ValueSource(booleans = {true, false}) public void testSubscope(boolean allowedListAction) throws Exception { @@ -172,7 +174,7 @@ private AccessConfig subscopedCredsForOperations( allowListAction, new HashSet<>(allowedReadLoc), new HashSet<>(allowedWriteLoc), - Optional.empty()); + Optional.of(REFRESH_ENDPOINT)); } @Test @@ -297,6 +299,22 @@ private boolean recursiveEquals(ContainerNode on1, ContainerNode on2) { return true; } + @Test + public void testRefreshCredentialsEndpointIsReturned() throws IOException { + Assumptions.assumeThat(gcsServiceKeyJsonFileLocation) + .describedAs("Environment variable GOOGLE_APPLICATION_CREDENTIALS not exits") + .isNotNull() + .isNotEmpty(); + + AccessConfig accessConfig = + subscopedCredsForOperations( + List.of("gs://bucket1/path/to/data"), List.of("gs://bucket1/path/to/data"), true); + assertThat(accessConfig.get(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENDPOINT)) + .isEqualTo(REFRESH_ENDPOINT); + assertThat(accessConfig.get(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENABLED)) + .isEqualTo(Boolean.TRUE.toString()); + } + private boolean isNotNull(JsonNode node) { return node != null && !node.isNull(); } From 77d096850fe18502c3671699df0112440f3f00f8 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 11:16:13 +0300 Subject: [PATCH 11/23] Set isCredential to false --- .../polaris/core/storage/StorageAccessProperty.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java index 8514d3e6bc..2deec2b53c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java @@ -46,13 +46,13 @@ public enum StorageAccessProperty { Boolean.class, AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, "whether to enable automatic refresh of credentials", - true, + false, false), AWS_REFRESH_CREDENTIALS_ENDPOINT( String.class, AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, "the endpoint to use for refreshing credentials", - true, + false, false), GCS_ACCESS_TOKEN(String.class, "gcs.oauth2.token", "the gcs scoped access token"), @@ -66,13 +66,13 @@ public enum StorageAccessProperty { Boolean.class, GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENABLED, "whether to enable automatic refresh of credentials", - true, + false, false), GCS_REFRESH_CREDENTIALS_ENDPOINT( String.class, GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT, "the endpoint to use for refreshing credentials", - true, + false, false), // Currently not using ACCESS TOKEN as the ResolvingFileIO is using ADLSFileIO for azure case and @@ -83,13 +83,13 @@ public enum StorageAccessProperty { Boolean.class, "adls.refresh-credentials-enabled", "whether to enable automatic refresh of credentials", - true, + false, false), AZURE_REFRESH_CREDENTIALS_ENDPOINT( String.class, "adls.refresh-credentials-endpoint", "the endpoint to use for refreshing credentials", - true, + false, false), EXPIRATION_TIME( Long.class, From c93d2b336e011a3cf8648eb62f30905caf880243 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 11:21:40 +0300 Subject: [PATCH 12/23] cache: use endpoint from key --- .../polaris/core/storage/cache/StorageCredentialCache.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java index ab3b3cc4fd..d166ee4b14 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java @@ -133,7 +133,7 @@ public AccessConfig getOrGenerateSubScopeCreds( k.allowedListAction(), k.allowedReadLocations(), k.allowedWriteLocations(), - refreshCredentialsEndpoint); + k.refreshCredentialsEndpoint()); if (scopedCredentialsResult.isSuccess()) { long maxCacheDurationMs = maxCacheDurationMs(callCtx.getRealmConfig()); return new StorageCredentialCacheEntry( From dd18fef0d84768a94c58b554880b67dfcc188e90 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 11:24:50 +0300 Subject: [PATCH 13/23] javadoc --- .../apache/polaris/core/storage/PolarisCredentialVendor.java | 4 ++++ .../polaris/core/storage/PolarisStorageIntegration.java | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java index a0060d43ac..d64e9ad88c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java @@ -38,6 +38,10 @@ public interface PolarisCredentialVendor { * allowedWriteLocations * @param allowedReadLocations a set of allowed to read locations * @param allowedWriteLocations a set of allowed to write locations + * @param refreshCredentialsEndpoint an optional endpoint to use for refreshing credentials. If + * supported by the storage type it will be returned to the client in the appropriate + * properties. The endpoint may be relative to the base URI and the client is responsible for + * handling the relative path * @return an enum map containing the scoped credentials */ @Nonnull diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java index 28df510ab2..1828d01c81 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java @@ -56,6 +56,10 @@ public String getStorageIdentifierOrId() { * locations * @param allowedReadLocations a set of allowed to read locations * @param allowedWriteLocations a set of allowed to write locations + * @param refreshCredentialsEndpoint an optional endpoint to use for refreshing credentials. If + * supported by the storage type it will be returned to the client in the appropriate + * properties. The endpoint may be relative to the base URI and the client is responsible for + * handling the relative path * @return An enum map including the scoped credentials */ public abstract AccessConfig getSubscopedCreds( From 8ae8f0e4a7594f4bb76f6f989038d8ca0b80ce9e Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 11:28:33 +0300 Subject: [PATCH 14/23] update property descriptions --- .../apache/polaris/core/storage/StorageAccessProperty.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java index 2deec2b53c..def0d0fd0a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java @@ -51,7 +51,7 @@ public enum StorageAccessProperty { AWS_REFRESH_CREDENTIALS_ENDPOINT( String.class, AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, - "the endpoint to use for refreshing credentials", + "the endpoint to load vended credentials for a table from the catalog", false, false), @@ -71,7 +71,7 @@ public enum StorageAccessProperty { GCS_REFRESH_CREDENTIALS_ENDPOINT( String.class, GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT, - "the endpoint to use for refreshing credentials", + "the endpoint to load vended credentials for a table from the catalog", false, false), @@ -88,7 +88,7 @@ public enum StorageAccessProperty { AZURE_REFRESH_CREDENTIALS_ENDPOINT( String.class, "adls.refresh-credentials-endpoint", - "the endpoint to use for refreshing credentials", + "the endpoint to load vended credentials for a table from the catalog", false, false), EXPIRATION_TIME( From 2d6dd0316cb73b4fc51dbd33dec4455de61d535e Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 11:31:39 +0300 Subject: [PATCH 15/23] changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f90392d8d..c6a2883039 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,9 @@ at locations that better optimize for object storage. - Introduced bootstrap command options to specify custom schema files for database initialization. +- Added refresh credentials endpoint configuration to LoadTableResponse for AWS, Azure, and GCP. Enabling +automatic credential refresh per table on the client side. + ### Changes - Polaris Management API clients must be prepared to deal with new attributes in `AwsStorageConfigInfo` objects. From 2cbfbd5dfc104e61cff2f6cf663962764bd96039 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 18:51:31 +0300 Subject: [PATCH 16/23] changelog update --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6a2883039..0d9f856cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,7 +82,7 @@ at locations that better optimize for object storage. - Introduced bootstrap command options to specify custom schema files for database initialization. - Added refresh credentials endpoint configuration to LoadTableResponse for AWS, Azure, and GCP. Enabling -automatic credential refresh per table on the client side. +automatic storage credential refresh per table on the client side. ### Changes From 4159327171dee45cfd720c0b877664de9db9d694 Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 18:55:57 +0300 Subject: [PATCH 17/23] fixed unit tests --- .../azure/AzureCredentialsStorageIntegrationTest.java | 7 ++++--- .../aws/AwsCredentialsStorageIntegrationTest.java | 9 +++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java index fab57532ee..58a83bc977 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java @@ -48,16 +48,17 @@ public void testAzureCredentialFormatting() { "some_account." + AzureLocation.ADLS_ENDPOINT, expiresAt, Optional.of("endpoint/credentials")); - Assertions.assertThat(adlsSuffixResult.credentials()).hasSize(5); + Assertions.assertThat(adlsSuffixResult.credentials()).hasSize(3); Assertions.assertThat(adlsSuffixResult.credentials()) .containsKey("adls.sas-token.some_account"); Assertions.assertThat(adlsSuffixResult.credentials()) .containsKey("adls.sas-token.some_account." + AzureLocation.ADLS_ENDPOINT); - Assertions.assertThat(adlsSuffixResult.credentials()) + + Assertions.assertThat(adlsSuffixResult.extraProperties()) .containsEntry( StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT.getPropertyName(), "endpoint/credentials"); - Assertions.assertThat(adlsSuffixResult.credentials()) + Assertions.assertThat(adlsSuffixResult.extraProperties()) .containsEntry( StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENABLED.getPropertyName(), "true"); diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java index 44f3b86c5b..7d398c0175 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java @@ -103,14 +103,15 @@ public void testGetSubscopedCreds(String scheme) { .containsEntry(StorageAccessProperty.AWS_TOKEN.getPropertyName(), "sess") .containsEntry(StorageAccessProperty.AWS_KEY_ID.getPropertyName(), "accessKey") .containsEntry(StorageAccessProperty.AWS_SECRET_KEY.getPropertyName(), "secretKey") + .containsEntry( + StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS.getPropertyName(), + String.valueOf(EXPIRE_TIME.toEpochMilli())); + assertThat(accessConfig.extraProperties()) .containsEntry( StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENABLED.getPropertyName(), "true") .containsEntry( StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENDPOINT.getPropertyName(), - "/namespace/table/credentials") - .containsEntry( - StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS.getPropertyName(), - String.valueOf(EXPIRE_TIME.toEpochMilli())); + "/namespace/table/credentials"); } @ParameterizedTest From cae063275e0224ad2007e7a4a16aa6fd83cc10ab Mon Sep 17 00:00:00 2001 From: Jason Date: Mon, 25 Aug 2025 19:00:57 +0300 Subject: [PATCH 18/23] fixed integration tests --- .../service/it/RestCatalogMinIOSpecialIT.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java index ef09478f8d..8513f09979 100644 --- a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java +++ b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java @@ -49,7 +49,6 @@ import org.apache.iceberg.io.PositionOutputStream; import org.apache.iceberg.rest.RESTCatalog; import org.apache.iceberg.rest.auth.OAuth2Properties; -import org.apache.iceberg.rest.credentials.Credential; import org.apache.iceberg.rest.responses.LoadTableResponse; import org.apache.iceberg.types.Types; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; @@ -268,16 +267,12 @@ public LoadTableResponse doTestCreateTable(RESTCatalog restCatalog) throws IOExc LoadTableResponse loadTableResponse = catalogApi.loadTableWithAccessDelegation(catalogName, id, "ALL"); - assertThat(loadTableResponse.config()).containsKey("s3.endpoint"); - assertThat(loadTableResponse.credentials().stream().map(Credential::config)) - .allSatisfy( - c -> - assertThat(c) - .containsEntry( - AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, Boolean.TRUE.toString()) - .containsEntry( - AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, - "v1/" + catalogName + "/namespaces/test-ns/tables/t1/credentials")); + assertThat(loadTableResponse.config()) + .containsKey("s3.endpoint") + .containsEntry(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, Boolean.TRUE.toString()) + .containsEntry( + AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, + "v1/" + catalogName + "/namespaces/test-ns/tables/t1/credentials"); restCatalog.dropTable(id); assertThat(restCatalog.tableExists(id)).isFalse(); From 2a123d1df204c748622ff4b85f3a8906daaa8a9c Mon Sep 17 00:00:00 2001 From: Jason Date: Tue, 26 Aug 2025 10:51:48 +0300 Subject: [PATCH 19/23] Remove REFRESH_CREDENTIALS_ENABLED from server response --- .../core/storage/StorageAccessProperty.java | 18 ------------------ .../aws/AwsCredentialsStorageIntegration.java | 1 - .../AzureCredentialsStorageIntegration.java | 1 - .../gcp/GcpCredentialsStorageIntegration.java | 1 - ...AzureCredentialsStorageIntegrationTest.java | 3 --- .../AwsCredentialsStorageIntegrationTest.java | 2 -- .../GcpCredentialsStorageIntegrationTest.java | 2 -- .../service/it/RestCatalogMinIOSpecialIT.java | 1 - 8 files changed, 29 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java index def0d0fd0a..faa29c31e2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageAccessProperty.java @@ -42,12 +42,6 @@ public enum StorageAccessProperty { Boolean.class, "s3.path-style-access", "whether to use S3 path style access", false), CLIENT_REGION( String.class, "client.region", "region to configure client for making requests to AWS"), - AWS_REFRESH_CREDENTIALS_ENABLED( - Boolean.class, - AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, - "whether to enable automatic refresh of credentials", - false, - false), AWS_REFRESH_CREDENTIALS_ENDPOINT( String.class, AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, @@ -62,12 +56,6 @@ public enum StorageAccessProperty { "the time the gcs access token expires, in milliseconds", true, true), - GCS_REFRESH_CREDENTIALS_ENABLED( - Boolean.class, - GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENABLED, - "whether to enable automatic refresh of credentials", - false, - false), GCS_REFRESH_CREDENTIALS_ENDPOINT( String.class, GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT, @@ -79,12 +67,6 @@ public enum StorageAccessProperty { // it expects for SAS AZURE_ACCESS_TOKEN(String.class, "", "the azure scoped access token"), AZURE_SAS_TOKEN(String.class, "adls.sas-token.", "an azure shared access signature token"), - AZURE_REFRESH_CREDENTIALS_ENABLED( - Boolean.class, - "adls.refresh-credentials-enabled", - "whether to enable automatic refresh of credentials", - false, - false), AZURE_REFRESH_CREDENTIALS_ENDPOINT( String.class, "adls.refresh-credentials-endpoint", diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java index 7c73b17c38..3e93ba7b4b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java @@ -123,7 +123,6 @@ public AccessConfig getSubscopedCreds( refreshCredentialsEndpoint.ifPresent( endpoint -> { - accessConfig.put(StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENABLED, "true"); accessConfig.put(StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENDPOINT, endpoint); }); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java index 9c9469b2c7..5b466b0c3c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java @@ -188,7 +188,6 @@ static AccessConfig toAccessConfig( refreshCredentialsEndpoint.ifPresent( endpoint -> { accessConfig.put(StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT, endpoint); - accessConfig.put(StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENABLED, "true"); }); return accessConfig.build(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java index 966332f02d..c0568cc9b5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java @@ -118,7 +118,6 @@ public AccessConfig getSubscopedCreds( refreshCredentialsEndpoint.ifPresent( endpoint -> { accessConfig.put(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENDPOINT, endpoint); - accessConfig.put(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENABLED, "true"); }); return accessConfig.build(); diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java index 58a83bc977..d613e5154d 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegrationTest.java @@ -58,9 +58,6 @@ public void testAzureCredentialFormatting() { .containsEntry( StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENDPOINT.getPropertyName(), "endpoint/credentials"); - Assertions.assertThat(adlsSuffixResult.extraProperties()) - .containsEntry( - StorageAccessProperty.AZURE_REFRESH_CREDENTIALS_ENABLED.getPropertyName(), "true"); AccessConfig blobSuffixResult = toAccessConfig( diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java index 7d398c0175..7b4b50dece 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java @@ -107,8 +107,6 @@ public void testGetSubscopedCreds(String scheme) { StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS.getPropertyName(), String.valueOf(EXPIRE_TIME.toEpochMilli())); assertThat(accessConfig.extraProperties()) - .containsEntry( - StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENABLED.getPropertyName(), "true") .containsEntry( StorageAccessProperty.AWS_REFRESH_CREDENTIALS_ENDPOINT.getPropertyName(), "/namespace/table/credentials"); diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java index 671f57ccee..f1a7afc636 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/gcp/GcpCredentialsStorageIntegrationTest.java @@ -311,8 +311,6 @@ public void testRefreshCredentialsEndpointIsReturned() throws IOException { List.of("gs://bucket1/path/to/data"), List.of("gs://bucket1/path/to/data"), true); assertThat(accessConfig.get(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENDPOINT)) .isEqualTo(REFRESH_ENDPOINT); - assertThat(accessConfig.get(StorageAccessProperty.GCS_REFRESH_CREDENTIALS_ENABLED)) - .isEqualTo(Boolean.TRUE.toString()); } private boolean isNotNull(JsonNode node) { diff --git a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java index 8513f09979..83ddd0c570 100644 --- a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java +++ b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java @@ -269,7 +269,6 @@ public LoadTableResponse doTestCreateTable(RESTCatalog restCatalog) throws IOExc catalogApi.loadTableWithAccessDelegation(catalogName, id, "ALL"); assertThat(loadTableResponse.config()) .containsKey("s3.endpoint") - .containsEntry(AwsClientProperties.REFRESH_CREDENTIALS_ENABLED, Boolean.TRUE.toString()) .containsEntry( AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT, "v1/" + catalogName + "/namespaces/test-ns/tables/t1/credentials"); From d8026f2338769a2425219b4eb53020f2288dc0ac Mon Sep 17 00:00:00 2001 From: Jason Date: Tue, 26 Aug 2025 18:41:36 +0300 Subject: [PATCH 20/23] Updated changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d9f856cd8..6353b1ab33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,7 +82,9 @@ at locations that better optimize for object storage. - Introduced bootstrap command options to specify custom schema files for database initialization. - Added refresh credentials endpoint configuration to LoadTableResponse for AWS, Azure, and GCP. Enabling -automatic storage credential refresh per table on the client side. +automatic storage credential refresh per table on the client side. Java client version >= 1.8.0 is required. +The endpoint path is always returned when using vended credentials, but the client must configure +REFRESH\_ENDPOINT\_ENABLED = "true" in the catalog properties for the client to use the endpoint. ### Changes From a6ac0cfa44288616057e3add8fe89495ff2b555b Mon Sep 17 00:00:00 2001 From: Jason Date: Tue, 26 Aug 2025 18:43:21 +0300 Subject: [PATCH 21/23] Updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6353b1ab33..a0e1b86202 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,7 +83,7 @@ at locations that better optimize for object storage. - Added refresh credentials endpoint configuration to LoadTableResponse for AWS, Azure, and GCP. Enabling automatic storage credential refresh per table on the client side. Java client version >= 1.8.0 is required. -The endpoint path is always returned when using vended credentials, but the client must configure +The endpoint path is always returned when using vended credentials, but the client must configure REFRESH\_ENDPOINT\_ENABLED = "true" in the catalog properties for the client to use the endpoint. ### Changes From 9b9dc412110ab11bf8b2f4c88f4a8cb523026209 Mon Sep 17 00:00:00 2001 From: Jason Date: Tue, 26 Aug 2025 18:51:59 +0300 Subject: [PATCH 22/23] Updated changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0e1b86202..5566aa6280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,7 +84,8 @@ at locations that better optimize for object storage. - Added refresh credentials endpoint configuration to LoadTableResponse for AWS, Azure, and GCP. Enabling automatic storage credential refresh per table on the client side. Java client version >= 1.8.0 is required. The endpoint path is always returned when using vended credentials, but the client must configure -REFRESH\_ENDPOINT\_ENABLED = "true" in the catalog properties for the client to use the endpoint. +"refresh-credentials-enabled" = "true" in the catalog properties for the client to use the endpoint. +Note, the "refresh-credentials-enabled" configuration prefix is specific to each storage provider. ### Changes From e59fc7e8c8fe579ab046cd09c358815ec0436d01 Mon Sep 17 00:00:00 2001 From: Jason Date: Tue, 26 Aug 2025 18:54:31 +0300 Subject: [PATCH 23/23] Updated changelog --- CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5566aa6280..01e9f9f087 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,9 +83,8 @@ at locations that better optimize for object storage. - Added refresh credentials endpoint configuration to LoadTableResponse for AWS, Azure, and GCP. Enabling automatic storage credential refresh per table on the client side. Java client version >= 1.8.0 is required. -The endpoint path is always returned when using vended credentials, but the client must configure -"refresh-credentials-enabled" = "true" in the catalog properties for the client to use the endpoint. -Note, the "refresh-credentials-enabled" configuration prefix is specific to each storage provider. +The endpoint path is always returned when using vended credentials, but clients must enable the +refresh-credentials flag for the desired storage provider. ### Changes