From 4e71b63aa355f569deaded26583ee6fe0a250995 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 10 Jun 2025 15:46:21 -0700 Subject: [PATCH 1/7] initial rebase --- ...EclipseLinkPolarisMetaStoreManagerFactory.java | 9 ++++++--- .../jdbc/JdbcMetaStoreManagerFactory.java | 4 +++- .../LocalPolarisMetaStoreManagerFactory.java | 9 +++++++-- .../persistence/cache/InMemoryEntityCache.java | 15 +++++++++++---- .../quarkus/catalog/IcebergCatalogTest.java | 5 +++-- .../quarkus/catalog/IcebergCatalogViewTest.java | 2 +- .../PolarisCatalogWithEntityCacheTest.java | 2 +- .../catalog/PolarisGenericTableCatalogTest.java | 4 ++-- .../quarkus/catalog/PolicyCatalogTest.java | 4 ++-- ...oryAtomicOperationMetaStoreManagerFactory.java | 9 ++++++--- .../InMemoryPolarisMetaStoreManagerFactory.java | 9 ++++++--- .../org/apache/polaris/service/TestServices.java | 2 +- 12 files changed, 49 insertions(+), 25 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java index 492f61668d..d8b32ffeea 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java @@ -25,6 +25,7 @@ import jakarta.inject.Inject; import java.nio.file.Path; import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.LocalPolarisMetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; @@ -46,12 +47,14 @@ public class EclipseLinkPolarisMetaStoreManagerFactory @Inject PolarisStorageIntegrationProvider storageIntegrationProvider; protected EclipseLinkPolarisMetaStoreManagerFactory() { - this(null); + this(null, null); } @Inject - protected EclipseLinkPolarisMetaStoreManagerFactory(PolarisDiagnostics diagnostics) { - super(diagnostics); + protected EclipseLinkPolarisMetaStoreManagerFactory( + PolarisDiagnostics diagnostics, + PolarisConfigurationStore configurationStore) { + super(diagnostics, configurationStore); } @Override diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java index f944654142..9fdbd969c3 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java @@ -32,6 +32,7 @@ import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.PolarisEntity; @@ -74,6 +75,7 @@ public class JdbcMetaStoreManagerFactory implements MetaStoreManagerFactory { @Inject PolarisStorageIntegrationProvider storageIntegrationProvider; @Inject Instance dataSource; @Inject RelationalJdbcConfiguration relationalJdbcConfiguration; + @Inject PolarisConfigurationStore configurationStore; protected JdbcMetaStoreManagerFactory() {} @@ -216,7 +218,7 @@ public synchronized EntityCache getOrCreateEntityCache(RealmContext realmContext if (!entityCacheMap.containsKey(realmContext.getRealmIdentifier())) { PolarisMetaStoreManager metaStoreManager = getOrCreateMetaStoreManager(realmContext); entityCacheMap.put( - realmContext.getRealmIdentifier(), new InMemoryEntityCache(metaStoreManager)); + realmContext.getRealmIdentifier(), new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); } return entityCacheMap.get(realmContext.getRealmIdentifier()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java index 1cfa89d0f0..98ced10a56 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java @@ -26,6 +26,7 @@ import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.PolarisEntity; @@ -63,10 +64,14 @@ public abstract class LocalPolarisMetaStoreManagerFactory LoggerFactory.getLogger(LocalPolarisMetaStoreManagerFactory.class); private final PolarisDiagnostics diagnostics; + private final PolarisConfigurationStore configurationStore; private boolean bootstrap; - protected LocalPolarisMetaStoreManagerFactory(@Nonnull PolarisDiagnostics diagnostics) { + protected LocalPolarisMetaStoreManagerFactory( + @Nonnull PolarisDiagnostics diagnostics, + @Nonnull PolarisConfigurationStore configurationStore) { this.diagnostics = diagnostics; + this.configurationStore = configurationStore; } protected abstract StoreType createBackingStore(@Nonnull PolarisDiagnostics diagnostics); @@ -188,7 +193,7 @@ public synchronized EntityCache getOrCreateEntityCache(RealmContext realmContext if (!entityCacheMap.containsKey(realmContext.getRealmIdentifier())) { PolarisMetaStoreManager metaStoreManager = getOrCreateMetaStoreManager(realmContext); entityCacheMap.put( - realmContext.getRealmIdentifier(), new InMemoryEntityCache(metaStoreManager)); + realmContext.getRealmIdentifier(), new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); } return entityCacheMap.get(realmContext.getRealmIdentifier()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java index 4599f4aed1..bc44821617 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java @@ -31,6 +31,8 @@ import org.apache.polaris.core.config.BehaviorChangeConfiguration; import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.config.PolarisConfiguration; +import org.apache.polaris.core.config.PolarisConfigurationStore; +import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.entity.PolarisGrantRecord; @@ -58,7 +60,10 @@ public class InMemoryEntityCache implements EntityCache { * * @param polarisMetaStoreManager the meta store manager implementation */ - public InMemoryEntityCache(@Nonnull PolarisMetaStoreManager polarisMetaStoreManager) { + public InMemoryEntityCache( + @Nonnull RealmContext realmContext, + @Nonnull PolarisConfigurationStore configurationStore, + @Nonnull PolarisMetaStoreManager polarisMetaStoreManager) { // by name cache this.byName = new ConcurrentHashMap<>(); @@ -75,8 +80,8 @@ public InMemoryEntityCache(@Nonnull PolarisMetaStoreManager polarisMetaStoreMana } }; - long weigherTarget = - PolarisConfiguration.loadConfig(FeatureConfiguration.ENTITY_CACHE_WEIGHER_TARGET); + long weigherTarget = configurationStore + .getConfiguration(realmContext, FeatureConfiguration.ENTITY_CACHE_WEIGHER_TARGET); Caffeine byIdBuilder = Caffeine.newBuilder() .maximumWeight(weigherTarget) @@ -84,7 +89,9 @@ public InMemoryEntityCache(@Nonnull PolarisMetaStoreManager polarisMetaStoreMana .expireAfterAccess(1, TimeUnit.HOURS) // Expire entries after 1 hour of no access .removalListener(removalListener); // Set the removal listener - if (PolarisConfiguration.loadConfig(BehaviorChangeConfiguration.ENTITY_CACHE_SOFT_VALUES)) { + boolean useSoftValues = configurationStore + .getConfiguration(realmContext, BehaviorChangeConfiguration.ENTITY_CACHE_SOFT_VALUES); + if (useSoftValues) { byIdBuilder.softValues(); } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 2b4c4205c9..88f54daadb 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -211,8 +211,9 @@ public Map getConfigOverrides() { @Inject PolarisDiagnostics diagServices; @Inject PolarisEventListener polarisEventListener; + protected String realmName; + private IcebergCatalog catalog; - private String realmName; private PolarisMetaStoreManager metaStoreManager; private UserSecretsManager userSecretsManager; private PolarisCallContext polarisContext; @@ -428,7 +429,7 @@ public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext rea @Override public InMemoryEntityCache getOrCreateEntityCache(RealmContext realmContext) { - return new InMemoryEntityCache(metaStoreManager); + return new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager); } @Override diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java index 0f06a02250..7ff4ee5e7c 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java @@ -180,7 +180,7 @@ public void before(TestInfo testInfo) { new PolarisEntityManager( metaStoreManager, new StorageCredentialCache(), - new InMemoryEntityCache(metaStoreManager)); + new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); CallContext.setCurrentContext(polarisContext); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java index 079b02befc..d3515c7775 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java @@ -31,6 +31,6 @@ public class PolarisCatalogWithEntityCacheTest extends IcebergCatalogTest { @Nullable @Override protected InMemoryEntityCache createEntityCache(PolarisMetaStoreManager metaStoreManager) { - return new InMemoryEntityCache(metaStoreManager); + return new InMemoryEntityCache(() -> realmName, configurationStore, metaStoreManager); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java index 2aba1773b9..1d1d4c4d7c 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java @@ -178,7 +178,7 @@ public void before(TestInfo testInfo) { new PolarisEntityManager( metaStoreManager, new StorageCredentialCache(), - new InMemoryEntityCache(metaStoreManager)); + new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); PrincipalEntity rootEntity = new PrincipalEntity( @@ -307,7 +307,7 @@ public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext rea @Override public InMemoryEntityCache getOrCreateEntityCache(RealmContext realmContext) { - return new InMemoryEntityCache(metaStoreManager); + return new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager); } @Override diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java index 93ee060723..fc9c69cece 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java @@ -204,7 +204,7 @@ public void before(TestInfo testInfo) { new PolarisEntityManager( metaStoreManager, new StorageCredentialCache(), - new InMemoryEntityCache(metaStoreManager)); + new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); callContext = polarisContext; @@ -331,7 +331,7 @@ public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext rea @Override public InMemoryEntityCache getOrCreateEntityCache(RealmContext realmContext) { - return new InMemoryEntityCache(metaStoreManager); + return new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager); } @Override diff --git a/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryAtomicOperationMetaStoreManagerFactory.java b/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryAtomicOperationMetaStoreManagerFactory.java index 3f0b1a3552..83908f0178 100644 --- a/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryAtomicOperationMetaStoreManagerFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryAtomicOperationMetaStoreManagerFactory.java @@ -22,6 +22,7 @@ import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.persistence.AtomicOperationMetaStoreManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.storage.PolarisStorageIntegrationProvider; @@ -36,13 +37,15 @@ public class InMemoryAtomicOperationMetaStoreManagerFactory extends InMemoryPolarisMetaStoreManagerFactory { public InMemoryAtomicOperationMetaStoreManagerFactory() { - super(null, null); + super(null, null, null); } @Inject public InMemoryAtomicOperationMetaStoreManagerFactory( - PolarisStorageIntegrationProvider storageIntegration, PolarisDiagnostics diagnostics) { - super(storageIntegration, diagnostics); + PolarisStorageIntegrationProvider storageIntegration, + PolarisDiagnostics diagnostics, + PolarisConfigurationStore configurationStore) { + super(storageIntegration, diagnostics, configurationStore); } @Override diff --git a/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java b/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java index 895e5b51b1..367a57de63 100644 --- a/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java +++ b/service/common/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java @@ -29,6 +29,7 @@ import java.util.Set; import java.util.function.Supplier; import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.LocalPolarisMetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; @@ -48,13 +49,15 @@ public class InMemoryPolarisMetaStoreManagerFactory private final Set bootstrappedRealms = new HashSet<>(); public InMemoryPolarisMetaStoreManagerFactory() { - this(null, null); + this(null, null, null); } @Inject public InMemoryPolarisMetaStoreManagerFactory( - PolarisStorageIntegrationProvider storageIntegration, PolarisDiagnostics diagnostics) { - super(diagnostics); + PolarisStorageIntegrationProvider storageIntegration, + PolarisDiagnostics diagnostics, + PolarisConfigurationStore configurationStore) { + super(diagnostics, configurationStore); this.storageIntegration = storageIntegration; } diff --git a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java index d3622126fb..8f63ecfb83 100644 --- a/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java +++ b/service/common/src/testFixtures/java/org/apache/polaris/service/TestServices.java @@ -154,7 +154,7 @@ public TestServices build() { () -> GoogleCredentials.create(new AccessToken(GCP_ACCESS_TOKEN, new Date()))); InMemoryPolarisMetaStoreManagerFactory metaStoreManagerFactory = new InMemoryPolarisMetaStoreManagerFactory( - storageIntegrationProvider, polarisDiagnostics); + storageIntegrationProvider, polarisDiagnostics, configurationStore); RealmEntityManagerFactory realmEntityManagerFactory = new RealmEntityManagerFactory(metaStoreManagerFactory) {}; UserSecretsManagerFactory userSecretsManagerFactory = From ce8af299fa694d125efdb0733de670008b8b4b6e Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 10 Jun 2025 15:51:18 -0700 Subject: [PATCH 2/7] continue refactor --- ...pseLinkPolarisMetaStoreManagerFactory.java | 3 +-- .../jdbc/JdbcMetaStoreManagerFactory.java | 3 ++- .../polaris/core/entity/CatalogEntity.java | 14 ------------ .../LocalPolarisMetaStoreManagerFactory.java | 3 ++- .../cache/InMemoryEntityCache.java | 11 +++++----- .../storage/cache/StorageCredentialCache.java | 22 ++++++++++++------- 6 files changed, 25 insertions(+), 31 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java index d8b32ffeea..9f7af1a268 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java @@ -52,8 +52,7 @@ protected EclipseLinkPolarisMetaStoreManagerFactory() { @Inject protected EclipseLinkPolarisMetaStoreManagerFactory( - PolarisDiagnostics diagnostics, - PolarisConfigurationStore configurationStore) { + PolarisDiagnostics diagnostics, PolarisConfigurationStore configurationStore) { super(diagnostics, configurationStore); } diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java index 9fdbd969c3..a0c48d5de6 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java @@ -218,7 +218,8 @@ public synchronized EntityCache getOrCreateEntityCache(RealmContext realmContext if (!entityCacheMap.containsKey(realmContext.getRealmIdentifier())) { PolarisMetaStoreManager metaStoreManager = getOrCreateMetaStoreManager(realmContext); entityCacheMap.put( - realmContext.getRealmIdentifier(), new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); + realmContext.getRealmIdentifier(), + new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); } return entityCacheMap.get(realmContext.getRealmIdentifier()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java index a995883304..54735a5993 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java @@ -261,7 +261,6 @@ public Builder setStorageConfigurationInfo( throw new BadRequestException("Must specify default base location"); } allowedLocations.add(defaultBaseLocation); - validateMaxAllowedLocations(allowedLocations); switch (storageConfigModel.getStorageType()) { case S3: AwsStorageConfigInfo awsConfigModel = (AwsStorageConfigInfo) storageConfigModel; @@ -304,19 +303,6 @@ public Builder setStorageConfigurationInfo( return this; } - /** Validate the number of allowed locations not exceeding the max value. */ - private void validateMaxAllowedLocations(Collection allowedLocations) { - int maxAllowedLocations = - BehaviorChangeConfiguration.loadConfig( - BehaviorChangeConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS); - if (maxAllowedLocations != -1 && allowedLocations.size() > maxAllowedLocations) { - throw new IllegalArgumentException( - String.format( - "Number of configured locations (%s) exceeds the limit of %s", - allowedLocations.size(), maxAllowedLocations)); - } - } - public Builder setConnectionConfigInfoDpoWithSecrets( ConnectionConfigInfo connectionConfigurationModel, Map secretReferences) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java index 98ced10a56..8a44cb6b7b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java @@ -193,7 +193,8 @@ public synchronized EntityCache getOrCreateEntityCache(RealmContext realmContext if (!entityCacheMap.containsKey(realmContext.getRealmIdentifier())) { PolarisMetaStoreManager metaStoreManager = getOrCreateMetaStoreManager(realmContext); entityCacheMap.put( - realmContext.getRealmIdentifier(), new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); + realmContext.getRealmIdentifier(), + new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); } return entityCacheMap.get(realmContext.getRealmIdentifier()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java index bc44821617..3e1fc3ec66 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCache.java @@ -30,7 +30,6 @@ import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.config.BehaviorChangeConfiguration; import org.apache.polaris.core.config.FeatureConfiguration; -import org.apache.polaris.core.config.PolarisConfiguration; import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.PolarisBaseEntity; @@ -80,8 +79,9 @@ public InMemoryEntityCache( } }; - long weigherTarget = configurationStore - .getConfiguration(realmContext, FeatureConfiguration.ENTITY_CACHE_WEIGHER_TARGET); + long weigherTarget = + configurationStore.getConfiguration( + realmContext, FeatureConfiguration.ENTITY_CACHE_WEIGHER_TARGET); Caffeine byIdBuilder = Caffeine.newBuilder() .maximumWeight(weigherTarget) @@ -89,8 +89,9 @@ public InMemoryEntityCache( .expireAfterAccess(1, TimeUnit.HOURS) // Expire entries after 1 hour of no access .removalListener(removalListener); // Set the removal listener - boolean useSoftValues = configurationStore - .getConfiguration(realmContext, BehaviorChangeConfiguration.ENTITY_CACHE_SOFT_VALUES); + boolean useSoftValues = + configurationStore.getConfiguration( + realmContext, BehaviorChangeConfiguration.ENTITY_CACHE_SOFT_VALUES); if (useSoftValues) { byIdBuilder.softValues(); } 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 ef92973600..70814eae64 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 @@ -33,6 +33,8 @@ import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.config.PolarisConfiguration; +import org.apache.polaris.core.config.PolarisConfigurationStore; +import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.PolarisEntity; import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; @@ -47,10 +49,15 @@ public class StorageCredentialCache { private static final Logger LOGGER = LoggerFactory.getLogger(StorageCredentialCache.class); private static final long CACHE_MAX_NUMBER_OF_ENTRIES = 10_000L; + private final LoadingCache cache; + private final RealmContext realmContext; + private final PolarisConfigurationStore configurationStore; /** Initialize the creds cache */ - public StorageCredentialCache() { + public StorageCredentialCache(RealmContext realmContext, PolarisConfigurationStore configurationStore) { + this.realmContext = realmContext; + this.configurationStore = configurationStore; cache = Caffeine.newBuilder() .maximumSize(CACHE_MAX_NUMBER_OF_ENTRIES) @@ -62,7 +69,7 @@ public StorageCredentialCache() { 0, Math.min( (entry.getExpirationTime() - System.currentTimeMillis()) / 2, - maxCacheDurationMs())); + this.maxCacheDurationMs())); return Duration.ofMillis(expireAfterMillis); })) .build( @@ -73,12 +80,11 @@ public StorageCredentialCache() { } /** How long credentials should remain in the cache. */ - private static long maxCacheDurationMs() { - var cacheDurationSeconds = - PolarisConfiguration.loadConfig( - FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS); - var credentialDurationSeconds = - PolarisConfiguration.loadConfig(FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS); + private long maxCacheDurationMs() { + var cacheDurationSeconds = configurationStore + .getConfiguration(realmContext, FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS); + var credentialDurationSeconds = configurationStore + .getConfiguration(realmContext, FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS); if (cacheDurationSeconds >= credentialDurationSeconds) { throw new IllegalArgumentException( String.format( From 34407d4aa593e3d57ba2b61032acd73531846ef1 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 10 Jun 2025 16:11:17 -0700 Subject: [PATCH 3/7] stable --- .../jdbc/JdbcMetaStoreManagerFactory.java | 3 ++- .../polaris/core/entity/CatalogEntity.java | 2 -- .../LocalPolarisMetaStoreManagerFactory.java | 3 ++- .../storage/cache/StorageCredentialCache.java | 14 +++++++------ .../cache/InMemoryEntityCacheTest.java | 3 ++- .../cache/StorageCredentialCacheTest.java | 20 +++++++++++-------- .../core/persistence/BaseResolverTest.java | 4 +++- .../quarkus/config/QuarkusProducers.java | 5 +++-- .../quarkus/catalog/IcebergCatalogTest.java | 6 ++++-- .../catalog/IcebergCatalogViewTest.java | 2 +- .../PolarisGenericTableCatalogTest.java | 4 ++-- .../quarkus/catalog/PolicyCatalogTest.java | 4 ++-- 12 files changed, 41 insertions(+), 29 deletions(-) diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java index a0c48d5de6..4b369953dc 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java @@ -207,7 +207,8 @@ public synchronized StorageCredentialCache getOrCreateStorageCredentialCache( RealmContext realmContext) { if (!storageCredentialCacheMap.containsKey(realmContext.getRealmIdentifier())) { storageCredentialCacheMap.put( - realmContext.getRealmIdentifier(), new StorageCredentialCache()); + realmContext.getRealmIdentifier(), + new StorageCredentialCache(realmContext, configurationStore)); } return storageCredentialCacheMap.get(realmContext.getRealmIdentifier()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java index 54735a5993..45c114a283 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java @@ -23,7 +23,6 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -41,7 +40,6 @@ import org.apache.polaris.core.admin.model.GcpStorageConfigInfo; import org.apache.polaris.core.admin.model.PolarisCatalog; import org.apache.polaris.core.admin.model.StorageConfigInfo; -import org.apache.polaris.core.config.BehaviorChangeConfiguration; import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; import org.apache.polaris.core.secrets.UserSecretReference; import org.apache.polaris.core.storage.FileStorageConfigurationInfo; diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java index 8a44cb6b7b..747991636a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java @@ -182,7 +182,8 @@ public synchronized StorageCredentialCache getOrCreateStorageCredentialCache( RealmContext realmContext) { if (!storageCredentialCacheMap.containsKey(realmContext.getRealmIdentifier())) { storageCredentialCacheMap.put( - realmContext.getRealmIdentifier(), new StorageCredentialCache()); + realmContext.getRealmIdentifier(), + new StorageCredentialCache(realmContext, configurationStore)); } return storageCredentialCacheMap.get(realmContext.getRealmIdentifier()); 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 70814eae64..36f666db04 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 @@ -32,7 +32,6 @@ import org.apache.iceberg.exceptions.UnprocessableEntityException; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.config.FeatureConfiguration; -import org.apache.polaris.core.config.PolarisConfiguration; import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.PolarisEntity; @@ -55,7 +54,8 @@ public class StorageCredentialCache { private final PolarisConfigurationStore configurationStore; /** Initialize the creds cache */ - public StorageCredentialCache(RealmContext realmContext, PolarisConfigurationStore configurationStore) { + public StorageCredentialCache( + RealmContext realmContext, PolarisConfigurationStore configurationStore) { this.realmContext = realmContext; this.configurationStore = configurationStore; cache = @@ -81,10 +81,12 @@ public StorageCredentialCache(RealmContext realmContext, PolarisConfigurationSto /** How long credentials should remain in the cache. */ private long maxCacheDurationMs() { - var cacheDurationSeconds = configurationStore - .getConfiguration(realmContext, FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS); - var credentialDurationSeconds = configurationStore - .getConfiguration(realmContext, FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS); + var cacheDurationSeconds = + configurationStore.getConfiguration( + realmContext, FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS); + var credentialDurationSeconds = + configurationStore.getConfiguration( + realmContext, FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS); if (cacheDurationSeconds >= credentialDurationSeconds) { throw new IllegalArgumentException( String.format( diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCacheTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCacheTest.java index 1d0564be9e..945f1ccb6b 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCacheTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/cache/InMemoryEntityCacheTest.java @@ -103,7 +103,8 @@ public InMemoryEntityCacheTest() { * @return new cache for the entity store */ InMemoryEntityCache allocateNewCache() { - return new InMemoryEntityCache(this.metaStoreManager); + return new InMemoryEntityCache( + callCtx.getRealmContext(), callCtx.getConfigurationStore(), this.metaStoreManager); } @Test 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 b1d1789dac..5a5e468491 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 @@ -72,12 +72,16 @@ public StorageCredentialCacheTest() { new TreeMapTransactionalPersistenceImpl(store, Mockito.mock(), RANDOM_SECRETS); callCtx = new PolarisCallContext(() -> "testRealm", metaStore, diagServices); metaStoreManager = Mockito.mock(PolarisMetaStoreManager.class); - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); + } + + private StorageCredentialCache getStorageCredentialCache() { + return new StorageCredentialCache(callCtx.getRealmContext(), callCtx.getConfigurationStore()); } @Test public void testBadResult() { - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); ScopedCredentialsResult badResult = new ScopedCredentialsResult( BaseResult.ReturnStatus.SUBSCOPE_CREDS_ERROR, "extra_error_info"); @@ -110,7 +114,7 @@ public void testBadResult() { @Test public void testCacheHit() { - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ false); Mockito.when( @@ -153,7 +157,7 @@ public void testCacheHit() { @RepeatedTest(10) public void testCacheEvict() throws InterruptedException { - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ true); Mockito.when( @@ -211,7 +215,7 @@ public void testCacheEvict() throws InterruptedException { @Test public void testCacheGenerateNewEntries() { - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ false); Mockito.when( @@ -298,7 +302,7 @@ public void testCacheGenerateNewEntries() { @Test public void testCacheNotAffectedBy() { - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ false); @@ -443,7 +447,7 @@ private static List getPolarisEntities() { @Test public void testAzureCredentialFormatting() { - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); List mockedScopedCreds = List.of( new ScopedCredentialsResult( @@ -532,7 +536,7 @@ public void testAzureCredentialFormatting() { @Test public void testExtraProperties() { - storageCredentialCache = new StorageCredentialCache(); + storageCredentialCache = getStorageCredentialCache(); ScopedCredentialsResult properties = new ScopedCredentialsResult( new EnumMap<>( diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BaseResolverTest.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BaseResolverTest.java index 728187a38d..1a7a827315 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BaseResolverTest.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BaseResolverTest.java @@ -468,7 +468,9 @@ private Resolver allocateResolver( // create a new cache if needs be if (cache == null) { - this.cache = new InMemoryEntityCache(metaStoreManager()); + this.cache = + new InMemoryEntityCache( + callCtx().getRealmContext(), callCtx().getConfigurationStore(), metaStoreManager()); } boolean allRoles = principalRolesScope == null; Optional> roleEntities = diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java index 593853c409..e1b197005a 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java @@ -95,8 +95,9 @@ public Clock clock() { @Produces @ApplicationScoped - public StorageCredentialCache storageCredentialCache() { - return new StorageCredentialCache(); + public StorageCredentialCache storageCredentialCache( + RealmContext realmContext, PolarisConfigurationStore configurationStore) { + return new StorageCredentialCache(realmContext, configurationStore); } @Produces diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 88f54daadb..824a4995ab 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -258,7 +258,9 @@ public void before(TestInfo testInfo) { entityManager = new PolarisEntityManager( - metaStoreManager, new StorageCredentialCache(), createEntityCache(metaStoreManager)); + metaStoreManager, + new StorageCredentialCache(realmContext, configurationStore), + createEntityCache(metaStoreManager)); PrincipalEntity rootEntity = new PrincipalEntity( @@ -424,7 +426,7 @@ public Supplier getOrCreateSessionSupplier( @Override public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext realmContext) { - return new StorageCredentialCache(); + return new StorageCredentialCache(realmContext, configurationStore); } @Override diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java index 7ff4ee5e7c..9dc5a4fe85 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java @@ -179,7 +179,7 @@ public void before(TestInfo testInfo) { PolarisEntityManager entityManager = new PolarisEntityManager( metaStoreManager, - new StorageCredentialCache(), + new StorageCredentialCache(realmContext, configurationStore), new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); CallContext.setCurrentContext(polarisContext); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java index 1d1d4c4d7c..24580aedc8 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java @@ -177,7 +177,7 @@ public void before(TestInfo testInfo) { entityManager = new PolarisEntityManager( metaStoreManager, - new StorageCredentialCache(), + new StorageCredentialCache(realmContext, configurationStore), new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); PrincipalEntity rootEntity = @@ -302,7 +302,7 @@ public Supplier getOrCreateSessionSupplier( @Override public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext realmContext) { - return new StorageCredentialCache(); + return new StorageCredentialCache(realmContext, configurationStore); } @Override diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java index fc9c69cece..32a27d6c28 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java @@ -203,7 +203,7 @@ public void before(TestInfo testInfo) { entityManager = new PolarisEntityManager( metaStoreManager, - new StorageCredentialCache(), + new StorageCredentialCache(realmContext, configurationStore), new InMemoryEntityCache(realmContext, configurationStore, metaStoreManager)); callContext = polarisContext; @@ -326,7 +326,7 @@ public Supplier getOrCreateSessionSupplier( @Override public StorageCredentialCache getOrCreateStorageCredentialCache(RealmContext realmContext) { - return new StorageCredentialCache(); + return new StorageCredentialCache(realmContext, configurationStore); } @Override From f4dc2682d0c9a3a7222c16be059231616b54c1f3 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 10 Jun 2025 21:05:50 -0700 Subject: [PATCH 4/7] check in --- .../AtomicOperationMetaStoreManager.java | 2 +- .../TransactionalMetaStoreManagerImpl.java | 2 +- .../storage/PolarisStorageIntegration.java | 6 ++--- .../aws/AwsCredentialsStorageIntegration.java | 12 ++++++---- .../AzureCredentialsStorageIntegration.java | 12 ++++++---- .../gcp/GcpCredentialsStorageIntegration.java | 4 ++-- .../InMemoryStorageIntegrationTest.java | 3 +-- .../AwsCredentialsStorageIntegrationTest.java | 22 +++++++++---------- ...AzureCredentialStorageIntegrationTest.java | 5 +++-- .../GcpCredentialsStorageIntegrationTest.java | 5 +++-- ...PolarisStorageIntegrationProviderImpl.java | 4 ++-- 11 files changed, 43 insertions(+), 34 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 060a908e15..4f5a98ce3c 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 @@ -1619,7 +1619,7 @@ private void revokeGrantRecord( try { EnumMap creds = storageIntegration.getSubscopedCreds( - callCtx.getDiagServices(), + callCtx, storageConfigurationInfo, allowListOperation, allowedReadLocations, 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 0d6111d54b..71686165dd 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 @@ -2053,7 +2053,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( try { EnumMap creds = storageIntegration.getSubscopedCreds( - callCtx.getDiagServices(), + callCtx, storageConfigurationInfo, allowListOperation, allowedReadLocations, 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 e549dd1f8a..147ec5c66e 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 @@ -22,7 +22,7 @@ import java.util.EnumMap; import java.util.Map; import java.util.Set; -import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.context.CallContext; /** * Abstract of Polaris Storage Integration. It holds the reference to an object that having the @@ -45,7 +45,7 @@ public String getStorageIdentifierOrId() { /** * Subscope the creds against the allowed read and write locations. * - * @param diagnostics the diagnostics service + * @param callContext the call context * @param storageConfig storage configuration * @param allowListOperation whether to allow LIST on all the provided allowed read/write * locations @@ -54,7 +54,7 @@ public String getStorageIdentifierOrId() { * @return An enum map including the scoped credentials */ public abstract EnumMap getSubscopedCreds( - @Nonnull PolarisDiagnostics diagnostics, + @Nonnull CallContext callContext, @Nonnull T storageConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, 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 bc8f5e33c5..04c1970dd9 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 @@ -19,7 +19,6 @@ package org.apache.polaris.core.storage.aws; import static org.apache.polaris.core.config.FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS; -import static org.apache.polaris.core.config.PolarisConfiguration.loadConfig; import jakarta.annotation.Nonnull; import java.net.URI; @@ -29,7 +28,7 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Stream; -import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.storage.InMemoryStorageIntegration; import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.core.storage.StorageUtil; @@ -63,11 +62,16 @@ public AwsCredentialsStorageIntegration( /** {@inheritDoc} */ @Override public EnumMap getSubscopedCreds( - @Nonnull PolarisDiagnostics diagnostics, + @Nonnull CallContext callContext, @Nonnull AwsStorageConfigurationInfo storageConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, @Nonnull Set allowedWriteLocations) { + int storageCredentialDurationSeconds = + callContext + .getPolarisCallContext() + .getConfigurationStore() + .getConfiguration(callContext.getRealmContext(), STORAGE_CREDENTIAL_DURATION_SECONDS); AssumeRoleRequest.Builder request = AssumeRoleRequest.builder() .externalId(storageConfig.getExternalId()) @@ -80,7 +84,7 @@ public EnumMap getSubscopedCreds( allowedReadLocations, allowedWriteLocations) .toJson()) - .durationSeconds(loadConfig(STORAGE_CREDENTIAL_DURATION_SECONDS)); + .durationSeconds(storageCredentialDurationSeconds); credentialsProvider.ifPresent( cp -> request.overrideConfiguration(b -> b.credentialsProvider(cp))); AssumeRoleResponse response = stsClient.assumeRole(request.build()); 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 bcbc91a5c8..39bd363d49 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 @@ -18,6 +18,8 @@ */ package org.apache.polaris.core.storage.azure; +import static org.apache.polaris.core.config.FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS; + import com.azure.core.credential.AccessToken; import com.azure.core.credential.TokenRequestContext; import com.azure.identity.DefaultAzureCredential; @@ -45,8 +47,7 @@ import java.util.HashSet; import java.util.Objects; import java.util.Set; -import org.apache.polaris.core.PolarisDiagnostics; -import org.apache.polaris.core.config.FeatureConfiguration; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.storage.InMemoryStorageIntegration; import org.apache.polaris.core.storage.StorageAccessProperty; import org.slf4j.Logger; @@ -71,7 +72,7 @@ public AzureCredentialsStorageIntegration() { @Override public EnumMap getSubscopedCreds( - @Nonnull PolarisDiagnostics diagnostics, + @Nonnull CallContext callContext, @Nonnull AzureStorageConfigurationInfo storageConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, @@ -126,7 +127,10 @@ public EnumMap getSubscopedCreds( // clock skew between the client and server, OffsetDateTime startTime = start.truncatedTo(ChronoUnit.SECONDS).atOffset(ZoneOffset.UTC); int intendedDurationSeconds = - FeatureConfiguration.loadConfig(FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS); + callContext + .getPolarisCallContext() + .getConfigurationStore() + .getConfiguration(callContext.getRealmContext(), STORAGE_CREDENTIAL_DURATION_SECONDS); OffsetDateTime intendedEndTime = start.plusSeconds(intendedDurationSeconds).atOffset(ZoneOffset.UTC); OffsetDateTime maxAllowedEndTime = 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 3a7c85780d..2fc1438fe2 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 @@ -38,7 +38,7 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Stream; -import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.storage.InMemoryStorageIntegration; import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.StorageAccessProperty; @@ -70,7 +70,7 @@ public GcpCredentialsStorageIntegration( @Override public EnumMap getSubscopedCreds( - @Nonnull PolarisDiagnostics diagnostics, + @Nonnull CallContext callContext, @Nonnull GcpStorageConfigurationInfo storageConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, 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 a4e58860de..e679d3e32a 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 @@ -27,7 +27,6 @@ import java.util.Set; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; -import org.apache.polaris.core.PolarisDiagnostics; import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.context.RealmContext; @@ -209,7 +208,7 @@ public MockInMemoryStorageIntegration() { @Override public EnumMap getSubscopedCreds( - @Nonnull PolarisDiagnostics diagnostics, + @Nonnull CallContext callContext, @Nonnull PolarisStorageConfigurationInfo storageConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, 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 b837033e17..26c18e6cdb 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 @@ -25,7 +25,7 @@ import java.util.EnumMap; import java.util.List; import java.util.Set; -import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.core.storage.aws.AwsCredentialsStorageIntegration; @@ -83,7 +83,7 @@ public void testGetSubscopedCreds() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(warehouseDir), @@ -231,7 +231,7 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -248,7 +248,7 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -349,7 +349,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutList() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -444,7 +444,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutWrites() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -509,7 +509,7 @@ public void testGetSubscopedCredsInlinePolicyWithEmptyReadAndWrite() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -549,7 +549,7 @@ public void testClientRegion(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -566,7 +566,7 @@ public void testClientRegion(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -604,7 +604,7 @@ public void testNoClientRegion(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -622,7 +622,7 @@ public void testNoClientRegion(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(PolarisDiagnostics.class), + Mockito.mock(CallContext.class), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), 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 f4738d73e1..e425b3213a 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 @@ -47,7 +47,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Stream; -import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.core.storage.azure.AzureCredentialsStorageIntegration; import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo; @@ -60,6 +60,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; +import org.mockito.Mockito; public class AzureCredentialStorageIntegrationTest { @@ -349,7 +350,7 @@ private Map subscopedCredsForOperations( new AzureCredentialsStorageIntegration(); EnumMap credsMap = azureCredsIntegration.getSubscopedCreds( - new PolarisDefaultDiagServiceImpl(), + Mockito.mock(CallContext.class), azureConfig, allowListAction, new HashSet<>(allowedReadLoc), 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 fc88bd8170..d823f481be 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 @@ -48,7 +48,7 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.core.storage.gcp.GcpCredentialsStorageIntegration; import org.apache.polaris.core.storage.gcp.GcpStorageConfigurationInfo; @@ -58,6 +58,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mockito; class GcpCredentialsStorageIntegrationTest { @@ -171,7 +172,7 @@ private Map subscopedCredsForOperations( ServiceOptions.getFromServiceLoader(HttpTransportFactory.class, NetHttpTransport::new)); EnumMap credsMap = gcpCredsIntegration.getSubscopedCreds( - new PolarisDefaultDiagServiceImpl(), + Mockito.mock(CallContext.class), gcpConfig, allowListAction, new HashSet<>(allowedReadLoc), diff --git a/service/common/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java b/service/common/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java index 2d84e3747d..253ab5d03a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java +++ b/service/common/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java @@ -31,7 +31,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Supplier; -import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageIntegration; @@ -99,7 +99,7 @@ public PolarisStorageIntegrationProviderImpl( new PolarisStorageIntegration<>("file") { @Override public EnumMap getSubscopedCreds( - @Nonnull PolarisDiagnostics diagnostics, + @Nonnull CallContext callContext, @Nonnull T storageConfig, boolean allowListOperation, @Nonnull Set allowedReadLocations, From 18e24ab8737e1604d28223e6c2b13ab049dbbf06 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 10 Jun 2025 21:17:15 -0700 Subject: [PATCH 5/7] stable --- .../storage/BaseStorageIntegrationTest.java | 33 +++++++++++++++++++ .../AwsCredentialsStorageIntegrationTest.java | 24 +++++++------- ...AzureCredentialStorageIntegrationTest.java | 7 ++-- .../GcpCredentialsStorageIntegrationTest.java | 7 ++-- 4 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java new file mode 100644 index 0000000000..295434b9d6 --- /dev/null +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.core.storage; + +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.persistence.BasePersistence; +import org.mockito.Mockito; + +public abstract class BaseStorageIntegrationTest { + protected CallContext getCallContext() { + return new PolarisCallContext( + () -> "realm", Mockito.mock(BasePersistence.class), Mockito.mock(PolarisDiagnostics.class)); + } +} 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 26c18e6cdb..2b87f04a66 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 @@ -25,7 +25,7 @@ import java.util.EnumMap; import java.util.List; import java.util.Set; -import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.storage.BaseStorageIntegrationTest; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.core.storage.aws.AwsCredentialsStorageIntegration; @@ -48,7 +48,7 @@ import software.amazon.awssdk.services.sts.model.AssumeRoleResponse; import software.amazon.awssdk.services.sts.model.Credentials; -class AwsCredentialsStorageIntegrationTest { +class AwsCredentialsStorageIntegrationTest extends BaseStorageIntegrationTest { public static final Instant EXPIRE_TIME = Instant.now().plusMillis(3600_000); @@ -83,7 +83,7 @@ public void testGetSubscopedCreds() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(warehouseDir), @@ -231,7 +231,7 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -248,7 +248,7 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -349,7 +349,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutList() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -444,7 +444,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutWrites() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -509,7 +509,7 @@ public void testGetSubscopedCredsInlinePolicyWithEmptyReadAndWrite() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -549,7 +549,7 @@ public void testClientRegion(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -566,7 +566,7 @@ public void testClientRegion(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -604,7 +604,7 @@ public void testNoClientRegion(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -622,7 +622,7 @@ public void testNoClientRegion(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), 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 e425b3213a..92744f5d51 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 @@ -47,7 +47,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Stream; -import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.storage.BaseStorageIntegrationTest; import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.core.storage.azure.AzureCredentialsStorageIntegration; import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo; @@ -60,9 +60,8 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; -import org.mockito.Mockito; -public class AzureCredentialStorageIntegrationTest { +public class AzureCredentialStorageIntegrationTest extends BaseStorageIntegrationTest { private final String clientId = System.getenv("AZURE_CLIENT_ID"); private final String clientSecret = System.getenv("AZURE_CLIENT_SECRET"); @@ -350,7 +349,7 @@ private Map subscopedCredsForOperations( new AzureCredentialsStorageIntegration(); EnumMap credsMap = azureCredsIntegration.getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), azureConfig, allowListAction, new HashSet<>(allowedReadLoc), 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 d823f481be..054caa1bbd 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 @@ -48,7 +48,7 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.storage.BaseStorageIntegrationTest; import org.apache.polaris.core.storage.StorageAccessProperty; import org.apache.polaris.core.storage.gcp.GcpCredentialsStorageIntegration; import org.apache.polaris.core.storage.gcp.GcpStorageConfigurationInfo; @@ -58,9 +58,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -import org.mockito.Mockito; -class GcpCredentialsStorageIntegrationTest { +class GcpCredentialsStorageIntegrationTest extends BaseStorageIntegrationTest { private final String gcsServiceKeyJsonFileLocation = System.getenv("GOOGLE_APPLICATION_CREDENTIALS"); @@ -172,7 +171,7 @@ private Map subscopedCredsForOperations( ServiceOptions.getFromServiceLoader(HttpTransportFactory.class, NetHttpTransport::new)); EnumMap credsMap = gcpCredsIntegration.getSubscopedCreds( - Mockito.mock(CallContext.class), + getCallContext(), gcpConfig, allowListAction, new HashSet<>(allowedReadLoc), From 5040ce8cfb58a76bec20e19000408278d1ecc969 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 11 Jun 2025 08:47:58 -0700 Subject: [PATCH 6/7] applying fixes from review --- .../core/config/PolarisConfiguration.java | 20 ------------- .../polaris/core/entity/CatalogEntity.java | 30 +++++++++++++++++-- .../storage/BaseStorageIntegrationTest.java | 2 +- .../cache/StorageCredentialCacheTest.java | 18 +++++------ .../AwsCredentialsStorageIntegrationTest.java | 20 ++++++------- ...AzureCredentialStorageIntegrationTest.java | 2 +- .../GcpCredentialsStorageIntegrationTest.java | 2 +- .../quarkus/catalog/IcebergCatalogTest.java | 7 +++-- .../quarkus/entity/CatalogEntityTest.java | 24 ++++++++------- .../service/admin/PolarisAdminService.java | 2 +- 10 files changed, 69 insertions(+), 58 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java index adb66daf55..31ae187955 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java @@ -24,7 +24,6 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.apache.polaris.core.context.CallContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -210,25 +209,6 @@ public BehaviorChangeConfiguration buildBehaviorChangeConfiguration() { } } - /** - * Returns the value of a `PolarisConfiguration`, or the default if it cannot be loaded. This - * method does not need to be used when a `CallContext` is already available - */ - public static T loadConfig(PolarisConfiguration configuration) { - var callContext = CallContext.getCurrentContext(); - if (callContext == null) { - LOGGER.warn( - String.format( - "Unable to load current call context; using %s = %s", - configuration.key, configuration.defaultValue)); - return configuration.defaultValue; - } - return callContext - .getPolarisCallContext() - .getConfigurationStore() - .getConfiguration(callContext.getRealmContext(), configuration); - } - public static Builder builder() { return new Builder<>(); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java index 45c114a283..354cea4640 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java @@ -23,6 +23,7 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -40,7 +41,9 @@ import org.apache.polaris.core.admin.model.GcpStorageConfigInfo; import org.apache.polaris.core.admin.model.PolarisCatalog; import org.apache.polaris.core.admin.model.StorageConfigInfo; +import org.apache.polaris.core.config.BehaviorChangeConfiguration; import org.apache.polaris.core.connection.ConnectionConfigInfoDpo; +import org.apache.polaris.core.context.CallContext; import org.apache.polaris.core.secrets.UserSecretReference; import org.apache.polaris.core.storage.FileStorageConfigurationInfo; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; @@ -78,7 +81,7 @@ public static CatalogEntity of(PolarisBaseEntity sourceEntity) { return null; } - public static CatalogEntity fromCatalog(Catalog catalog) { + public static CatalogEntity fromCatalog(CallContext callContext, Catalog catalog) { Builder builder = new Builder() .setName(catalog.getName()) @@ -88,7 +91,7 @@ public static CatalogEntity fromCatalog(Catalog catalog) { internalProperties.put(CATALOG_TYPE_PROPERTY, catalog.getType().name()); builder.setInternalProperties(internalProperties); builder.setStorageConfigurationInfo( - catalog.getStorageConfigInfo(), getDefaultBaseLocation(catalog)); + callContext, catalog.getStorageConfigInfo(), getDefaultBaseLocation(catalog)); return builder.build(); } @@ -245,7 +248,9 @@ public Builder setReplaceNewLocationPrefixWithCatalogDefault(String value) { } public Builder setStorageConfigurationInfo( - StorageConfigInfo storageConfigModel, String defaultBaseLocation) { + CallContext callContext, + StorageConfigInfo storageConfigModel, + String defaultBaseLocation) { if (storageConfigModel != null) { PolarisStorageConfigurationInfo config; Set allowedLocations = new HashSet<>(storageConfigModel.getAllowedLocations()); @@ -259,6 +264,7 @@ public Builder setStorageConfigurationInfo( throw new BadRequestException("Must specify default base location"); } allowedLocations.add(defaultBaseLocation); + validateMaxAllowedLocations(callContext, allowedLocations); switch (storageConfigModel.getStorageType()) { case S3: AwsStorageConfigInfo awsConfigModel = (AwsStorageConfigInfo) storageConfigModel; @@ -301,6 +307,24 @@ public Builder setStorageConfigurationInfo( return this; } + /** Validate the number of allowed locations not exceeding the max value. */ + private void validateMaxAllowedLocations( + CallContext callContext, + Collection allowedLocations) { + int maxAllowedLocations = callContext + .getPolarisCallContext() + .getConfigurationStore() + .getConfiguration( + callContext.getRealmContext(), + BehaviorChangeConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS); + if (maxAllowedLocations != -1 && allowedLocations.size() > maxAllowedLocations) { + throw new IllegalArgumentException( + String.format( + "Number of configured locations (%s) exceeds the limit of %s", + allowedLocations.size(), maxAllowedLocations)); + } + } + public Builder setConnectionConfigInfoDpoWithSecrets( ConnectionConfigInfo connectionConfigurationModel, Map secretReferences) { diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java index 295434b9d6..e008abf74b 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/BaseStorageIntegrationTest.java @@ -26,7 +26,7 @@ import org.mockito.Mockito; public abstract class BaseStorageIntegrationTest { - protected CallContext getCallContext() { + protected CallContext newCallContext() { return new PolarisCallContext( () -> "realm", Mockito.mock(BasePersistence.class), Mockito.mock(PolarisDiagnostics.class)); } 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 5a5e468491..3d889b7657 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 @@ -72,16 +72,16 @@ public StorageCredentialCacheTest() { new TreeMapTransactionalPersistenceImpl(store, Mockito.mock(), RANDOM_SECRETS); callCtx = new PolarisCallContext(() -> "testRealm", metaStore, diagServices); metaStoreManager = Mockito.mock(PolarisMetaStoreManager.class); - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); } - private StorageCredentialCache getStorageCredentialCache() { + private StorageCredentialCache newStorageCredentialCache() { return new StorageCredentialCache(callCtx.getRealmContext(), callCtx.getConfigurationStore()); } @Test public void testBadResult() { - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); ScopedCredentialsResult badResult = new ScopedCredentialsResult( BaseResult.ReturnStatus.SUBSCOPE_CREDS_ERROR, "extra_error_info"); @@ -114,7 +114,7 @@ public void testBadResult() { @Test public void testCacheHit() { - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ false); Mockito.when( @@ -157,7 +157,7 @@ public void testCacheHit() { @RepeatedTest(10) public void testCacheEvict() throws InterruptedException { - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ true); Mockito.when( @@ -215,7 +215,7 @@ public void testCacheEvict() throws InterruptedException { @Test public void testCacheGenerateNewEntries() { - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ false); Mockito.when( @@ -302,7 +302,7 @@ public void testCacheGenerateNewEntries() { @Test public void testCacheNotAffectedBy() { - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); List mockedScopedCreds = getFakeScopedCreds(3, /* expireSoon= */ false); @@ -447,7 +447,7 @@ private static List getPolarisEntities() { @Test public void testAzureCredentialFormatting() { - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); List mockedScopedCreds = List.of( new ScopedCredentialsResult( @@ -536,7 +536,7 @@ public void testAzureCredentialFormatting() { @Test public void testExtraProperties() { - storageCredentialCache = getStorageCredentialCache(); + storageCredentialCache = newStorageCredentialCache(); ScopedCredentialsResult properties = new ScopedCredentialsResult( new EnumMap<>( 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 2b87f04a66..93f96b3589 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 @@ -83,7 +83,7 @@ public void testGetSubscopedCreds() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(warehouseDir), @@ -231,7 +231,7 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -248,7 +248,7 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -349,7 +349,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutList() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -444,7 +444,7 @@ public void testGetSubscopedCredsInlinePolicyWithoutWrites() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -509,7 +509,7 @@ public void testGetSubscopedCredsInlinePolicyWithEmptyReadAndWrite() { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -549,7 +549,7 @@ public void testClientRegion(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -566,7 +566,7 @@ public void testClientRegion(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -604,7 +604,7 @@ public void testNoClientRegion(String awsPartition) { EnumMap credentials = new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), @@ -622,7 +622,7 @@ public void testNoClientRegion(String awsPartition) { () -> new AwsCredentialsStorageIntegration(stsClient) .getSubscopedCreds( - getCallContext(), + newCallContext(), new AwsStorageConfigurationInfo( PolarisStorageConfigurationInfo.StorageType.S3, List.of(s3Path(bucket, warehouseKeyPrefix)), 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 92744f5d51..9f43d42bd1 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 @@ -349,7 +349,7 @@ private Map subscopedCredsForOperations( new AzureCredentialsStorageIntegration(); EnumMap credsMap = azureCredsIntegration.getSubscopedCreds( - getCallContext(), + newCallContext(), azureConfig, allowListAction, new HashSet<>(allowedReadLoc), 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 054caa1bbd..861ad43acf 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 @@ -171,7 +171,7 @@ private Map subscopedCredsForOperations( ServiceOptions.getFromServiceLoader(HttpTransportFactory.class, NetHttpTransport::new)); EnumMap credsMap = gcpCredsIntegration.getSubscopedCreds( - getCallContext(), + newCallContext(), gcpConfig, allowListAction, new HashSet<>(allowedReadLoc), diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index 824a4995ab..d87656c201 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -211,9 +211,8 @@ public Map getConfigOverrides() { @Inject PolarisDiagnostics diagServices; @Inject PolarisEventListener polarisEventListener; - protected String realmName; - private IcebergCatalog catalog; + private String realmName; private PolarisMetaStoreManager metaStoreManager; private UserSecretsManager userSecretsManager; private PolarisCallContext polarisContext; @@ -226,6 +225,10 @@ public Map getConfigOverrides() { private TestPolarisEventListener testPolarisEventListener; private ReservedProperties reservedProperties; + protected String getRealmName() { + return realmName; + } + @BeforeAll public static void setUpMocks() { PolarisStorageIntegrationProviderImpl mock = diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java index 0a6dea8fed..752f0897be 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java @@ -35,14 +35,17 @@ import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; public class CatalogEntityTest { - @BeforeAll - public static void setup() { + private CallContext callContext; + + @BeforeEach + public void setup() { MetaStoreManagerFactory metaStoreManagerFactory = new InMemoryPolarisMetaStoreManagerFactory(); RealmContext realmContext = () -> "realm"; PolarisCallContext polarisCallContext = @@ -50,6 +53,7 @@ public static void setup() { realmContext, metaStoreManagerFactory.getOrCreateSessionSupplier(() -> "realm").get(), new PolarisDefaultDiagServiceImpl()); + this.callContext = polarisCallContext; CallContext.setCurrentContext(polarisCallContext); } @@ -72,7 +76,7 @@ public void testInvalidAllowedLocationPrefix() { .setProperties(prop) .setStorageConfigInfo(awsStorageConfigModel) .build(); - Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(awsCatalog)) + Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(callContext, awsCatalog)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining( "Location prefix not allowed: 'unsupportPrefix://mybucket/path', expected prefixes"); @@ -93,7 +97,7 @@ public void testInvalidAllowedLocationPrefix() { new CatalogProperties("abfs://container@storageaccount.blob.windows.net/path")) .setStorageConfigInfo(azureStorageConfigModel) .build(); - Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(azureCatalog)) + Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(callContext, azureCatalog)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Invalid azure location uri unsupportPrefix://mybucket/path"); @@ -110,7 +114,7 @@ public void testInvalidAllowedLocationPrefix() { .setProperties(new CatalogProperties("gs://externally-owned-bucket")) .setStorageConfigInfo(gcpStorageConfigModel) .build(); - Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(gcpCatalog)) + Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(callContext, gcpCatalog)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining( "Location prefix not allowed: 'unsupportPrefix://mybucket/path', expected prefixes"); @@ -142,7 +146,7 @@ public void testExceedMaxAllowedLocations() { .setProperties(prop) .setStorageConfigInfo(awsStorageConfigModel) .build(); - Assertions.assertThatCode(() -> CatalogEntity.fromCatalog(awsCatalog)) + Assertions.assertThatCode(() -> CatalogEntity.fromCatalog(callContext, awsCatalog)) .doesNotThrowAnyException(); } @@ -166,7 +170,7 @@ public void testValidAllowedLocationPrefix() { .setProperties(prop) .setStorageConfigInfo(awsStorageConfigModel) .build(); - Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(awsCatalog)); + Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(callContext, awsCatalog)); basedLocation = "abfs://container@storageaccount.blob.windows.net/path"; prop.put(CatalogEntity.DEFAULT_BASE_LOCATION_KEY, basedLocation); @@ -183,7 +187,7 @@ public void testValidAllowedLocationPrefix() { .setProperties(new CatalogProperties(basedLocation)) .setStorageConfigInfo(azureStorageConfigModel) .build(); - Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(azureCatalog)); + Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(callContext, azureCatalog)); basedLocation = "gs://externally-owned-bucket"; prop.put(CatalogEntity.DEFAULT_BASE_LOCATION_KEY, basedLocation); @@ -199,7 +203,7 @@ public void testValidAllowedLocationPrefix() { .setProperties(new CatalogProperties(basedLocation)) .setStorageConfigInfo(gcpStorageConfigModel) .build(); - Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(gcpCatalog)); + Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(callContext, gcpCatalog)); } @ParameterizedTest @@ -234,7 +238,7 @@ public void testInvalidArn(String roleArn) { expectedMessage = "Invalid role ARN format"; } ; - Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(awsCatalog)) + Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(callContext, awsCatalog)) .isInstanceOf(IllegalArgumentException.class) .hasMessage(expectedMessage); } diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 4495d96fd8..2174a41a1c 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -716,7 +716,7 @@ public PolarisEntity createCatalog(CreateCatalogRequest catalogRequest) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_CATALOG; authorizeBasicRootOperationOrThrow(op); - CatalogEntity entity = CatalogEntity.fromCatalog(catalogRequest.getCatalog()); + CatalogEntity entity = CatalogEntity.fromCatalog(callContext, catalogRequest.getCatalog()); checkArgument(entity.getId() == -1, "Entity to be created must have no ID assigned"); From 762b2307b53134a8c0eabd01188d7548a79b1865 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Wed, 11 Jun 2025 08:55:29 -0700 Subject: [PATCH 7/7] stable --- .../polaris/core/entity/CatalogEntity.java | 20 +++++++++---------- .../quarkus/admin/PolarisAuthzTestBase.java | 2 +- .../IcebergCatalogHandlerAuthzTest.java | 2 +- .../quarkus/catalog/IcebergCatalogTest.java | 6 ++++-- .../catalog/IcebergCatalogViewTest.java | 1 + .../PolarisCatalogWithEntityCacheTest.java | 2 +- .../PolarisGenericTableCatalogTest.java | 3 ++- .../quarkus/catalog/PolicyCatalogTest.java | 3 ++- .../quarkus/entity/CatalogEntityTest.java | 10 ++++++---- .../service/admin/PolarisAdminService.java | 2 +- 10 files changed, 28 insertions(+), 23 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java index 354cea4640..f975ae7392 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java @@ -248,9 +248,7 @@ public Builder setReplaceNewLocationPrefixWithCatalogDefault(String value) { } public Builder setStorageConfigurationInfo( - CallContext callContext, - StorageConfigInfo storageConfigModel, - String defaultBaseLocation) { + CallContext callContext, StorageConfigInfo storageConfigModel, String defaultBaseLocation) { if (storageConfigModel != null) { PolarisStorageConfigurationInfo config; Set allowedLocations = new HashSet<>(storageConfigModel.getAllowedLocations()); @@ -309,14 +307,14 @@ public Builder setStorageConfigurationInfo( /** Validate the number of allowed locations not exceeding the max value. */ private void validateMaxAllowedLocations( - CallContext callContext, - Collection allowedLocations) { - int maxAllowedLocations = callContext - .getPolarisCallContext() - .getConfigurationStore() - .getConfiguration( - callContext.getRealmContext(), - BehaviorChangeConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS); + CallContext callContext, Collection allowedLocations) { + int maxAllowedLocations = + callContext + .getPolarisCallContext() + .getConfigurationStore() + .getConfiguration( + callContext.getRealmContext(), + BehaviorChangeConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS); if (maxAllowedLocations != -1 && allowedLocations.size() > maxAllowedLocations) { throw new IllegalArgumentException( String.format( diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java index 4e8748de55..1c232bf54c 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java @@ -284,7 +284,7 @@ public void before(TestInfo testInfo) { .setName(CATALOG_NAME) .setCatalogType("INTERNAL") .setDefaultBaseLocation(storageLocation) - .setStorageConfigurationInfo(storageConfigModel, storageLocation) + .setStorageConfigurationInfo(callContext, storageConfigModel, storageLocation) .build() .asCatalog())); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java index 5c488809d1..4d89d55234 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogHandlerAuthzTest.java @@ -1726,7 +1726,7 @@ public void testSendNotificationSufficientPrivileges() { new CatalogEntity.Builder() .setName(externalCatalog) .setDefaultBaseLocation(storageLocation) - .setStorageConfigurationInfo(storageConfigModel, storageLocation) + .setStorageConfigurationInfo(callContext, storageConfigModel, storageLocation) .setCatalogType("EXTERNAL") .build() .asCatalog())); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index d87656c201..614fab7255 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -319,7 +319,8 @@ public void before(TestInfo testInfo) { "true") .addProperty( FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "true") - .setStorageConfigurationInfo(storageConfigModel, storageLocation) + .setStorageConfigurationInfo( + polarisContext, storageConfigModel, storageLocation) .build() .asCatalog())); @@ -1596,7 +1597,8 @@ public void testDropTableWithPurgeDisabled() { .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") .addProperty(FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "false") - .setStorageConfigurationInfo(noPurgeStorageConfigModel, storageLocation) + .setStorageConfigurationInfo( + polarisContext, noPurgeStorageConfigModel, storageLocation) .build() .asCatalog())); PolarisPassthroughResolutionView passthroughView = diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java index 9dc5a4fe85..6ea276cc09 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java @@ -224,6 +224,7 @@ public void before(TestInfo testInfo) { .addProperty(FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "true") .setDefaultBaseLocation("file://tmp") .setStorageConfigurationInfo( + polarisContext, new FileStorageConfigInfo( StorageConfigInfo.StorageTypeEnum.FILE, List.of("file://", "/", "*")), "file://tmp") diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java index d3515c7775..3c59dff186 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisCatalogWithEntityCacheTest.java @@ -31,6 +31,6 @@ public class PolarisCatalogWithEntityCacheTest extends IcebergCatalogTest { @Nullable @Override protected InMemoryEntityCache createEntityCache(PolarisMetaStoreManager metaStoreManager) { - return new InMemoryEntityCache(() -> realmName, configurationStore, metaStoreManager); + return new InMemoryEntityCache(() -> getRealmName(), configurationStore, metaStoreManager); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java index 24580aedc8..c9196f1750 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolarisGenericTableCatalogTest.java @@ -233,7 +233,8 @@ public void before(TestInfo testInfo) { "true") .addProperty( FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "true") - .setStorageConfigurationInfo(storageConfigModel, storageLocation) + .setStorageConfigurationInfo( + polarisContext, storageConfigModel, storageLocation) .build() .asCatalog())); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java index 32a27d6c28..c0d4b8b463 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java @@ -259,7 +259,8 @@ public void before(TestInfo testInfo) { .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") - .setStorageConfigurationInfo(storageConfigModel, storageLocation) + .setStorageConfigurationInfo( + polarisContext, storageConfigModel, storageLocation) .build() .asCatalog())); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java index 752f0897be..ed44510a9d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java @@ -34,7 +34,6 @@ import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -170,7 +169,8 @@ public void testValidAllowedLocationPrefix() { .setProperties(prop) .setStorageConfigInfo(awsStorageConfigModel) .build(); - Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(callContext, awsCatalog)); + Assertions.assertThatNoException() + .isThrownBy(() -> CatalogEntity.fromCatalog(callContext, awsCatalog)); basedLocation = "abfs://container@storageaccount.blob.windows.net/path"; prop.put(CatalogEntity.DEFAULT_BASE_LOCATION_KEY, basedLocation); @@ -187,7 +187,8 @@ public void testValidAllowedLocationPrefix() { .setProperties(new CatalogProperties(basedLocation)) .setStorageConfigInfo(azureStorageConfigModel) .build(); - Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(callContext, azureCatalog)); + Assertions.assertThatNoException() + .isThrownBy(() -> CatalogEntity.fromCatalog(callContext, azureCatalog)); basedLocation = "gs://externally-owned-bucket"; prop.put(CatalogEntity.DEFAULT_BASE_LOCATION_KEY, basedLocation); @@ -203,7 +204,8 @@ public void testValidAllowedLocationPrefix() { .setProperties(new CatalogProperties(basedLocation)) .setStorageConfigInfo(gcpStorageConfigModel) .build(); - Assertions.assertThatNoException().isThrownBy(() -> CatalogEntity.fromCatalog(callContext, gcpCatalog)); + Assertions.assertThatNoException() + .isThrownBy(() -> CatalogEntity.fromCatalog(callContext, gcpCatalog)); } @ParameterizedTest diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 2174a41a1c..1a1914ad10 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -898,7 +898,7 @@ private void validateUpdateCatalogDiffOrThrow( } if (updateRequest.getStorageConfigInfo() != null) { updateBuilder.setStorageConfigurationInfo( - updateRequest.getStorageConfigInfo(), defaultBaseLocation); + callContext, updateRequest.getStorageConfigInfo(), defaultBaseLocation); } CatalogEntity updatedEntity = updateBuilder.build();