diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java index 59de973bef..d8ec613207 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java @@ -55,7 +55,7 @@ public static void enforceFeatureEnabledOrThrow( .getConfigurationStore() .getConfiguration(callContext.getRealmContext(), featureConfig); if (!enabled) { - throw new UnsupportedOperationException("Feature not enabled: " + featureConfig.key); + throw new UnsupportedOperationException("Feature not enabled: " + featureConfig.key()); } } @@ -211,7 +211,7 @@ public static void enforceFeatureEnabledOrThrow( .key("STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS") .description( "How long to store storage credentials in the local cache. This should be less than " - + STORAGE_CREDENTIAL_DURATION_SECONDS.key) + + STORAGE_CREDENTIAL_DURATION_SECONDS.key()) .defaultValue(30 * 60) // 30 minutes .buildFeatureConfiguration(); 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 31ae187955..f9cf8192f7 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 @@ -39,9 +39,9 @@ public abstract class PolarisConfiguration { private static final List> allConfigurations = new ArrayList<>(); - public final String key; - public final String description; - public final T defaultValue; + private final String key; + private final String description; + private final T defaultValue; private final Optional catalogConfigImpl; private final Optional catalogConfigUnsafeImpl; private final Class typ; @@ -98,22 +98,22 @@ protected PolarisConfiguration( this.typ = (Class) defaultValue.getClass(); } - public boolean hasCatalogConfig() { + public final boolean hasCatalogConfig() { return catalogConfigImpl.isPresent(); } - public String catalogConfig() { + public final String catalogConfig() { return catalogConfigImpl.orElseThrow( () -> new IllegalStateException( "Attempted to read a catalog config key from a configuration that doesn't have one.")); } - public boolean hasCatalogConfigUnsafe() { + public final boolean hasCatalogConfigUnsafe() { return catalogConfigUnsafeImpl.isPresent(); } - public String catalogConfigUnsafe() { + public final String catalogConfigUnsafe() { return catalogConfigUnsafeImpl.orElseThrow( () -> new IllegalStateException( @@ -124,6 +124,18 @@ T cast(Object value) { return this.typ.cast(value); } + public final String key() { + return key; + } + + public final String description() { + return description; + } + + public final T defaultValue() { + return defaultValue; + } + public static class Builder { private String key; private String description; diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java index 4923d97eea..1e2a1928d7 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java @@ -71,18 +71,18 @@ public interface PolarisConfigurationStore { */ private @Nonnull T tryCast(PolarisConfiguration config, Object value) { if (value == null) { - return config.defaultValue; + return config.defaultValue(); } - if (config.defaultValue instanceof Boolean) { + if (config.defaultValue() instanceof Boolean) { return config.cast(Boolean.valueOf(String.valueOf(value))); - } else if (config.defaultValue instanceof Integer) { + } else if (config.defaultValue() instanceof Integer) { return config.cast(Integer.valueOf(String.valueOf(value))); - } else if (config.defaultValue instanceof Long) { + } else if (config.defaultValue() instanceof Long) { return config.cast(Long.valueOf(String.valueOf(value))); - } else if (config.defaultValue instanceof Double) { + } else if (config.defaultValue() instanceof Double) { return config.cast(Double.valueOf(String.valueOf(value))); - } else if (config.defaultValue instanceof List) { + } else if (config.defaultValue() instanceof List) { return config.cast(new ArrayList<>((List) value)); } else { return config.cast(value); @@ -99,7 +99,7 @@ public interface PolarisConfigurationStore { */ default @Nonnull T getConfiguration( @Nonnull RealmContext realmContext, PolarisConfiguration config) { - T result = getConfiguration(realmContext, config.key, config.defaultValue); + T result = getConfiguration(realmContext, config.key(), config.defaultValue()); return tryCast(config, result); } 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 36f666db04..e3a7a4f13f 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 @@ -91,8 +91,8 @@ private long maxCacheDurationMs() { throw new IllegalArgumentException( String.format( "%s should be less than %s", - FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS.key, - FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS.key)); + FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS.key(), + FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS.key())); } else { return cacheDurationSeconds * 1000L; } diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java index 4b190d3d15..612b8716bf 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java @@ -56,8 +56,8 @@ public void testConfigsCanBeCastedFromString() { @Override public @Nullable T getConfiguration(@Nonnull RealmContext ctx, String configName) { for (PolarisConfiguration c : configs) { - if (c.key.equals(configName)) { - return (T) String.valueOf(c.defaultValue); + if (c.key().equals(configName)) { + return (T) String.valueOf(c.defaultValue()); } } @@ -71,7 +71,7 @@ public void testConfigsCanBeCastedFromString() { // Ensure that we can fetch all the configs and that the value is what we expect, which // is the config's default value based on how we've implemented PolarisConfigurationStore above. for (PolarisConfiguration c : configs) { - Assertions.assertEquals(c.defaultValue, store.getConfiguration(testRealmContext, c)); + Assertions.assertEquals(c.defaultValue(), store.getConfiguration(testRealmContext, c)); } } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java b/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java index ef2c1813e9..4205ef4cfe 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java @@ -217,26 +217,26 @@ public ProductionReadinessCheck checkInsecureStorageSettings( var insecure = FeatureConfiguration.ALLOW_INSECURE_STORAGE_TYPES; var errors = new ArrayList(); - if (Boolean.parseBoolean(featureConfiguration.defaults().get(insecure.key))) { + if (Boolean.parseBoolean(featureConfiguration.defaults().get(insecure.key()))) { errors.add( Error.ofSevere( "Must not enable a configuration that exposes known and severe security risks: " - + insecure.description, - format("polaris.features.\"%s\"", insecure.key))); + + insecure.description(), + format("polaris.features.\"%s\"", insecure.key()))); } featureConfiguration .realmOverrides() .forEach( (realmId, overrides) -> { - if (Boolean.parseBoolean(overrides.overrides().get(insecure.key))) { + if (Boolean.parseBoolean(overrides.overrides().get(insecure.key()))) { errors.add( Error.ofSevere( "Must not enable a configuration that exposes known and severe security risks: " - + insecure.description, + + insecure.description(), format( "polaris.features.realm-overrides.\"%s\".overrides.\"%s\"", - realmId, insecure.key))); + realmId, insecure.key()))); } }); @@ -245,7 +245,7 @@ public ProductionReadinessCheck checkInsecureStorageSettings( var defaults = featureConfiguration.parseDefaults(mapper); var realmOverrides = featureConfiguration.parseRealmOverrides(mapper); @SuppressWarnings("unchecked") - var supported = (List) defaults.getOrDefault(storageTypes.key, List.of()); + var supported = (List) defaults.getOrDefault(storageTypes.key(), List.of()); supported.stream() .filter(n -> !IcebergPropertiesValidation.safeStorageType(n)) .forEach( @@ -255,11 +255,11 @@ public ProductionReadinessCheck checkInsecureStorageSettings( format( "The storage type '%s' is considered insecure and exposes the service to severe security risks!", t), - format("polaris.features.\"%s\"", storageTypes.key)))); + format("polaris.features.\"%s\"", storageTypes.key())))); realmOverrides.forEach( (realmId, overrides) -> { @SuppressWarnings("unchecked") - var s = (List) overrides.getOrDefault(storageTypes.key, List.of()); + var s = (List) overrides.getOrDefault(storageTypes.key(), List.of()); s.stream() .filter(n -> !IcebergPropertiesValidation.safeStorageType(n)) .forEach( @@ -271,7 +271,7 @@ public ProductionReadinessCheck checkInsecureStorageSettings( t), format( "polaris.features.realm-overrides.\"%s\".overrides.\"%s\"", - realmId, storageTypes.key)))); + realmId, storageTypes.key())))); }); return errors.isEmpty() ? ProductionReadinessCheck.OK diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java index a658b8e882..2245bb9e9b 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java @@ -340,7 +340,7 @@ void testInvalidSetupsForObjectStorageLocation( "true", "SUPPORTED_CATALOG_STORAGE_TYPES", List.of("FILE", "S3"), - OPTIMIZED_SIBLING_CHECK.key, + OPTIMIZED_SIBLING_CHECK.key(), "false"); TestServices services = @@ -369,7 +369,7 @@ public void testHashedTableLocations(@TempDir Path tempDir) { "true", "SUPPORTED_CATALOG_STORAGE_TYPES", List.of("FILE", "S3"), - OPTIMIZED_SIBLING_CHECK.key, + OPTIMIZED_SIBLING_CHECK.key(), "true"); Map hashedAndOverlapButNoOptimizedCatalog = Map.of( diff --git a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java index a5f4f90800..8534e18eaf 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java @@ -1956,7 +1956,7 @@ public void testDropTableWithPurgeDisabled() { // Attempt to drop the table: Assertions.assertThatThrownBy(() -> noPurgeCatalog.dropTable(TABLE, true)) .isInstanceOf(ForbiddenException.class) - .hasMessageContaining(FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key); + .hasMessageContaining(FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key()); } private TableMetadata createSampleTableMetadata(String tableLocation) { diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index e1b317a91f..5c0e6b18d3 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -949,8 +949,8 @@ private String applyDefaultLocationObjectStoragePrefix( throw new IllegalStateException( String.format( "The configuration %s is enabled, but %s is not enabled", - FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key, - FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key)); + FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key(), + FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key())); } else if (!allowTableLocationOverlap) { // TODO consider doing this check any time ALLOW_EXTERNAL_TABLE_LOCATION is enabled, not just // here @@ -961,17 +961,17 @@ private String applyDefaultLocationObjectStoragePrefix( + " performed, but only within each namespace. However, %s is enabled, which indicates" + " that tables may be created outside of their parent namespace. This is not a safe" + " combination of configurations.", - FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key, - FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key, - FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key)); + FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key(), + FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key(), + FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key())); } else if (!loggedPrefixOverlapWarning.getAndSet(true)) { LOGGER.warn( "A table is being created with {} and {} enabled, but with {} disabled. " + "This is a safe combination of configurations which may prevent table overlap, but only if the " + "underlying persistence actually implements %s. Exercise caution.", - FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key, - FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key, - FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key); + FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key(), + FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key(), + FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key()); } return buildPrefixedLocation(tableIdentifier); } else { @@ -2460,7 +2460,7 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) { "Unable to purge entity: %s. To enable this feature, set the Polaris configuration %s " + "or the catalog configuration %s", identifier.name(), - FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key, + FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key(), FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig())); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java index 0e62110765..db3771b96c 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java @@ -88,8 +88,8 @@ public static AccessConfig refreshAccessConfig( boolean skipCredentialSubscopingIndirection = configurationStore.getConfiguration( callContext.getRealmContext(), - FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key, - FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue); + FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key(), + FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue()); if (skipCredentialSubscopingIndirection) { LOGGER .atDebug()