From 21df5a5061811b506a043dc6da9f05aa11d38b0c Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 11:10:51 -0700 Subject: [PATCH 01/27] double WithParentName --- .../quarkus/config/QuarkusFeaturesConfiguration.java | 11 ++++++++++- service/common/build.gradle.kts | 1 + .../polaris/service/config/FeaturesConfiguration.java | 3 +++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java index 2ae58c8fe7..44111b1c2b 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java @@ -20,6 +20,8 @@ import io.smallrye.config.ConfigMapping; import java.util.Map; + +import io.smallrye.config.WithParentName; import org.apache.polaris.service.config.FeaturesConfiguration; @ConfigMapping(prefix = "polaris.features") @@ -28,6 +30,13 @@ public interface QuarkusFeaturesConfiguration extends FeaturesConfiguration { @Override Map defaults(); + @WithParentName @Override - Map realmOverrides(); + Map realmOverrides(); + + interface QuarkusRealmOverrides extends RealmOverrides { + @WithParentName + @Override + Map overrides(); + } } diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts index d38838a85c..7997729a13 100644 --- a/service/common/build.gradle.kts +++ b/service/common/build.gradle.kts @@ -59,6 +59,7 @@ dependencies { implementation(libs.jakarta.ws.rs.api) implementation(libs.smallrye.common.annotation) + implementation(libs.smallrye.config.core) implementation(platform(libs.jackson.bom)) implementation("com.fasterxml.jackson.core:jackson-annotations") diff --git a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java index c171598882..c34e48399a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java +++ b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java @@ -21,6 +21,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import io.smallrye.config.WithParentName; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -35,6 +37,7 @@ interface RealmOverrides { Map defaults(); + @WithParentName Map realmOverrides(); default Map parseDefaults(ObjectMapper objectMapper) { From 5a3b827f5f96e8bd325d2317cc7dc64b78314330 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 11:10:59 -0700 Subject: [PATCH 02/27] autolint --- .../service/quarkus/config/QuarkusFeaturesConfiguration.java | 3 +-- .../apache/polaris/service/config/FeaturesConfiguration.java | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java index 44111b1c2b..7c4eabbf76 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java @@ -19,9 +19,8 @@ package org.apache.polaris.service.quarkus.config; import io.smallrye.config.ConfigMapping; -import java.util.Map; - import io.smallrye.config.WithParentName; +import java.util.Map; import org.apache.polaris.service.config.FeaturesConfiguration; @ConfigMapping(prefix = "polaris.features") diff --git a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java index c34e48399a..fc2b7cc801 100644 --- a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java +++ b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import io.smallrye.config.WithParentName; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; From 81d842e1b6e26fb9d4205f97e3e6b197600c1e57 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 11:13:48 -0700 Subject: [PATCH 03/27] Revert some --- service/common/build.gradle.kts | 1 - .../org/apache/polaris/service/config/FeaturesConfiguration.java | 1 - 2 files changed, 2 deletions(-) diff --git a/service/common/build.gradle.kts b/service/common/build.gradle.kts index 7997729a13..d38838a85c 100644 --- a/service/common/build.gradle.kts +++ b/service/common/build.gradle.kts @@ -59,7 +59,6 @@ dependencies { implementation(libs.jakarta.ws.rs.api) implementation(libs.smallrye.common.annotation) - implementation(libs.smallrye.config.core) implementation(platform(libs.jackson.bom)) implementation("com.fasterxml.jackson.core:jackson-annotations") diff --git a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java index fc2b7cc801..d490741000 100644 --- a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java +++ b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java @@ -36,7 +36,6 @@ interface RealmOverrides { Map defaults(); - @WithParentName Map realmOverrides(); default Map parseDefaults(ObjectMapper objectMapper) { From c03b04d6594818c2dbcfb3cf687f1ebbe6ba8b32 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 11:13:50 -0700 Subject: [PATCH 04/27] autolint --- .../org/apache/polaris/service/config/FeaturesConfiguration.java | 1 - 1 file changed, 1 deletion(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java index d490741000..c171598882 100644 --- a/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java +++ b/service/common/src/main/java/org/apache/polaris/service/config/FeaturesConfiguration.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import io.smallrye.config.WithParentName; import java.util.ArrayList; import java.util.HashMap; import java.util.List; From 63213b931bd3731fdd1cbc74f9740359052e9e30 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 11:37:30 -0700 Subject: [PATCH 05/27] add to BCconfigs --- .../config/QuarkusBehaviorChangesConfiguration.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java index 1e4fe946e0..cf7e04ceb4 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java @@ -26,6 +26,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; + +import io.smallrye.config.WithParentName; import org.apache.polaris.service.config.FeaturesConfiguration; @ConfigMapping(prefix = "polaris.behavior-changes") @@ -35,7 +37,14 @@ public interface QuarkusBehaviorChangesConfiguration { Map defaults(); - Map realmOverrides(); + @WithParentName + Map realmOverrides(); + + interface QuarkusRealmOverrides extends FeaturesConfiguration.RealmOverrides { + @WithParentName + @Override + Map overrides(); + } default Map parseDefaults(ObjectMapper objectMapper) { return convertMap(objectMapper, defaults()); From 363250d2da4268fd44fc5b7a2ed846153930043f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 11:37:32 -0700 Subject: [PATCH 06/27] autolint --- .../quarkus/config/QuarkusBehaviorChangesConfiguration.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java index cf7e04ceb4..66bb2c3116 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java @@ -22,12 +22,11 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import io.smallrye.config.ConfigMapping; +import io.smallrye.config.WithParentName; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; - -import io.smallrye.config.WithParentName; import org.apache.polaris.service.config.FeaturesConfiguration; @ConfigMapping(prefix = "polaris.behavior-changes") From 7959c72fe06c0fea645da27c5a232418d71b698f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 13:25:24 -0700 Subject: [PATCH 07/27] yank --- .../quarkus/config/QuarkusBehaviorChangesConfiguration.java | 1 - .../service/quarkus/config/QuarkusFeaturesConfiguration.java | 1 - 2 files changed, 2 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java index 66bb2c3116..1432db7119 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java @@ -36,7 +36,6 @@ public interface QuarkusBehaviorChangesConfiguration { Map defaults(); - @WithParentName Map realmOverrides(); interface QuarkusRealmOverrides extends FeaturesConfiguration.RealmOverrides { diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java index 7c4eabbf76..e4fdfec764 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java @@ -29,7 +29,6 @@ public interface QuarkusFeaturesConfiguration extends FeaturesConfiguration { @Override Map defaults(); - @WithParentName @Override Map realmOverrides(); From 44db4f5301fb555c362283c4afc1b99e31add666 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 13:29:41 -0700 Subject: [PATCH 08/27] copy yuns test --- .../config/DefaultConfigurationStoreTest.java | 150 +++++++++++------- 1 file changed, 96 insertions(+), 54 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index 04f4a8680e..88d9176c95 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -20,15 +20,77 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.time.Clock; import java.util.Map; + +import io.quarkus.test.junit.QuarkusMock; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.QuarkusTestProfile; +import io.quarkus.test.junit.TestProfile; +import jakarta.inject.Inject; 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.persistence.MetaStoreManagerFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; +import org.apache.polaris.service.config.FeaturesConfiguration; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +@QuarkusTest +@TestProfile(DefaultConfigurationStoreTest.Profile.class) public class DefaultConfigurationStoreTest { + private static final String keyOne = "ALLOW_SPECIFYING_FILE_IO_IMPL"; + private static final String keyTwo = "INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST"; + private static final String keyThree = "SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"; + private static final String realmOne = "realm1"; + private static final String realmTwo = "realm2"; + + public static class Profile implements QuarkusTestProfile { + + @Override + public Map getConfigOverrides() { + return Map.of( + "polaris.realm-context.realms", + "realm1,realm2", + String.format("polaris.features.defaults.\"%s\"", keyOne), + "true", + String.format("polaris.features.defaults.\"%s\"", keyTwo), + "false", + String.format("polaris.features.defaults.\"%s\"", keyThree), + "true", + String.format("polaris.features.realm-overrides.\"%s\".\"%s\"", realmOne, keyOne), + "false", + String.format("polaris.features.realm-overrides.\"%s\".\"%s\"", realmTwo, keyTwo), + "true"); + } + } + + private PolarisCallContext polarisContext; + + @Inject FeaturesConfiguration featuresConfiguration; + @Inject MetaStoreManagerFactory managerFactory; + @Inject PolarisConfigurationStore configurationStore; + @Inject PolarisDiagnostics diagServices; + + @BeforeEach + public void before(TestInfo testInfo) { + String realmName = "realm_%s_%s".formatted( + testInfo.getTestMethod().map(java.lang.reflect.Method::getName).orElse("test"), System.nanoTime()); + RealmContext realmContext = () -> realmName; + QuarkusMock.installMockForType(realmContext, RealmContext.class); + polarisContext = + new PolarisCallContext( + managerFactory.getOrCreateSessionSupplier(realmContext).get(), + diagServices, + configurationStore, + Clock.systemDefaultZone()); + } @Test public void testGetConfiguration() { @@ -53,62 +115,42 @@ public void testGetConfiguration() { } @Test - public void testGetRealmConfiguration() { - int defaultKeyOneValue = 1; - String defaultKeyTwoValue = "value"; - - int realm1KeyOneValue = 2; - int realm2KeyOneValue = 3; - String realm2KeyTwoValue = "value3"; - DefaultConfigurationStore defaultConfigurationStore = - new DefaultConfigurationStore( - Map.of("key1", defaultKeyOneValue, "key2", defaultKeyTwoValue), - Map.of( - "realm1", - Map.of("key1", realm1KeyOneValue), - "realm2", - Map.of("key1", realm2KeyOneValue, "key2", realm2KeyTwoValue))); - InMemoryPolarisMetaStoreManagerFactory metastoreFactory = - new InMemoryPolarisMetaStoreManagerFactory(); - - // check realm1 values - PolarisCallContext realm1Ctx = - new PolarisCallContext( - metastoreFactory.getOrCreateSessionSupplier(() -> "realm1").get(), - new PolarisDefaultDiagServiceImpl()); - Object value = - defaultConfigurationStore.getConfiguration(realm1Ctx, "missingKeyWithoutDefault"); + public void testGetDefaultConfigurations() { + Object value = configurationStore.getConfiguration(polarisContext, "missingKeyWithoutDefault"); assertThat(value).isNull(); + Object defaultValue = - defaultConfigurationStore.getConfiguration( - realm1Ctx, "missingKeyWithDefault", "defaultValue"); + configurationStore.getConfiguration( + polarisContext, "missingKeyWithDefault", "defaultValue"); assertThat(defaultValue).isEqualTo("defaultValue"); - CallContext.setCurrentContext(CallContext.of(() -> "realm1", realm1Ctx)); - Integer keyOneRealm1 = defaultConfigurationStore.getConfiguration(realm1Ctx, "key1"); - assertThat(keyOneRealm1).isEqualTo(realm1KeyOneValue); - String keyTwoRealm1 = defaultConfigurationStore.getConfiguration(realm1Ctx, "key2"); - assertThat(keyTwoRealm1).isEqualTo(defaultKeyTwoValue); - - // check realm2 values - PolarisCallContext realm2Ctx = - new PolarisCallContext( - metastoreFactory.getOrCreateSessionSupplier(() -> "realm2").get(), - new PolarisDefaultDiagServiceImpl()); - CallContext.setCurrentContext(CallContext.of(() -> "realm2", realm2Ctx)); - Integer keyOneRealm2 = defaultConfigurationStore.getConfiguration(realm2Ctx, "key1"); - assertThat(keyOneRealm2).isEqualTo(realm2KeyOneValue); - String keyTwoRealm2 = defaultConfigurationStore.getConfiguration(realm2Ctx, "key2"); - assertThat(keyTwoRealm2).isEqualTo(realm2KeyTwoValue); - - // realm3 has no realm-overrides, so just returns default values - PolarisCallContext realm3Ctx = - new PolarisCallContext( - metastoreFactory.getOrCreateSessionSupplier(() -> "realm3").get(), - new PolarisDefaultDiagServiceImpl()); - CallContext.setCurrentContext(CallContext.of(() -> "realm3", realm3Ctx)); - Integer keyOneRealm3 = defaultConfigurationStore.getConfiguration(realm3Ctx, "key1"); - assertThat(keyOneRealm3).isEqualTo(defaultKeyOneValue); - String keyTwoRealm3 = defaultConfigurationStore.getConfiguration(realm3Ctx, "key2"); - assertThat(keyTwoRealm3).isEqualTo(defaultKeyTwoValue); + + boolean keyOneValue = configurationStore.getConfiguration(polarisContext, keyOne); + assertThat(keyOneValue).isTrue(); + boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, keyTwo); + assertThat(keyTwoValue).isFalse(); + boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, keyThree); + assertThat(keyThreeValue).isTrue(); + } + + @Test + public void testGetRealmConfiguration() { + RealmContext realmContext = () -> realmOne; + QuarkusMock.installMockForType(realmContext, RealmContext.class); + + boolean keyOneValue = configurationStore.getConfiguration(polarisContext, keyOne); + assertThat(keyOneValue).isFalse(); + boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, keyTwo); + assertThat(keyTwoValue).isFalse(); + boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, keyThree); + assertThat(keyThreeValue).isTrue(); + + realmContext = () -> realmTwo; + QuarkusMock.installMockForType(realmContext, RealmContext.class); + keyOneValue = configurationStore.getConfiguration(polarisContext, keyOne); + assertThat(keyOneValue).isTrue(); + keyTwoValue = configurationStore.getConfiguration(polarisContext, keyTwo); + assertThat(keyTwoValue).isTrue(); + keyThreeValue = configurationStore.getConfiguration(polarisContext, keyThree); + assertThat(keyThreeValue).isTrue(); } } From 6cd9bc4100c9a6e4f9a0e9e54073c3f73358a474 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 13:29:44 -0700 Subject: [PATCH 09/27] autolint --- .../config/DefaultConfigurationStoreTest.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index 88d9176c95..f7c0f6a195 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -20,19 +20,17 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.time.Clock; -import java.util.Map; - import io.quarkus.test.junit.QuarkusMock; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.QuarkusTestProfile; import io.quarkus.test.junit.TestProfile; import jakarta.inject.Inject; +import java.time.Clock; +import java.util.Map; 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.persistence.MetaStoreManagerFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; @@ -80,8 +78,11 @@ public Map getConfigOverrides() { @BeforeEach public void before(TestInfo testInfo) { - String realmName = "realm_%s_%s".formatted( - testInfo.getTestMethod().map(java.lang.reflect.Method::getName).orElse("test"), System.nanoTime()); + String realmName = + "realm_%s_%s" + .formatted( + testInfo.getTestMethod().map(java.lang.reflect.Method::getName).orElse("test"), + System.nanoTime()); RealmContext realmContext = () -> realmName; QuarkusMock.installMockForType(realmContext, RealmContext.class); polarisContext = From a20984c8c4dfca14ff585d17844b73a61b5b3d3d Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 13:36:11 -0700 Subject: [PATCH 10/27] remove defaults --- helm/polaris/templates/configmap.yaml | 2 +- helm/polaris/tests/configmap_test.yaml | 4 ++-- .../main/resources/application-it.properties | 20 +++++++++---------- .../src/main/resources/application.properties | 8 ++++---- .../QuarkusBehaviorChangesConfiguration.java | 1 + .../config/QuarkusFeaturesConfiguration.java | 1 + .../quarkus/admin/PolarisAuthzTestBase.java | 4 ++-- .../catalog/GenericTableCatalogTest.java | 6 +++--- .../IcebergCatalogHandlerAuthzTest.java | 4 ++-- .../quarkus/catalog/IcebergCatalogTest.java | 8 ++++---- .../catalog/IcebergCatalogViewTest.java | 10 +++++----- .../quarkus/catalog/PolicyCatalogTest.java | 6 +++--- .../config/DefaultConfigurationStoreTest.java | 6 +++--- .../it/QuarkusApplicationIntegrationTest.java | 4 ++-- ...arkusManagementServiceIntegrationTest.java | 4 ++-- .../it/QuarkusRestCatalogIntegrationTest.java | 2 +- ...rkusRestCatalogViewAwsIntegrationTest.java | 2 +- ...usRestCatalogViewAzureIntegrationTest.java | 2 +- ...kusRestCatalogViewFileIntegrationTest.java | 2 +- ...rkusRestCatalogViewGcpIntegrationTest.java | 2 +- .../in-dev/unreleased/configuration.md | 4 ++-- .../configuring-polaris-for-production.md | 2 +- 22 files changed, 53 insertions(+), 51 deletions(-) diff --git a/helm/polaris/templates/configmap.yaml b/helm/polaris/templates/configmap.yaml index 58ba96c0b6..44153a37bd 100644 --- a/helm/polaris/templates/configmap.yaml +++ b/helm/polaris/templates/configmap.yaml @@ -37,7 +37,7 @@ data: {{- /* Features */ -}} {{- range $k, $v := .Values.features.defaults -}} - {{- $_ = set $map (printf "polaris.features.defaults.\"%s\"" $k) (toJson $v) -}} + {{- $_ = set $map (printf "polaris.features.\"%s\"" $k) (toJson $v) -}} {{- end -}} {{- range $realm, $overrides := .Values.features.realmOverrides -}} {{- range $k, $v := $overrides -}} diff --git a/helm/polaris/tests/configmap_test.yaml b/helm/polaris/tests/configmap_test.yaml index ef725ec4f3..3c8f018959 100644 --- a/helm/polaris/tests/configmap_test.yaml +++ b/helm/polaris/tests/configmap_test.yaml @@ -98,8 +98,8 @@ tests: realm2: feature2: 43 asserts: - - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.defaults.\"feature1\"=true" } - - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.defaults.\"feature2\"=42" } + - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.\"feature1\"=true" } + - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.\"feature2\"=42" } - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.config.realm-overrides.\"realm1\".\"feature1\"=false" } - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.config.realm-overrides.\"realm2\".\"feature2\"=43" } diff --git a/quarkus/defaults/src/main/resources/application-it.properties b/quarkus/defaults/src/main/resources/application-it.properties index 4419d4d590..b172114cab 100644 --- a/quarkus/defaults/src/main/resources/application-it.properties +++ b/quarkus/defaults/src/main/resources/application-it.properties @@ -31,16 +31,16 @@ polaris.persistence.type=in-memory polaris.secrets-manager.type=in-memory -polaris.features.defaults."ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING"=false -polaris.features.defaults."ALLOW_EXTERNAL_METADATA_FILE_LOCATION"=false -polaris.features.defaults."ALLOW_OVERLAPPING_CATALOG_URLS"=true -polaris.features.defaults."ALLOW_SPECIFYING_FILE_IO_IMPL"=true -polaris.features.defaults."ALLOW_WILDCARD_LOCATION"=true -polaris.features.defaults."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"=true -polaris.features.defaults."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_it"=true -polaris.features.defaults."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"=true -polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["FILE","S3","GCS","AZURE"] -polaris.features.defaults."ENABLE_CATALOG_FEDERATION"=true +polaris.features."ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING"=false +polaris.features."ALLOW_EXTERNAL_METADATA_FILE_LOCATION"=false +polaris.features."ALLOW_OVERLAPPING_CATALOG_URLS"=true +polaris.features."ALLOW_SPECIFYING_FILE_IO_IMPL"=true +polaris.features."ALLOW_WILDCARD_LOCATION"=true +polaris.features."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"=true +polaris.features."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_it"=true +polaris.features."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"=true +polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"=["FILE","S3","GCS","AZURE"] +polaris.features."ENABLE_CATALOG_FEDERATION"=true polaris.realm-context.realms=POLARIS,OTHER diff --git a/quarkus/defaults/src/main/resources/application.properties b/quarkus/defaults/src/main/resources/application.properties index 11f23de1b8..64f6f24742 100644 --- a/quarkus/defaults/src/main/resources/application.properties +++ b/quarkus/defaults/src/main/resources/application.properties @@ -108,10 +108,10 @@ polaris.realm-context.realms=POLARIS polaris.realm-context.header-name=Polaris-Realm polaris.realm-context.require-header=false -polaris.features.defaults."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"=false -polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE","FILE"] -# polaris.features.defaults."ENABLE_CATALOG_FEDERATION"=true -polaris.features.defaults."SUPPORTED_CATALOG_CONNECTION_TYPES"=["ICEBERG_REST"] +polaris.features."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"=false +polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE","FILE"] +# polaris.features."ENABLE_CATALOG_FEDERATION"=true +polaris.features."SUPPORTED_CATALOG_CONNECTION_TYPES"=["ICEBERG_REST"] # realm overrides # polaris.features.realm-overrides."my-realm"."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST"=true diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java index 1432db7119..8e4d059f5c 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusBehaviorChangesConfiguration.java @@ -34,6 +34,7 @@ // QuarkusFeaturesConfiguration public interface QuarkusBehaviorChangesConfiguration { + @WithParentName Map defaults(); Map realmOverrides(); diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java index e4fdfec764..cbcfbc0576 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusFeaturesConfiguration.java @@ -26,6 +26,7 @@ @ConfigMapping(prefix = "polaris.features") public interface QuarkusFeaturesConfiguration extends FeaturesConfiguration { + @WithParentName @Override Map defaults(); 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 91f0a33d61..b5ef68a6bd 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 @@ -113,9 +113,9 @@ public Set> getEnabledAlternatives() { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", + "polaris.features.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", "true", - "polaris.features.defaults.\"ALLOW_EXTERNAL_METADATA_FILE_LOCATION\"", + "polaris.features.\"ALLOW_EXTERNAL_METADATA_FILE_LOCATION\"", "true"); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java index a296feca64..04c84c5f60 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/GenericTableCatalogTest.java @@ -106,11 +106,11 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", + "polaris.features.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", "true", - "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", + "polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", "true", - "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", + "polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]"); } } 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 08dd790e90..cbfaa41f3b 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 @@ -88,9 +88,9 @@ public static class Profile extends PolarisAuthzTestBase.Profile { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", + "polaris.features.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", "true", - "polaris.features.defaults.\"ALLOW_EXTERNAL_METADATA_FILE_LOCATION\"", + "polaris.features.\"ALLOW_EXTERNAL_METADATA_FILE_LOCATION\"", "true"); } } 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 a2d50d9a4f..b54d032a81 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 @@ -169,13 +169,13 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", + "polaris.features.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", "true", - "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", + "polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", "true", - "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", + "polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]", - "polaris.features.defaults.\"LIST_PAGINATION_ENABLED\"", + "polaris.features.\"LIST_PAGINATION_ENABLED\"", "true", "polaris.event-listener.type", "test"); 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 a7bdec4f10..8b87b75a74 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 @@ -100,15 +100,15 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_WILDCARD_LOCATION\"", + "polaris.features.\"ALLOW_WILDCARD_LOCATION\"", "true", - "polaris.features.defaults.\"SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION\"", + "polaris.features.\"SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION\"", "true", - "polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", + "polaris.features.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", "true", - "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", + "polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", "true", - "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", + "polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]", "polaris.event-listener.type", "test"); 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 0ea91bb19c..308971c7cd 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 @@ -121,11 +121,11 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", + "polaris.features.\"ALLOW_SPECIFYING_FILE_IO_IMPL\"", "true", - "polaris.features.defaults.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", + "polaris.features.\"INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST\"", "true", - "polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", + "polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]"); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index f7c0f6a195..336f173ee6 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -56,11 +56,11 @@ public Map getConfigOverrides() { return Map.of( "polaris.realm-context.realms", "realm1,realm2", - String.format("polaris.features.defaults.\"%s\"", keyOne), + String.format("polaris.features.\"%s\"", keyOne), "true", - String.format("polaris.features.defaults.\"%s\"", keyTwo), + String.format("polaris.features.\"%s\"", keyTwo), "false", - String.format("polaris.features.defaults.\"%s\"", keyThree), + String.format("polaris.features.\"%s\"", keyThree), "true", String.format("polaris.features.realm-overrides.\"%s\".\"%s\"", realmOne, keyOne), "false", diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java index c98da4d5a9..594efc2219 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusApplicationIntegrationTest.java @@ -53,8 +53,8 @@ public Map getConfigOverrides() { return Map.of( "quarkus.http.limits.max-body-size", "1000000", "polaris.realm-context.realms", "POLARIS,OTHER", - "polaris.features.defaults.\"ALLOW_OVERLAPPING_CATALOG_URLS\"", "true", - "polaris.features.defaults.\"SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION\"", "true"); + "polaris.features.\"ALLOW_OVERLAPPING_CATALOG_URLS\"", "true", + "polaris.features.\"SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION\"", "true"); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusManagementServiceIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusManagementServiceIntegrationTest.java index 33735636d5..88421b8fed 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusManagementServiceIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusManagementServiceIntegrationTest.java @@ -34,9 +34,9 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_OVERLAPPING_CATALOG_URLS\"", + "polaris.features.\"ALLOW_OVERLAPPING_CATALOG_URLS\"", "true", - "polaris.features.defaults.\"ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING\"", + "polaris.features.\"ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING\"", "true", "polaris.storage.gcp.token", "token", diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java index 11dd821ef2..eb0e35242b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java @@ -33,7 +33,7 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { return Map.of( - "polaris.features.defaults.\"ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING\"", "false"); + "polaris.features.\"ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING\"", "false"); } } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAwsIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAwsIntegrationTest.java index b86c313a15..16380c24de 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAwsIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAwsIntegrationTest.java @@ -38,7 +38,7 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { - return Map.of("polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"S3\"]"); + return Map.of("polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"S3\"]"); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAzureIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAzureIntegrationTest.java index 8cc89b9c6b..49ff842b7d 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAzureIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewAzureIntegrationTest.java @@ -37,7 +37,7 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { - return Map.of("polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"AZURE\"]"); + return Map.of("polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"AZURE\"]"); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java index 4444bcbee3..3987e94d28 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewFileIntegrationTest.java @@ -36,7 +36,7 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { - return Map.of("polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]"); + return Map.of("polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"FILE\"]"); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewGcpIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewGcpIntegrationTest.java index 75168043b8..ef04d745e0 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewGcpIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogViewGcpIntegrationTest.java @@ -36,7 +36,7 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { - return Map.of("polaris.features.defaults.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"GCS\"]"); + return Map.of("polaris.features.\"SUPPORTED_CATALOG_STORAGE_TYPES\"", "[\"GCS\"]"); } } diff --git a/site/content/in-dev/unreleased/configuration.md b/site/content/in-dev/unreleased/configuration.md index b0626da814..e59a7b84ff 100644 --- a/site/content/in-dev/unreleased/configuration.md +++ b/site/content/in-dev/unreleased/configuration.md @@ -85,8 +85,8 @@ read-only mode, as Polaris only reads the configuration file once, at startup. | `polaris.realm-context.type` | `default` | Define the type of the Polaris realm to use. | | `polaris.realm-context.realms` | `POLARIS` | Define the list of realms to use. | | `polaris.realm-context.header-name` | `Polaris-Realm` | Define the header name defining the realm context. | -| `polaris.features.defaults."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"` | `false` | Flag to enforce check if credential rotation. | -| `polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"` | `FILE` | Define the catalog supported storage. Supported values are `S3`, `GCS`, `AZURE`, `FILE`. | +| `polaris.features."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"` | `false` | Flag to enforce check if credential rotation. | +| `polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"` | `FILE` | Define the catalog supported storage. Supported values are `S3`, `GCS`, `AZURE`, `FILE`. | | `polaris.features.realm-overrides."my-realm"."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST"` | `true` | "Override" realm features, here the catalog init default flag. | | `polaris.features.realm-overrides."my-realm"."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"` | `true` | "Override" realm features, here the skip credential subscoping indirection flag. | | `polaris.authentication.authenticator.type` | `default` | Define the Polaris authenticator type. | diff --git a/site/content/in-dev/unreleased/configuring-polaris-for-production.md b/site/content/in-dev/unreleased/configuring-polaris-for-production.md index d38dd33810..84a3cec5eb 100644 --- a/site/content/in-dev/unreleased/configuring-polaris-for-production.md +++ b/site/content/in-dev/unreleased/configuring-polaris-for-production.md @@ -213,7 +213,7 @@ curl -X POST http://localhost:8181/api/catalog/v1/oauth/tokens \ When deploying Polaris in production, consider adjusting the following configurations: -#### `polaris.features.defaults."SUPPORTED_CATALOG_STORAGE_TYPES"` +#### `polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"` - By default, Polaris catalogs are allowed to be located in local filesystem with the `FILE` storage type. This should be disabled for production systems. From 8a07197e9e1aa3d265ce7f998ed88ea4ac941d24 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 14:00:15 -0700 Subject: [PATCH 11/27] repair test --- .../config/DefaultConfigurationStoreTest.java | 53 ++++++++----------- .../in-dev/unreleased/configuration.md | 4 +- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index 336f173ee6..35d0144b00 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -34,7 +34,6 @@ import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; -import org.apache.polaris.service.config.FeaturesConfiguration; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -43,9 +42,9 @@ @QuarkusTest @TestProfile(DefaultConfigurationStoreTest.Profile.class) public class DefaultConfigurationStoreTest { - private static final String keyOne = "ALLOW_SPECIFYING_FILE_IO_IMPL"; - private static final String keyTwo = "INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST"; - private static final String keyThree = "SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"; + private static final String falseByDefaultKey = "ALLOW_SPECIFYING_FILE_IO_IMPL"; + private static final String trueByDefaultKey1 = "ENABLE_GENERIC_TABLES"; + private static final String trueByDefaultKey2 = "ENABLE_POLICY_STORE"; private static final String realmOne = "realm1"; private static final String realmTwo = "realm2"; @@ -56,22 +55,17 @@ public Map getConfigOverrides() { return Map.of( "polaris.realm-context.realms", "realm1,realm2", - String.format("polaris.features.\"%s\"", keyOne), + String.format("polaris.features.\"%s\"", falseByDefaultKey), "true", - String.format("polaris.features.\"%s\"", keyTwo), + String.format("polaris.features.\"%s\"", trueByDefaultKey1), "false", - String.format("polaris.features.\"%s\"", keyThree), - "true", - String.format("polaris.features.realm-overrides.\"%s\".\"%s\"", realmOne, keyOne), - "false", - String.format("polaris.features.realm-overrides.\"%s\".\"%s\"", realmTwo, keyTwo), - "true"); + String.format("polaris.features.realm-overrides.\"%s\".\"%s\"", realmOne, falseByDefaultKey), + "false"); } } private PolarisCallContext polarisContext; - @Inject FeaturesConfiguration featuresConfiguration; @Inject MetaStoreManagerFactory managerFactory; @Inject PolarisConfigurationStore configurationStore; @Inject PolarisDiagnostics diagServices; @@ -113,6 +107,13 @@ public void testGetConfiguration() { assertThat(keyOne).isEqualTo(1); String keyTwo = defaultConfigurationStore.getConfiguration(callCtx, "key2"); assertThat(keyTwo).isEqualTo("value"); + + boolean keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + assertThat(keyOneValue).isTrue(); + boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey1); + assertThat(keyTwoValue).isFalse(); + boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey2); + assertThat(keyThreeValue).isTrue(); } @Test @@ -125,11 +126,11 @@ public void testGetDefaultConfigurations() { polarisContext, "missingKeyWithDefault", "defaultValue"); assertThat(defaultValue).isEqualTo("defaultValue"); - boolean keyOneValue = configurationStore.getConfiguration(polarisContext, keyOne); - assertThat(keyOneValue).isTrue(); - boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, keyTwo); - assertThat(keyTwoValue).isFalse(); - boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, keyThree); + boolean keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + assertThat(keyOneValue).isFalse(); + boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey1); + assertThat(keyTwoValue).isTrue(); + boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey2); assertThat(keyThreeValue).isTrue(); } @@ -138,20 +139,12 @@ public void testGetRealmConfiguration() { RealmContext realmContext = () -> realmOne; QuarkusMock.installMockForType(realmContext, RealmContext.class); - boolean keyOneValue = configurationStore.getConfiguration(polarisContext, keyOne); - assertThat(keyOneValue).isFalse(); - boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, keyTwo); - assertThat(keyTwoValue).isFalse(); - boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, keyThree); - assertThat(keyThreeValue).isTrue(); + boolean keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + assertThat(keyOneValue).isTrue(); realmContext = () -> realmTwo; QuarkusMock.installMockForType(realmContext, RealmContext.class); - keyOneValue = configurationStore.getConfiguration(polarisContext, keyOne); - assertThat(keyOneValue).isTrue(); - keyTwoValue = configurationStore.getConfiguration(polarisContext, keyTwo); - assertThat(keyTwoValue).isTrue(); - keyThreeValue = configurationStore.getConfiguration(polarisContext, keyThree); - assertThat(keyThreeValue).isTrue(); + keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + assertThat(keyOneValue).isFalse(); } } diff --git a/site/content/in-dev/unreleased/configuration.md b/site/content/in-dev/unreleased/configuration.md index e59a7b84ff..ec864651c4 100644 --- a/site/content/in-dev/unreleased/configuration.md +++ b/site/content/in-dev/unreleased/configuration.md @@ -85,8 +85,8 @@ read-only mode, as Polaris only reads the configuration file once, at startup. | `polaris.realm-context.type` | `default` | Define the type of the Polaris realm to use. | | `polaris.realm-context.realms` | `POLARIS` | Define the list of realms to use. | | `polaris.realm-context.header-name` | `Polaris-Realm` | Define the header name defining the realm context. | -| `polaris.features."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"` | `false` | Flag to enforce check if credential rotation. | -| `polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"` | `FILE` | Define the catalog supported storage. Supported values are `S3`, `GCS`, `AZURE`, `FILE`. | +| `polaris.features."ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING"` | `false` | Flag to enforce check if credential rotation. | +| `polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"` | `FILE` | Define the catalog supported storage. Supported values are `S3`, `GCS`, `AZURE`, `FILE`. | | `polaris.features.realm-overrides."my-realm"."INITIALIZE_DEFAULT_CATALOG_FILEIO_FOR_TEST"` | `true` | "Override" realm features, here the catalog init default flag. | | `polaris.features.realm-overrides."my-realm"."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"` | `true` | "Override" realm features, here the skip credential subscoping indirection flag. | | `polaris.authentication.authenticator.type` | `default` | Define the Polaris authenticator type. | From 439ab040d34261e5d370c42a0d78011397556365 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 14:00:18 -0700 Subject: [PATCH 12/27] autolint --- .../service/quarkus/config/DefaultConfigurationStoreTest.java | 3 ++- .../service/quarkus/it/QuarkusRestCatalogIntegrationTest.java | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index 35d0144b00..bb4e63bdd8 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -59,7 +59,8 @@ public Map getConfigOverrides() { "true", String.format("polaris.features.\"%s\"", trueByDefaultKey1), "false", - String.format("polaris.features.realm-overrides.\"%s\".\"%s\"", realmOne, falseByDefaultKey), + String.format( + "polaris.features.realm-overrides.\"%s\".\"%s\"", realmOne, falseByDefaultKey), "false"); } } diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java index eb0e35242b..27e29fd613 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogIntegrationTest.java @@ -32,8 +32,7 @@ public static class Profile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { - return Map.of( - "polaris.features.\"ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING\"", "false"); + return Map.of("polaris.features.\"ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING\"", "false"); } } } From 7761cc0a879fdac638be608d985874100bcd2c6b Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 14:48:54 -0700 Subject: [PATCH 13/27] stablize test --- helm/polaris/README.md | 2 +- helm/polaris/templates/configmap.yaml | 2 +- .../config/DefaultConfigurationStoreTest.java | 107 ++++++++++++------ 3 files changed, 73 insertions(+), 38 deletions(-) diff --git a/helm/polaris/README.md b/helm/polaris/README.md index f67736614f..523c6e515b 100644 --- a/helm/polaris/README.md +++ b/helm/polaris/README.md @@ -220,7 +220,7 @@ kubectl delete namespace polaris | extraVolumeMounts | list | `[]` | Extra volume mounts to add to the polaris container. See https://kubernetes.io/docs/concepts/storage/volumes/. | | extraVolumes | list | `[]` | Extra volumes to add to the polaris pod. See https://kubernetes.io/docs/concepts/storage/volumes/. | | features | object | `{"defaults":{},"realmOverrides":{}}` | Polaris features configuration. | -| features.defaults | object | `{}` | Features to enable or disable globally. If a feature is not present in the map, the default built-in value is used. | +| features | object | `{}` | Features to enable or disable globally. If a feature is not present in the map, the default built-in value is used. | | features.realmOverrides | object | `{}` | Features to enable or disable per realm. This field is a map of maps. The realm name is the key, and the value is a map of feature names to values. If a feature is not present in the map, the default value from the 'defaults' field is used. | | fileIo | object | `{"type":"default"}` | Polaris FileIO configuration. | | fileIo.type | string | `"default"` | The type of file IO to use. Two built-in types are supported: default and wasb. The wasb one translates WASB paths to ABFS ones. | diff --git a/helm/polaris/templates/configmap.yaml b/helm/polaris/templates/configmap.yaml index 44153a37bd..db6c84f172 100644 --- a/helm/polaris/templates/configmap.yaml +++ b/helm/polaris/templates/configmap.yaml @@ -36,7 +36,7 @@ data: {{- $_ = set $map "polaris.realm-context.realms" (join "," .Values.realmContext.realms) -}} {{- /* Features */ -}} - {{- range $k, $v := .Values.features.defaults -}} + {{- range $k, $v := .Values.features -}} {{- $_ = set $map (printf "polaris.features.\"%s\"" $k) (toJson $v) -}} {{- end -}} {{- range $realm, $overrides := .Values.features.realmOverrides -}} diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index bb4e63bdd8..f620729503 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -20,17 +20,19 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.time.Clock; +import java.util.Map; + import io.quarkus.test.junit.QuarkusMock; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.QuarkusTestProfile; import io.quarkus.test.junit.TestProfile; import jakarta.inject.Inject; -import java.time.Clock; -import java.util.Map; 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.persistence.MetaStoreManagerFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; @@ -43,8 +45,7 @@ @TestProfile(DefaultConfigurationStoreTest.Profile.class) public class DefaultConfigurationStoreTest { private static final String falseByDefaultKey = "ALLOW_SPECIFYING_FILE_IO_IMPL"; - private static final String trueByDefaultKey1 = "ENABLE_GENERIC_TABLES"; - private static final String trueByDefaultKey2 = "ENABLE_POLICY_STORE"; + private static final String trueByDefaultKey = "ENABLE_GENERIC_TABLES"; private static final String realmOne = "realm1"; private static final String realmTwo = "realm2"; @@ -57,7 +58,7 @@ public Map getConfigOverrides() { "realm1,realm2", String.format("polaris.features.\"%s\"", falseByDefaultKey), "true", - String.format("polaris.features.\"%s\"", trueByDefaultKey1), + String.format("polaris.features.\"%s\"", trueByDefaultKey), "false", String.format( "polaris.features.realm-overrides.\"%s\".\"%s\"", realmOne, falseByDefaultKey), @@ -67,9 +68,12 @@ public Map getConfigOverrides() { private PolarisCallContext polarisContext; - @Inject MetaStoreManagerFactory managerFactory; - @Inject PolarisConfigurationStore configurationStore; - @Inject PolarisDiagnostics diagServices; + @Inject + MetaStoreManagerFactory managerFactory; + @Inject + PolarisConfigurationStore configurationStore; + @Inject + PolarisDiagnostics diagServices; @BeforeEach public void before(TestInfo testInfo) { @@ -108,44 +112,75 @@ public void testGetConfiguration() { assertThat(keyOne).isEqualTo(1); String keyTwo = defaultConfigurationStore.getConfiguration(callCtx, "key2"); assertThat(keyTwo).isEqualTo("value"); - - boolean keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); - assertThat(keyOneValue).isTrue(); - boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey1); - assertThat(keyTwoValue).isFalse(); - boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey2); - assertThat(keyThreeValue).isTrue(); } @Test - public void testGetDefaultConfigurations() { - Object value = configurationStore.getConfiguration(polarisContext, "missingKeyWithoutDefault"); - assertThat(value).isNull(); + public void testGetRealmConfiguration() { + int defaultKeyOneValue = 1; + String defaultKeyTwoValue = "value"; + + int realm1KeyOneValue = 2; + int realm2KeyOneValue = 3; + String realm2KeyTwoValue = "value3"; + DefaultConfigurationStore defaultConfigurationStore = + new DefaultConfigurationStore( + Map.of("key1", defaultKeyOneValue, "key2", defaultKeyTwoValue), + Map.of( + "realm1", + Map.of("key1", realm1KeyOneValue), + "realm2", + Map.of("key1", realm2KeyOneValue, "key2", realm2KeyTwoValue))); + InMemoryPolarisMetaStoreManagerFactory metastoreFactory = + new InMemoryPolarisMetaStoreManagerFactory(); + // check realm1 values + PolarisCallContext realm1Ctx = + new PolarisCallContext( + metastoreFactory.getOrCreateSessionSupplier(() -> "realm1").get(), + new PolarisDefaultDiagServiceImpl()); + Object value = + defaultConfigurationStore.getConfiguration(realm1Ctx, "missingKeyWithoutDefault"); + assertThat(value).isNull(); Object defaultValue = - configurationStore.getConfiguration( - polarisContext, "missingKeyWithDefault", "defaultValue"); + defaultConfigurationStore.getConfiguration( + realm1Ctx, "missingKeyWithDefault", "defaultValue"); assertThat(defaultValue).isEqualTo("defaultValue"); - - boolean keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); - assertThat(keyOneValue).isFalse(); - boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey1); - assertThat(keyTwoValue).isTrue(); - boolean keyThreeValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey2); - assertThat(keyThreeValue).isTrue(); + CallContext.setCurrentContext(CallContext.of(() -> "realm1", realm1Ctx)); + Integer keyOneRealm1 = defaultConfigurationStore.getConfiguration(realm1Ctx, "key1"); + assertThat(keyOneRealm1).isEqualTo(realm1KeyOneValue); + String keyTwoRealm1 = defaultConfigurationStore.getConfiguration(realm1Ctx, "key2"); + assertThat(keyTwoRealm1).isEqualTo(defaultKeyTwoValue); + + // check realm2 values + PolarisCallContext realm2Ctx = + new PolarisCallContext( + metastoreFactory.getOrCreateSessionSupplier(() -> "realm2").get(), + new PolarisDefaultDiagServiceImpl()); + CallContext.setCurrentContext(CallContext.of(() -> "realm2", realm2Ctx)); + Integer keyOneRealm2 = defaultConfigurationStore.getConfiguration(realm2Ctx, "key1"); + assertThat(keyOneRealm2).isEqualTo(realm2KeyOneValue); + String keyTwoRealm2 = defaultConfigurationStore.getConfiguration(realm2Ctx, "key2"); + assertThat(keyTwoRealm2).isEqualTo(realm2KeyTwoValue); + + // realm3 has no realm-overrides, so just returns default values + PolarisCallContext realm3Ctx = + new PolarisCallContext( + metastoreFactory.getOrCreateSessionSupplier(() -> "realm3").get(), + new PolarisDefaultDiagServiceImpl()); + CallContext.setCurrentContext(CallContext.of(() -> "realm3", realm3Ctx)); + Integer keyOneRealm3 = defaultConfigurationStore.getConfiguration(realm3Ctx, "key1"); + assertThat(keyOneRealm3).isEqualTo(defaultKeyOneValue); + String keyTwoRealm3 = defaultConfigurationStore.getConfiguration(realm3Ctx, "key2"); + assertThat(keyTwoRealm3).isEqualTo(defaultKeyTwoValue); } @Test - public void testGetRealmConfiguration() { - RealmContext realmContext = () -> realmOne; - QuarkusMock.installMockForType(realmContext, RealmContext.class); - + public void testInjectedConfigurationStore() { + // Realm override makes this `false` boolean keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); - assertThat(keyOneValue).isTrue(); - - realmContext = () -> realmTwo; - QuarkusMock.installMockForType(realmContext, RealmContext.class); - keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); assertThat(keyOneValue).isFalse(); + // Default override makes this `false` + boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey); + assertThat(keyTwoValue).isFalse(); } } From a1d2d451d22a9069e8251ce1ce7372cb9e007a0e Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 14:56:19 -0700 Subject: [PATCH 14/27] stable --- helm/polaris/templates/configmap.yaml | 2 +- helm/polaris/tests/configmap_test.yaml | 4 +- .../config/DefaultConfigurationStoreTest.java | 56 +++++++++++++------ 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/helm/polaris/templates/configmap.yaml b/helm/polaris/templates/configmap.yaml index db6c84f172..ae732760cb 100644 --- a/helm/polaris/templates/configmap.yaml +++ b/helm/polaris/templates/configmap.yaml @@ -41,7 +41,7 @@ data: {{- end -}} {{- range $realm, $overrides := .Values.features.realmOverrides -}} {{- range $k, $v := $overrides -}} - {{- $_ = set $map (printf "polaris.config.realm-overrides.\"%s\".\"%s\"" $realm $k) (toJson $v) -}} + {{- $_ = set $map (printf "polaris.features.realm-overrides.\"%s\".\"%s\"" $realm $k) (toJson $v) -}} {{- end -}} {{- end -}} diff --git a/helm/polaris/tests/configmap_test.yaml b/helm/polaris/tests/configmap_test.yaml index 3c8f018959..ee61df860b 100644 --- a/helm/polaris/tests/configmap_test.yaml +++ b/helm/polaris/tests/configmap_test.yaml @@ -100,8 +100,8 @@ tests: asserts: - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.\"feature1\"=true" } - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.\"feature2\"=42" } - - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.config.realm-overrides.\"realm1\".\"feature1\"=false" } - - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.config.realm-overrides.\"realm2\".\"feature2\"=43" } + - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.realm-overrides.\"realm1\".\"feature1\"=false" } + - matchRegex: { path: 'data["application.properties"]', pattern: "polaris.features.realm-overrides.\"realm2\".\"feature2\"=43" } - it: should configure persistence set: diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index f620729503..4f69670cfa 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -20,14 +20,13 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.time.Clock; -import java.util.Map; - import io.quarkus.test.junit.QuarkusMock; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.QuarkusTestProfile; import io.quarkus.test.junit.TestProfile; import jakarta.inject.Inject; +import java.time.Clock; +import java.util.Map; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; import org.apache.polaris.core.PolarisDiagnostics; @@ -47,7 +46,6 @@ public class DefaultConfigurationStoreTest { private static final String falseByDefaultKey = "ALLOW_SPECIFYING_FILE_IO_IMPL"; private static final String trueByDefaultKey = "ENABLE_GENERIC_TABLES"; private static final String realmOne = "realm1"; - private static final String realmTwo = "realm2"; public static class Profile implements QuarkusTestProfile { @@ -68,12 +66,9 @@ public Map getConfigOverrides() { private PolarisCallContext polarisContext; - @Inject - MetaStoreManagerFactory managerFactory; - @Inject - PolarisConfigurationStore configurationStore; - @Inject - PolarisDiagnostics diagServices; + @Inject MetaStoreManagerFactory managerFactory; + @Inject PolarisConfigurationStore configurationStore; + @Inject PolarisDiagnostics diagServices; @BeforeEach public void before(TestInfo testInfo) { @@ -174,13 +169,42 @@ public void testGetRealmConfiguration() { assertThat(keyTwoRealm3).isEqualTo(defaultKeyTwoValue); } + // TODO simplify once DefaultConfigrationStore doesn't rely on CallContext.getCurrentContext + private void setCurrentRealm(String realmName) { + RealmContext realmContext = () -> realmName; + QuarkusMock.installMockForType(realmContext, RealmContext.class); + CallContext.setCurrentContext(new CallContext() { + @Override + public RealmContext getRealmContext() { + return realmContext; + } + + @Override + public PolarisCallContext getPolarisCallContext() { + return CallContext.getCurrentContext().getPolarisCallContext(); + } + + @Override + public Map contextVariables() { + return CallContext.getCurrentContext().contextVariables(); + } + }); + } + @Test public void testInjectedConfigurationStore() { - // Realm override makes this `false` - boolean keyOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); - assertThat(keyOneValue).isFalse(); - // Default override makes this `false` - boolean keyTwoValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey); - assertThat(keyTwoValue).isFalse(); + // Feature override makes this `false` + boolean featureOverrideValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey); + assertThat(featureOverrideValue).isFalse(); + + // Feature override value makes this `true` + setCurrentRealm("not-" + realmOne); + boolean realmOverrideValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + assertThat(realmOverrideValue).isTrue(); + + // Now, realm override value makes this `false` + setCurrentRealm(realmOne); + realmOverrideValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + assertThat(realmOverrideValue).isFalse(); } } From ef79de5b10b174413433d91fb54c5feb280be3e7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 14:56:23 -0700 Subject: [PATCH 15/27] autolint --- .../config/DefaultConfigurationStoreTest.java | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index 4f69670cfa..26eb26c26b 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -173,33 +173,36 @@ public void testGetRealmConfiguration() { private void setCurrentRealm(String realmName) { RealmContext realmContext = () -> realmName; QuarkusMock.installMockForType(realmContext, RealmContext.class); - CallContext.setCurrentContext(new CallContext() { - @Override - public RealmContext getRealmContext() { - return realmContext; - } - - @Override - public PolarisCallContext getPolarisCallContext() { - return CallContext.getCurrentContext().getPolarisCallContext(); - } - - @Override - public Map contextVariables() { - return CallContext.getCurrentContext().contextVariables(); - } - }); + CallContext.setCurrentContext( + new CallContext() { + @Override + public RealmContext getRealmContext() { + return realmContext; + } + + @Override + public PolarisCallContext getPolarisCallContext() { + return CallContext.getCurrentContext().getPolarisCallContext(); + } + + @Override + public Map contextVariables() { + return CallContext.getCurrentContext().contextVariables(); + } + }); } @Test public void testInjectedConfigurationStore() { // Feature override makes this `false` - boolean featureOverrideValue = configurationStore.getConfiguration(polarisContext, trueByDefaultKey); + boolean featureOverrideValue = + configurationStore.getConfiguration(polarisContext, trueByDefaultKey); assertThat(featureOverrideValue).isFalse(); // Feature override value makes this `true` setCurrentRealm("not-" + realmOne); - boolean realmOverrideValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + boolean realmOverrideValue = + configurationStore.getConfiguration(polarisContext, falseByDefaultKey); assertThat(realmOverrideValue).isTrue(); // Now, realm override value makes this `false` From 9819f3da5d077392ce1b000c1f5180beeadbf1fc Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 15:01:22 -0700 Subject: [PATCH 16/27] configmap change --- helm/polaris/tests/configmap_test.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/helm/polaris/tests/configmap_test.yaml b/helm/polaris/tests/configmap_test.yaml index ee61df860b..562adad220 100644 --- a/helm/polaris/tests/configmap_test.yaml +++ b/helm/polaris/tests/configmap_test.yaml @@ -89,9 +89,8 @@ tests: - it: should configure features set: features: - defaults: - feature1: true - feature2: 42 + feature1: true + feature2: 42 realmOverrides: realm1: feature1: false From fe38c22e76af659fce97d78f5ff6f0e5525cb282 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 16:17:40 -0700 Subject: [PATCH 17/27] copypaste --- .../catalog/iceberg/CatalogHandlerUtils.java | 599 ++++++++++++++++++ 1 file changed, 599 insertions(+) create mode 100644 service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java new file mode 100644 index 0000000000..75217c9b6e --- /dev/null +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java @@ -0,0 +1,599 @@ +/* + * 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.service.catalog.iceberg; + +import static org.apache.iceberg.TableProperties.COMMIT_MAX_RETRY_WAIT_MS_DEFAULT; +import static org.apache.iceberg.TableProperties.COMMIT_MIN_RETRY_WAIT_MS_DEFAULT; +import static org.apache.iceberg.TableProperties.COMMIT_NUM_RETRIES_DEFAULT; +import static org.apache.iceberg.TableProperties.COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT; + +import java.time.OffsetDateTime; +import java.time.ZoneOffset; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; + +import com.google.common.base.Preconditions; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import org.apache.iceberg.BaseMetadataTable; +import org.apache.iceberg.BaseTable; +import org.apache.iceberg.BaseTransaction; +import org.apache.iceberg.MetadataUpdate.UpgradeFormatVersion; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.SortOrder; +import org.apache.iceberg.Table; +import org.apache.iceberg.TableMetadata; +import org.apache.iceberg.TableOperations; +import org.apache.iceberg.Transaction; +import org.apache.iceberg.UpdateRequirement; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.catalog.ViewCatalog; +import org.apache.iceberg.exceptions.AlreadyExistsException; +import org.apache.iceberg.exceptions.CommitFailedException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.iceberg.exceptions.NoSuchViewException; +import org.apache.iceberg.rest.requests.CreateNamespaceRequest; +import org.apache.iceberg.rest.requests.CreateTableRequest; +import org.apache.iceberg.rest.requests.CreateViewRequest; +import org.apache.iceberg.rest.requests.RegisterTableRequest; +import org.apache.iceberg.rest.requests.RenameTableRequest; +import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest; +import org.apache.iceberg.rest.requests.UpdateTableRequest; +import org.apache.iceberg.rest.responses.CreateNamespaceResponse; +import org.apache.iceberg.rest.responses.GetNamespaceResponse; +import org.apache.iceberg.rest.responses.ImmutableLoadViewResponse; +import org.apache.iceberg.rest.responses.ListNamespacesResponse; +import org.apache.iceberg.rest.responses.ListTablesResponse; +import org.apache.iceberg.rest.responses.LoadTableResponse; +import org.apache.iceberg.rest.responses.LoadViewResponse; +import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse; +import org.apache.iceberg.util.Pair; +import org.apache.iceberg.util.Tasks; +import org.apache.iceberg.view.BaseView; +import org.apache.iceberg.view.SQLViewRepresentation; +import org.apache.iceberg.view.View; +import org.apache.iceberg.view.ViewBuilder; +import org.apache.iceberg.view.ViewMetadata; +import org.apache.iceberg.view.ViewOperations; +import org.apache.iceberg.view.ViewRepresentation; + +/** + * A collection of utilities related to or copied from Iceberg's CatalogHandlers + * CODE_COPIED_TO_POLARIS Iceberg 1.8.0 + */ +public class CatalogHandlerUtils { + private static final Schema EMPTY_SCHEMA = new Schema(); + private static final String INITIAL_PAGE_TOKEN = ""; + + private CatalogHandlerUtils() {} + + /** + * Exception used to avoid retrying commits when assertions fail. + * + *

When a REST assertion fails, it will throw CommitFailedException to send back to the client. + * But the assertion checks happen in the block that is retried if {@link + * TableOperations#commit(TableMetadata, TableMetadata)} throws CommitFailedException. This is + * used to avoid retries for assertion failures, which are unwrapped and rethrown outside of the + * commit loop. + */ + private static class ValidationFailureException extends RuntimeException { + private final CommitFailedException wrapped; + + private ValidationFailureException(CommitFailedException cause) { + super(cause); + this.wrapped = cause; + } + + public CommitFailedException wrapped() { + return wrapped; + } + } + + private static Pair, String> paginate(List list, String pageToken, int pageSize) { + int pageStart = INITIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken); + if (pageStart >= list.size()) { + return Pair.of(Collections.emptyList(), null); + } + + int end = Math.min(pageStart + pageSize, list.size()); + List subList = list.subList(pageStart, end); + String nextPageToken = end >= list.size() ? null : String.valueOf(end); + + return Pair.of(subList, nextPageToken); + } + + public static ListNamespacesResponse listNamespaces( + SupportsNamespaces catalog, Namespace parent) { + List results; + if (parent.isEmpty()) { + results = catalog.listNamespaces(); + } else { + results = catalog.listNamespaces(parent); + } + + return ListNamespacesResponse.builder().addAll(results).build(); + } + + public static ListNamespacesResponse listNamespaces( + SupportsNamespaces catalog, Namespace parent, String pageToken, String pageSize) { + List results; + + if (parent.isEmpty()) { + results = catalog.listNamespaces(); + } else { + results = catalog.listNamespaces(parent); + } + + Pair, String> page = paginate(results, pageToken, Integer.parseInt(pageSize)); + + return ListNamespacesResponse.builder() + .addAll(page.first()) + .nextPageToken(page.second()) + .build(); + } + + public static CreateNamespaceResponse createNamespace( + SupportsNamespaces catalog, CreateNamespaceRequest request) { + Namespace namespace = request.namespace(); + catalog.createNamespace(namespace, request.properties()); + return CreateNamespaceResponse.builder() + .withNamespace(namespace) + .setProperties(catalog.loadNamespaceMetadata(namespace)) + .build(); + } + + public static void namespaceExists(SupportsNamespaces catalog, Namespace namespace) { + if (!catalog.namespaceExists(namespace)) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + } + + public static GetNamespaceResponse loadNamespace( + SupportsNamespaces catalog, Namespace namespace) { + Map properties = catalog.loadNamespaceMetadata(namespace); + return GetNamespaceResponse.builder() + .withNamespace(namespace) + .setProperties(properties) + .build(); + } + + public static void dropNamespace(SupportsNamespaces catalog, Namespace namespace) { + boolean dropped = catalog.dropNamespace(namespace); + if (!dropped) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + } + + public static UpdateNamespacePropertiesResponse updateNamespaceProperties( + SupportsNamespaces catalog, Namespace namespace, UpdateNamespacePropertiesRequest request) { + request.validate(); + + Set removals = Sets.newHashSet(request.removals()); + Map updates = request.updates(); + + Map startProperties = catalog.loadNamespaceMetadata(namespace); + Set missing = Sets.difference(removals, startProperties.keySet()); + + if (!updates.isEmpty()) { + catalog.setProperties(namespace, updates); + } + + if (!removals.isEmpty()) { + // remove the original set just in case there was an update just after loading properties + catalog.removeProperties(namespace, removals); + } + + return UpdateNamespacePropertiesResponse.builder() + .addMissing(missing) + .addUpdated(updates.keySet()) + .addRemoved(Sets.difference(removals, missing)) + .build(); + } + + public static ListTablesResponse listTables(Catalog catalog, Namespace namespace) { + List idents = catalog.listTables(namespace); + return ListTablesResponse.builder().addAll(idents).build(); + } + + public static ListTablesResponse listTables( + Catalog catalog, Namespace namespace, String pageToken, String pageSize) { + List results = catalog.listTables(namespace); + + Pair, String> page = + paginate(results, pageToken, Integer.parseInt(pageSize)); + + return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); + } + + public static LoadTableResponse stageTableCreate( + Catalog catalog, Namespace namespace, CreateTableRequest request) { + request.validate(); + + TableIdentifier ident = TableIdentifier.of(namespace, request.name()); + if (catalog.tableExists(ident)) { + throw new AlreadyExistsException("Table already exists: %s", ident); + } + + Map properties = Maps.newHashMap(); + properties.put("created-at", OffsetDateTime.now(ZoneOffset.UTC).toString()); + properties.putAll(request.properties()); + + String location; + if (request.location() != null) { + location = request.location(); + } else { + location = + catalog + .buildTable(ident, request.schema()) + .withPartitionSpec(request.spec()) + .withSortOrder(request.writeOrder()) + .withProperties(properties) + .createTransaction() + .table() + .location(); + } + + TableMetadata metadata = + TableMetadata.newTableMetadata( + request.schema(), + request.spec() != null ? request.spec() : PartitionSpec.unpartitioned(), + request.writeOrder() != null ? request.writeOrder() : SortOrder.unsorted(), + location, + properties); + + return LoadTableResponse.builder().withTableMetadata(metadata).build(); + } + + public static LoadTableResponse createTable( + Catalog catalog, Namespace namespace, CreateTableRequest request) { + request.validate(); + + TableIdentifier ident = TableIdentifier.of(namespace, request.name()); + Table table = + catalog + .buildTable(ident, request.schema()) + .withLocation(request.location()) + .withPartitionSpec(request.spec()) + .withSortOrder(request.writeOrder()) + .withProperties(request.properties()) + .create(); + + if (table instanceof BaseTable) { + return LoadTableResponse.builder() + .withTableMetadata(((BaseTable) table).operations().current()) + .build(); + } + + throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + } + + public static LoadTableResponse registerTable( + Catalog catalog, Namespace namespace, RegisterTableRequest request) { + request.validate(); + + TableIdentifier identifier = TableIdentifier.of(namespace, request.name()); + Table table = catalog.registerTable(identifier, request.metadataLocation()); + if (table instanceof BaseTable) { + return LoadTableResponse.builder() + .withTableMetadata(((BaseTable) table).operations().current()) + .build(); + } + + throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + } + + public static void dropTable(Catalog catalog, TableIdentifier ident) { + boolean dropped = catalog.dropTable(ident, false); + if (!dropped) { + throw new NoSuchTableException("Table does not exist: %s", ident); + } + } + + public static void purgeTable(Catalog catalog, TableIdentifier ident) { + boolean dropped = catalog.dropTable(ident, true); + if (!dropped) { + throw new NoSuchTableException("Table does not exist: %s", ident); + } + } + + public static void tableExists(Catalog catalog, TableIdentifier ident) { + boolean exists = catalog.tableExists(ident); + if (!exists) { + throw new NoSuchTableException("Table does not exist: %s", ident); + } + } + + public static LoadTableResponse loadTable(Catalog catalog, TableIdentifier ident) { + Table table = catalog.loadTable(ident); + + if (table instanceof BaseTable) { + return LoadTableResponse.builder() + .withTableMetadata(((BaseTable) table).operations().current()) + .build(); + } else if (table instanceof BaseMetadataTable) { + // metadata tables are loaded on the client side, return NoSuchTableException for now + throw new NoSuchTableException("Table does not exist: %s", ident.toString()); + } + + throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + } + + public static LoadTableResponse updateTable( + Catalog catalog, TableIdentifier ident, UpdateTableRequest request) { + TableMetadata finalMetadata; + if (isCreate(request)) { + // this is a hacky way to get TableOperations for an uncommitted table + Transaction transaction = + catalog.buildTable(ident, EMPTY_SCHEMA).createOrReplaceTransaction(); + if (transaction instanceof BaseTransaction) { + BaseTransaction baseTransaction = (BaseTransaction) transaction; + finalMetadata = create(baseTransaction.underlyingOps(), request); + } else { + throw new IllegalStateException( + "Cannot wrap catalog that does not produce BaseTransaction"); + } + + } else { + Table table = catalog.loadTable(ident); + if (table instanceof BaseTable) { + TableOperations ops = ((BaseTable) table).operations(); + finalMetadata = commit(ops, request); + } else { + throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + } + } + + return LoadTableResponse.builder().withTableMetadata(finalMetadata).build(); + } + + public static void renameTable(Catalog catalog, RenameTableRequest request) { + catalog.renameTable(request.source(), request.destination()); + } + + private static boolean isCreate(UpdateTableRequest request) { + boolean isCreate = + request.requirements().stream() + .anyMatch(UpdateRequirement.AssertTableDoesNotExist.class::isInstance); + + if (isCreate) { + List invalidRequirements = + request.requirements().stream() + .filter(req -> !(req instanceof UpdateRequirement.AssertTableDoesNotExist)) + .collect(Collectors.toList()); + Preconditions.checkArgument( + invalidRequirements.isEmpty(), "Invalid create requirements: %s", invalidRequirements); + } + + return isCreate; + } + + private static TableMetadata create(TableOperations ops, UpdateTableRequest request) { + // the only valid requirement is that the table will be created + request.requirements().forEach(requirement -> requirement.validate(ops.current())); + Optional formatVersion = + request.updates().stream() + .filter(update -> update instanceof UpgradeFormatVersion) + .map(update -> ((UpgradeFormatVersion) update).formatVersion()) + .findFirst(); + + TableMetadata.Builder builder = + formatVersion.map(TableMetadata::buildFromEmpty).orElseGet(TableMetadata::buildFromEmpty); + request.updates().forEach(update -> update.applyTo(builder)); + // create transactions do not retry. if the table exists, retrying is not a solution + ops.commit(null, builder.build()); + + return ops.current(); + } + + static TableMetadata commit(TableOperations ops, UpdateTableRequest request) { + AtomicBoolean isRetry = new AtomicBoolean(false); + try { + Tasks.foreach(ops) + .retry(COMMIT_NUM_RETRIES_DEFAULT) + .exponentialBackoff( + COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, + COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, + COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, + 2.0 /* exponential */) + .onlyRetryOn(CommitFailedException.class) + .run( + taskOps -> { + TableMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); + isRetry.set(true); + + // validate requirements + try { + request.requirements().forEach(requirement -> requirement.validate(base)); + } catch (CommitFailedException e) { + // wrap and rethrow outside of tasks to avoid unnecessary retry + throw new ValidationFailureException(e); + } + + // apply changes + TableMetadata.Builder metadataBuilder = TableMetadata.buildFrom(base); + request.updates().forEach(update -> update.applyTo(metadataBuilder)); + + TableMetadata updated = metadataBuilder.build(); + if (updated.changes().isEmpty()) { + // do not commit if the metadata has not changed + return; + } + + // commit + taskOps.commit(base, updated); + }); + + } catch (ValidationFailureException e) { + throw e.wrapped(); + } + + return ops.current(); + } + + private static BaseView asBaseView(View view) { + Preconditions.checkState( + view instanceof BaseView, "Cannot wrap catalog that does not produce BaseView"); + return (BaseView) view; + } + + public static ListTablesResponse listViews(ViewCatalog catalog, Namespace namespace) { + return ListTablesResponse.builder().addAll(catalog.listViews(namespace)).build(); + } + + public static ListTablesResponse listViews( + ViewCatalog catalog, Namespace namespace, String pageToken, String pageSize) { + List results = catalog.listViews(namespace); + + Pair, String> page = + paginate(results, pageToken, Integer.parseInt(pageSize)); + + return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); + } + + public static LoadViewResponse createView( + ViewCatalog catalog, Namespace namespace, CreateViewRequest request) { + request.validate(); + + ViewBuilder viewBuilder = + catalog + .buildView(TableIdentifier.of(namespace, request.name())) + .withSchema(request.schema()) + .withProperties(request.properties()) + .withDefaultNamespace(request.viewVersion().defaultNamespace()) + .withDefaultCatalog(request.viewVersion().defaultCatalog()) + .withLocation(request.location()); + + Set unsupportedRepresentations = + request.viewVersion().representations().stream() + .filter(r -> !(r instanceof SQLViewRepresentation)) + .map(ViewRepresentation::type) + .collect(Collectors.toSet()); + + if (!unsupportedRepresentations.isEmpty()) { + throw new IllegalStateException( + String.format("Found unsupported view representations: %s", unsupportedRepresentations)); + } + + request.viewVersion().representations().stream() + .filter(SQLViewRepresentation.class::isInstance) + .map(SQLViewRepresentation.class::cast) + .forEach(r -> viewBuilder.withQuery(r.dialect(), r.sql())); + + View view = viewBuilder.create(); + + return viewResponse(view); + } + + private static LoadViewResponse viewResponse(View view) { + ViewMetadata metadata = asBaseView(view).operations().current(); + return ImmutableLoadViewResponse.builder() + .metadata(metadata) + .metadataLocation(metadata.metadataFileLocation()) + .build(); + } + + public static void viewExists(ViewCatalog catalog, TableIdentifier viewIdentifier) { + if (!catalog.viewExists(viewIdentifier)) { + throw new NoSuchViewException("View does not exist: %s", viewIdentifier); + } + } + + public static LoadViewResponse loadView(ViewCatalog catalog, TableIdentifier viewIdentifier) { + View view = catalog.loadView(viewIdentifier); + return viewResponse(view); + } + + public static LoadViewResponse updateView( + ViewCatalog catalog, TableIdentifier ident, UpdateTableRequest request) { + View view = catalog.loadView(ident); + ViewMetadata metadata = commit(asBaseView(view).operations(), request); + + return ImmutableLoadViewResponse.builder() + .metadata(metadata) + .metadataLocation(metadata.metadataFileLocation()) + .build(); + } + + public static void renameView(ViewCatalog catalog, RenameTableRequest request) { + catalog.renameView(request.source(), request.destination()); + } + + public static void dropView(ViewCatalog catalog, TableIdentifier viewIdentifier) { + boolean dropped = catalog.dropView(viewIdentifier); + if (!dropped) { + throw new NoSuchViewException("View does not exist: %s", viewIdentifier); + } + } + + static ViewMetadata commit(ViewOperations ops, UpdateTableRequest request) { + AtomicBoolean isRetry = new AtomicBoolean(false); + try { + Tasks.foreach(ops) + .retry(COMMIT_NUM_RETRIES_DEFAULT) + .exponentialBackoff( + COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, + COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, + COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, + 2.0 /* exponential */) + .onlyRetryOn(CommitFailedException.class) + .run( + taskOps -> { + ViewMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); + isRetry.set(true); + + // validate requirements + try { + request.requirements().forEach(requirement -> requirement.validate(base)); + } catch (CommitFailedException e) { + // wrap and rethrow outside of tasks to avoid unnecessary retry + throw new ValidationFailureException(e); + } + + // apply changes + ViewMetadata.Builder metadataBuilder = ViewMetadata.buildFrom(base); + request.updates().forEach(update -> update.applyTo(metadataBuilder)); + + ViewMetadata updated = metadataBuilder.build(); + + if (updated.changes().isEmpty()) { + // do not commit if the metadata has not changed + return; + } + + // commit + taskOps.commit(base, updated); + }); + + } catch (ValidationFailureException e) { + throw e.wrapped(); + } + + return ops.current(); + } +} \ No newline at end of file From ef31c93d55e31728773166d654c49739a7860842 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 16:19:59 -0700 Subject: [PATCH 18/27] regen helm docs --- helm/polaris/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/helm/polaris/README.md b/helm/polaris/README.md index 523c6e515b..886cd4ed3f 100644 --- a/helm/polaris/README.md +++ b/helm/polaris/README.md @@ -48,7 +48,7 @@ A Helm chart for Apache Polaris (incubating). ### Optional -When using EclipseLink backed metastore a custom `persistence.xml` is required, a Kubernetes Secret must be created for it. Below is a sample command: +When using a custom `persistence.xml`, a Kubernetes Secret must be created for it. Below is a sample command: ```bash kubectl create secret generic polaris-secret -n polaris --from-file=persistence.xml ``` @@ -67,7 +67,7 @@ helm unittest helm/polaris The below instructions assume Kind and Helm are installed. Simply run the `run.sh` script from the Polaris repo root, making sure to specify the -`--eclipse-link-deps` if using EclipseLink backed metastore, option: +`--eclipse-link-deps` option: ```bash ./run.sh @@ -186,8 +186,8 @@ kubectl delete namespace polaris ## Values - Key | Type | Default | Description | -|-----|------|-----|-------------| +| Key | Type | Default | Description | +|-----|------|---------|-------------| | advancedConfig | object | `{}` | Advanced configuration. You can pass here any valid Polaris or Quarkus configuration property. Any property that is defined here takes precedence over all the other configuration values generated by this chart. Properties can be passed "flattened" or as nested YAML objects (see examples below). Note: values should be strings; avoid using numbers, booleans, or other types. | | affinity | object | `{}` | Affinity and anti-affinity for polaris pods. See https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity. | | authentication | object | `{"authenticator":{"type":"default"},"tokenBroker":{"maxTokenGeneration":"PT1H","secret":{"name":null,"privateKey":"private.pem","publicKey":"public.pem","secretKey":"secret"},"type":"rsa-key-pair"},"tokenService":{"type":"default"}}` | Polaris authentication configuration. | @@ -220,7 +220,7 @@ kubectl delete namespace polaris | extraVolumeMounts | list | `[]` | Extra volume mounts to add to the polaris container. See https://kubernetes.io/docs/concepts/storage/volumes/. | | extraVolumes | list | `[]` | Extra volumes to add to the polaris pod. See https://kubernetes.io/docs/concepts/storage/volumes/. | | features | object | `{"defaults":{},"realmOverrides":{}}` | Polaris features configuration. | -| features | object | `{}` | Features to enable or disable globally. If a feature is not present in the map, the default built-in value is used. | +| features.defaults | object | `{}` | Features to enable or disable globally. If a feature is not present in the map, the default built-in value is used. | | features.realmOverrides | object | `{}` | Features to enable or disable per realm. This field is a map of maps. The realm name is the key, and the value is a map of feature names to values. If a feature is not present in the map, the default value from the 'defaults' field is used. | | fileIo | object | `{"type":"default"}` | Polaris FileIO configuration. | | fileIo.type | string | `"default"` | The type of file IO to use. Two built-in types are supported: default and wasb. The wasb one translates WASB paths to ABFS ones. | @@ -285,7 +285,7 @@ kubectl delete namespace polaris | persistence.eclipseLink.secret | object | `{"key":"persistence.xml","name":null}` | The secret name to pull persistence.xml from. | | persistence.eclipseLink.secret.key | string | `"persistence.xml"` | The key in the secret to pull persistence.xml from. | | persistence.eclipseLink.secret.name | string | `nil` | The name of the secret to pull persistence.xml from. If not provided, the default built-in persistence.xml will be used. This is probably not what you want. | -| persistence.type | string | `"relational-jdbc"` | Three built-in types are available: "relational-jdbc", "in-memory", "eclipse-link". The in-memory type is not recommended for production use. The eclipse-link type is deprecated and will be unsupported in a future release. | +| persistence.type | string | `"eclipse-link"` | The type of persistence to use. Two built-in types are supported: in-memory and eclipse-link. | | podAnnotations | object | `{}` | Annotations to apply to polaris pods. | | podLabels | object | `{}` | Additional Labels to apply to polaris pods. | | podSecurityContext | object | `{"fsGroup":10001,"seccompProfile":{"type":"RuntimeDefault"}}` | Security context for the polaris pod. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/. | @@ -343,4 +343,4 @@ kubectl delete namespace polaris | tracing.attributes | object | `{}` | Resource attributes to identify the polaris service among other tracing sources. See https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/#service. If left empty, traces will be attached to a service named "Apache Polaris"; to change this, provide a service.name attribute here. | | tracing.enabled | bool | `false` | Specifies whether tracing for the polaris server should be enabled. | | tracing.endpoint | string | `"http://otlp-collector:4317"` | The collector endpoint URL to connect to (required). The endpoint URL must have either the http:// or the https:// scheme. The collector must talk the OpenTelemetry protocol (OTLP) and the port must be its gRPC port (by default 4317). See https://quarkus.io/guides/opentelemetry for more information. | -| tracing.sample | string | `"1.0d"` | Which requests should be sampled. Valid values are: "all", "none", or a ratio between 0.0 and "1.0d" (inclusive). E.g. "0.5d" means that 50% of the requests will be sampled. Note: avoid entering numbers here, always prefer a string representation of the ratio. | \ No newline at end of file +| tracing.sample | string | `"1.0d"` | Which requests should be sampled. Valid values are: "all", "none", or a ratio between 0.0 and "1.0d" (inclusive). E.g. "0.5d" means that 50% of the requests will be sampled. Note: avoid entering numbers here, always prefer a string representation of the ratio. | From 61383a8a9096ee83a8a481e27bd4a9f299c9db56 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 16:20:02 -0700 Subject: [PATCH 19/27] autolint --- .../catalog/iceberg/CatalogHandlerUtils.java | 973 +++++++++--------- 1 file changed, 486 insertions(+), 487 deletions(-) diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java index 75217c9b6e..0c4420ce8a 100644 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java +++ b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java @@ -23,6 +23,9 @@ import static org.apache.iceberg.TableProperties.COMMIT_NUM_RETRIES_DEFAULT; import static org.apache.iceberg.TableProperties.COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT; +import com.google.common.base.Preconditions; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.Collections; @@ -32,10 +35,6 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; - -import com.google.common.base.Preconditions; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; import org.apache.iceberg.BaseMetadataTable; import org.apache.iceberg.BaseTable; import org.apache.iceberg.BaseTransaction; @@ -88,512 +87,512 @@ * CODE_COPIED_TO_POLARIS Iceberg 1.8.0 */ public class CatalogHandlerUtils { - private static final Schema EMPTY_SCHEMA = new Schema(); - private static final String INITIAL_PAGE_TOKEN = ""; - - private CatalogHandlerUtils() {} - - /** - * Exception used to avoid retrying commits when assertions fail. - * - *

When a REST assertion fails, it will throw CommitFailedException to send back to the client. - * But the assertion checks happen in the block that is retried if {@link - * TableOperations#commit(TableMetadata, TableMetadata)} throws CommitFailedException. This is - * used to avoid retries for assertion failures, which are unwrapped and rethrown outside of the - * commit loop. - */ - private static class ValidationFailureException extends RuntimeException { - private final CommitFailedException wrapped; - - private ValidationFailureException(CommitFailedException cause) { - super(cause); - this.wrapped = cause; - } - - public CommitFailedException wrapped() { - return wrapped; - } - } - - private static Pair, String> paginate(List list, String pageToken, int pageSize) { - int pageStart = INITIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken); - if (pageStart >= list.size()) { - return Pair.of(Collections.emptyList(), null); - } - - int end = Math.min(pageStart + pageSize, list.size()); - List subList = list.subList(pageStart, end); - String nextPageToken = end >= list.size() ? null : String.valueOf(end); - - return Pair.of(subList, nextPageToken); - } - - public static ListNamespacesResponse listNamespaces( - SupportsNamespaces catalog, Namespace parent) { - List results; - if (parent.isEmpty()) { - results = catalog.listNamespaces(); - } else { - results = catalog.listNamespaces(parent); - } - - return ListNamespacesResponse.builder().addAll(results).build(); - } - - public static ListNamespacesResponse listNamespaces( - SupportsNamespaces catalog, Namespace parent, String pageToken, String pageSize) { - List results; - - if (parent.isEmpty()) { - results = catalog.listNamespaces(); - } else { - results = catalog.listNamespaces(parent); - } - - Pair, String> page = paginate(results, pageToken, Integer.parseInt(pageSize)); - - return ListNamespacesResponse.builder() - .addAll(page.first()) - .nextPageToken(page.second()) - .build(); - } + private static final Schema EMPTY_SCHEMA = new Schema(); + private static final String INITIAL_PAGE_TOKEN = ""; - public static CreateNamespaceResponse createNamespace( - SupportsNamespaces catalog, CreateNamespaceRequest request) { - Namespace namespace = request.namespace(); - catalog.createNamespace(namespace, request.properties()); - return CreateNamespaceResponse.builder() - .withNamespace(namespace) - .setProperties(catalog.loadNamespaceMetadata(namespace)) - .build(); - } + private CatalogHandlerUtils() {} + + /** + * Exception used to avoid retrying commits when assertions fail. + * + *

When a REST assertion fails, it will throw CommitFailedException to send back to the client. + * But the assertion checks happen in the block that is retried if {@link + * TableOperations#commit(TableMetadata, TableMetadata)} throws CommitFailedException. This is + * used to avoid retries for assertion failures, which are unwrapped and rethrown outside of the + * commit loop. + */ + private static class ValidationFailureException extends RuntimeException { + private final CommitFailedException wrapped; - public static void namespaceExists(SupportsNamespaces catalog, Namespace namespace) { - if (!catalog.namespaceExists(namespace)) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } + private ValidationFailureException(CommitFailedException cause) { + super(cause); + this.wrapped = cause; } - public static GetNamespaceResponse loadNamespace( - SupportsNamespaces catalog, Namespace namespace) { - Map properties = catalog.loadNamespaceMetadata(namespace); - return GetNamespaceResponse.builder() - .withNamespace(namespace) - .setProperties(properties) - .build(); + public CommitFailedException wrapped() { + return wrapped; } + } - public static void dropNamespace(SupportsNamespaces catalog, Namespace namespace) { - boolean dropped = catalog.dropNamespace(namespace); - if (!dropped) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } + private static Pair, String> paginate(List list, String pageToken, int pageSize) { + int pageStart = INITIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken); + if (pageStart >= list.size()) { + return Pair.of(Collections.emptyList(), null); } - public static UpdateNamespacePropertiesResponse updateNamespaceProperties( - SupportsNamespaces catalog, Namespace namespace, UpdateNamespacePropertiesRequest request) { - request.validate(); - - Set removals = Sets.newHashSet(request.removals()); - Map updates = request.updates(); - - Map startProperties = catalog.loadNamespaceMetadata(namespace); - Set missing = Sets.difference(removals, startProperties.keySet()); - - if (!updates.isEmpty()) { - catalog.setProperties(namespace, updates); - } + int end = Math.min(pageStart + pageSize, list.size()); + List subList = list.subList(pageStart, end); + String nextPageToken = end >= list.size() ? null : String.valueOf(end); - if (!removals.isEmpty()) { - // remove the original set just in case there was an update just after loading properties - catalog.removeProperties(namespace, removals); - } + return Pair.of(subList, nextPageToken); + } - return UpdateNamespacePropertiesResponse.builder() - .addMissing(missing) - .addUpdated(updates.keySet()) - .addRemoved(Sets.difference(removals, missing)) - .build(); + public static ListNamespacesResponse listNamespaces( + SupportsNamespaces catalog, Namespace parent) { + List results; + if (parent.isEmpty()) { + results = catalog.listNamespaces(); + } else { + results = catalog.listNamespaces(parent); } - public static ListTablesResponse listTables(Catalog catalog, Namespace namespace) { - List idents = catalog.listTables(namespace); - return ListTablesResponse.builder().addAll(idents).build(); - } - - public static ListTablesResponse listTables( - Catalog catalog, Namespace namespace, String pageToken, String pageSize) { - List results = catalog.listTables(namespace); + return ListNamespacesResponse.builder().addAll(results).build(); + } - Pair, String> page = - paginate(results, pageToken, Integer.parseInt(pageSize)); + public static ListNamespacesResponse listNamespaces( + SupportsNamespaces catalog, Namespace parent, String pageToken, String pageSize) { + List results; - return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); + if (parent.isEmpty()) { + results = catalog.listNamespaces(); + } else { + results = catalog.listNamespaces(parent); } - public static LoadTableResponse stageTableCreate( - Catalog catalog, Namespace namespace, CreateTableRequest request) { - request.validate(); - - TableIdentifier ident = TableIdentifier.of(namespace, request.name()); - if (catalog.tableExists(ident)) { - throw new AlreadyExistsException("Table already exists: %s", ident); - } - - Map properties = Maps.newHashMap(); - properties.put("created-at", OffsetDateTime.now(ZoneOffset.UTC).toString()); - properties.putAll(request.properties()); - - String location; - if (request.location() != null) { - location = request.location(); - } else { - location = - catalog - .buildTable(ident, request.schema()) - .withPartitionSpec(request.spec()) - .withSortOrder(request.writeOrder()) - .withProperties(properties) - .createTransaction() - .table() - .location(); - } - - TableMetadata metadata = - TableMetadata.newTableMetadata( - request.schema(), - request.spec() != null ? request.spec() : PartitionSpec.unpartitioned(), - request.writeOrder() != null ? request.writeOrder() : SortOrder.unsorted(), - location, - properties); - - return LoadTableResponse.builder().withTableMetadata(metadata).build(); - } + Pair, String> page = paginate(results, pageToken, Integer.parseInt(pageSize)); - public static LoadTableResponse createTable( - Catalog catalog, Namespace namespace, CreateTableRequest request) { - request.validate(); - - TableIdentifier ident = TableIdentifier.of(namespace, request.name()); - Table table = - catalog - .buildTable(ident, request.schema()) - .withLocation(request.location()) - .withPartitionSpec(request.spec()) - .withSortOrder(request.writeOrder()) - .withProperties(request.properties()) - .create(); - - if (table instanceof BaseTable) { - return LoadTableResponse.builder() - .withTableMetadata(((BaseTable) table).operations().current()) - .build(); - } + return ListNamespacesResponse.builder() + .addAll(page.first()) + .nextPageToken(page.second()) + .build(); + } - throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - } + public static CreateNamespaceResponse createNamespace( + SupportsNamespaces catalog, CreateNamespaceRequest request) { + Namespace namespace = request.namespace(); + catalog.createNamespace(namespace, request.properties()); + return CreateNamespaceResponse.builder() + .withNamespace(namespace) + .setProperties(catalog.loadNamespaceMetadata(namespace)) + .build(); + } + + public static void namespaceExists(SupportsNamespaces catalog, Namespace namespace) { + if (!catalog.namespaceExists(namespace)) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + } - public static LoadTableResponse registerTable( - Catalog catalog, Namespace namespace, RegisterTableRequest request) { - request.validate(); + public static GetNamespaceResponse loadNamespace( + SupportsNamespaces catalog, Namespace namespace) { + Map properties = catalog.loadNamespaceMetadata(namespace); + return GetNamespaceResponse.builder() + .withNamespace(namespace) + .setProperties(properties) + .build(); + } + + public static void dropNamespace(SupportsNamespaces catalog, Namespace namespace) { + boolean dropped = catalog.dropNamespace(namespace); + if (!dropped) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + } + + public static UpdateNamespacePropertiesResponse updateNamespaceProperties( + SupportsNamespaces catalog, Namespace namespace, UpdateNamespacePropertiesRequest request) { + request.validate(); + + Set removals = Sets.newHashSet(request.removals()); + Map updates = request.updates(); + + Map startProperties = catalog.loadNamespaceMetadata(namespace); + Set missing = Sets.difference(removals, startProperties.keySet()); + + if (!updates.isEmpty()) { + catalog.setProperties(namespace, updates); + } - TableIdentifier identifier = TableIdentifier.of(namespace, request.name()); - Table table = catalog.registerTable(identifier, request.metadataLocation()); - if (table instanceof BaseTable) { - return LoadTableResponse.builder() - .withTableMetadata(((BaseTable) table).operations().current()) - .build(); - } - - throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + if (!removals.isEmpty()) { + // remove the original set just in case there was an update just after loading properties + catalog.removeProperties(namespace, removals); } - public static void dropTable(Catalog catalog, TableIdentifier ident) { - boolean dropped = catalog.dropTable(ident, false); - if (!dropped) { - throw new NoSuchTableException("Table does not exist: %s", ident); - } - } + return UpdateNamespacePropertiesResponse.builder() + .addMissing(missing) + .addUpdated(updates.keySet()) + .addRemoved(Sets.difference(removals, missing)) + .build(); + } + + public static ListTablesResponse listTables(Catalog catalog, Namespace namespace) { + List idents = catalog.listTables(namespace); + return ListTablesResponse.builder().addAll(idents).build(); + } + + public static ListTablesResponse listTables( + Catalog catalog, Namespace namespace, String pageToken, String pageSize) { + List results = catalog.listTables(namespace); + + Pair, String> page = + paginate(results, pageToken, Integer.parseInt(pageSize)); + + return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); + } + + public static LoadTableResponse stageTableCreate( + Catalog catalog, Namespace namespace, CreateTableRequest request) { + request.validate(); - public static void purgeTable(Catalog catalog, TableIdentifier ident) { - boolean dropped = catalog.dropTable(ident, true); - if (!dropped) { - throw new NoSuchTableException("Table does not exist: %s", ident); - } - } - - public static void tableExists(Catalog catalog, TableIdentifier ident) { - boolean exists = catalog.tableExists(ident); - if (!exists) { - throw new NoSuchTableException("Table does not exist: %s", ident); - } - } - - public static LoadTableResponse loadTable(Catalog catalog, TableIdentifier ident) { - Table table = catalog.loadTable(ident); - - if (table instanceof BaseTable) { - return LoadTableResponse.builder() - .withTableMetadata(((BaseTable) table).operations().current()) - .build(); - } else if (table instanceof BaseMetadataTable) { - // metadata tables are loaded on the client side, return NoSuchTableException for now - throw new NoSuchTableException("Table does not exist: %s", ident.toString()); - } + TableIdentifier ident = TableIdentifier.of(namespace, request.name()); + if (catalog.tableExists(ident)) { + throw new AlreadyExistsException("Table already exists: %s", ident); + } + + Map properties = Maps.newHashMap(); + properties.put("created-at", OffsetDateTime.now(ZoneOffset.UTC).toString()); + properties.putAll(request.properties()); + String location; + if (request.location() != null) { + location = request.location(); + } else { + location = + catalog + .buildTable(ident, request.schema()) + .withPartitionSpec(request.spec()) + .withSortOrder(request.writeOrder()) + .withProperties(properties) + .createTransaction() + .table() + .location(); + } + + TableMetadata metadata = + TableMetadata.newTableMetadata( + request.schema(), + request.spec() != null ? request.spec() : PartitionSpec.unpartitioned(), + request.writeOrder() != null ? request.writeOrder() : SortOrder.unsorted(), + location, + properties); + + return LoadTableResponse.builder().withTableMetadata(metadata).build(); + } + + public static LoadTableResponse createTable( + Catalog catalog, Namespace namespace, CreateTableRequest request) { + request.validate(); + + TableIdentifier ident = TableIdentifier.of(namespace, request.name()); + Table table = + catalog + .buildTable(ident, request.schema()) + .withLocation(request.location()) + .withPartitionSpec(request.spec()) + .withSortOrder(request.writeOrder()) + .withProperties(request.properties()) + .create(); + + if (table instanceof BaseTable) { + return LoadTableResponse.builder() + .withTableMetadata(((BaseTable) table).operations().current()) + .build(); + } + + throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + } + + public static LoadTableResponse registerTable( + Catalog catalog, Namespace namespace, RegisterTableRequest request) { + request.validate(); + + TableIdentifier identifier = TableIdentifier.of(namespace, request.name()); + Table table = catalog.registerTable(identifier, request.metadataLocation()); + if (table instanceof BaseTable) { + return LoadTableResponse.builder() + .withTableMetadata(((BaseTable) table).operations().current()) + .build(); + } + + throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + } + + public static void dropTable(Catalog catalog, TableIdentifier ident) { + boolean dropped = catalog.dropTable(ident, false); + if (!dropped) { + throw new NoSuchTableException("Table does not exist: %s", ident); + } + } + + public static void purgeTable(Catalog catalog, TableIdentifier ident) { + boolean dropped = catalog.dropTable(ident, true); + if (!dropped) { + throw new NoSuchTableException("Table does not exist: %s", ident); + } + } + + public static void tableExists(Catalog catalog, TableIdentifier ident) { + boolean exists = catalog.tableExists(ident); + if (!exists) { + throw new NoSuchTableException("Table does not exist: %s", ident); + } + } + + public static LoadTableResponse loadTable(Catalog catalog, TableIdentifier ident) { + Table table = catalog.loadTable(ident); + + if (table instanceof BaseTable) { + return LoadTableResponse.builder() + .withTableMetadata(((BaseTable) table).operations().current()) + .build(); + } else if (table instanceof BaseMetadataTable) { + // metadata tables are loaded on the client side, return NoSuchTableException for now + throw new NoSuchTableException("Table does not exist: %s", ident.toString()); + } + + throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); + } + + public static LoadTableResponse updateTable( + Catalog catalog, TableIdentifier ident, UpdateTableRequest request) { + TableMetadata finalMetadata; + if (isCreate(request)) { + // this is a hacky way to get TableOperations for an uncommitted table + Transaction transaction = + catalog.buildTable(ident, EMPTY_SCHEMA).createOrReplaceTransaction(); + if (transaction instanceof BaseTransaction) { + BaseTransaction baseTransaction = (BaseTransaction) transaction; + finalMetadata = create(baseTransaction.underlyingOps(), request); + } else { + throw new IllegalStateException( + "Cannot wrap catalog that does not produce BaseTransaction"); + } + + } else { + Table table = catalog.loadTable(ident); + if (table instanceof BaseTable) { + TableOperations ops = ((BaseTable) table).operations(); + finalMetadata = commit(ops, request); + } else { throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - } - - public static LoadTableResponse updateTable( - Catalog catalog, TableIdentifier ident, UpdateTableRequest request) { - TableMetadata finalMetadata; - if (isCreate(request)) { - // this is a hacky way to get TableOperations for an uncommitted table - Transaction transaction = - catalog.buildTable(ident, EMPTY_SCHEMA).createOrReplaceTransaction(); - if (transaction instanceof BaseTransaction) { - BaseTransaction baseTransaction = (BaseTransaction) transaction; - finalMetadata = create(baseTransaction.underlyingOps(), request); - } else { - throw new IllegalStateException( - "Cannot wrap catalog that does not produce BaseTransaction"); - } - - } else { - Table table = catalog.loadTable(ident); - if (table instanceof BaseTable) { - TableOperations ops = ((BaseTable) table).operations(); - finalMetadata = commit(ops, request); - } else { - throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - } - } - - return LoadTableResponse.builder().withTableMetadata(finalMetadata).build(); - } - - public static void renameTable(Catalog catalog, RenameTableRequest request) { - catalog.renameTable(request.source(), request.destination()); - } - - private static boolean isCreate(UpdateTableRequest request) { - boolean isCreate = - request.requirements().stream() - .anyMatch(UpdateRequirement.AssertTableDoesNotExist.class::isInstance); - - if (isCreate) { - List invalidRequirements = - request.requirements().stream() - .filter(req -> !(req instanceof UpdateRequirement.AssertTableDoesNotExist)) - .collect(Collectors.toList()); - Preconditions.checkArgument( - invalidRequirements.isEmpty(), "Invalid create requirements: %s", invalidRequirements); - } - - return isCreate; - } - - private static TableMetadata create(TableOperations ops, UpdateTableRequest request) { - // the only valid requirement is that the table will be created - request.requirements().forEach(requirement -> requirement.validate(ops.current())); - Optional formatVersion = - request.updates().stream() - .filter(update -> update instanceof UpgradeFormatVersion) - .map(update -> ((UpgradeFormatVersion) update).formatVersion()) - .findFirst(); - - TableMetadata.Builder builder = - formatVersion.map(TableMetadata::buildFromEmpty).orElseGet(TableMetadata::buildFromEmpty); - request.updates().forEach(update -> update.applyTo(builder)); - // create transactions do not retry. if the table exists, retrying is not a solution - ops.commit(null, builder.build()); - - return ops.current(); - } - - static TableMetadata commit(TableOperations ops, UpdateTableRequest request) { - AtomicBoolean isRetry = new AtomicBoolean(false); - try { - Tasks.foreach(ops) - .retry(COMMIT_NUM_RETRIES_DEFAULT) - .exponentialBackoff( - COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, - COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, - COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, - 2.0 /* exponential */) - .onlyRetryOn(CommitFailedException.class) - .run( - taskOps -> { - TableMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); - isRetry.set(true); - - // validate requirements - try { - request.requirements().forEach(requirement -> requirement.validate(base)); - } catch (CommitFailedException e) { - // wrap and rethrow outside of tasks to avoid unnecessary retry - throw new ValidationFailureException(e); - } - - // apply changes - TableMetadata.Builder metadataBuilder = TableMetadata.buildFrom(base); - request.updates().forEach(update -> update.applyTo(metadataBuilder)); - - TableMetadata updated = metadataBuilder.build(); - if (updated.changes().isEmpty()) { - // do not commit if the metadata has not changed - return; - } - - // commit - taskOps.commit(base, updated); - }); - - } catch (ValidationFailureException e) { - throw e.wrapped(); - } - - return ops.current(); - } - - private static BaseView asBaseView(View view) { - Preconditions.checkState( - view instanceof BaseView, "Cannot wrap catalog that does not produce BaseView"); - return (BaseView) view; - } - - public static ListTablesResponse listViews(ViewCatalog catalog, Namespace namespace) { - return ListTablesResponse.builder().addAll(catalog.listViews(namespace)).build(); - } - - public static ListTablesResponse listViews( - ViewCatalog catalog, Namespace namespace, String pageToken, String pageSize) { - List results = catalog.listViews(namespace); - - Pair, String> page = - paginate(results, pageToken, Integer.parseInt(pageSize)); - - return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); - } - - public static LoadViewResponse createView( - ViewCatalog catalog, Namespace namespace, CreateViewRequest request) { - request.validate(); - - ViewBuilder viewBuilder = - catalog - .buildView(TableIdentifier.of(namespace, request.name())) - .withSchema(request.schema()) - .withProperties(request.properties()) - .withDefaultNamespace(request.viewVersion().defaultNamespace()) - .withDefaultCatalog(request.viewVersion().defaultCatalog()) - .withLocation(request.location()); - - Set unsupportedRepresentations = - request.viewVersion().representations().stream() - .filter(r -> !(r instanceof SQLViewRepresentation)) - .map(ViewRepresentation::type) - .collect(Collectors.toSet()); - - if (!unsupportedRepresentations.isEmpty()) { - throw new IllegalStateException( - String.format("Found unsupported view representations: %s", unsupportedRepresentations)); - } - + } + } + + return LoadTableResponse.builder().withTableMetadata(finalMetadata).build(); + } + + public static void renameTable(Catalog catalog, RenameTableRequest request) { + catalog.renameTable(request.source(), request.destination()); + } + + private static boolean isCreate(UpdateTableRequest request) { + boolean isCreate = + request.requirements().stream() + .anyMatch(UpdateRequirement.AssertTableDoesNotExist.class::isInstance); + + if (isCreate) { + List invalidRequirements = + request.requirements().stream() + .filter(req -> !(req instanceof UpdateRequirement.AssertTableDoesNotExist)) + .collect(Collectors.toList()); + Preconditions.checkArgument( + invalidRequirements.isEmpty(), "Invalid create requirements: %s", invalidRequirements); + } + + return isCreate; + } + + private static TableMetadata create(TableOperations ops, UpdateTableRequest request) { + // the only valid requirement is that the table will be created + request.requirements().forEach(requirement -> requirement.validate(ops.current())); + Optional formatVersion = + request.updates().stream() + .filter(update -> update instanceof UpgradeFormatVersion) + .map(update -> ((UpgradeFormatVersion) update).formatVersion()) + .findFirst(); + + TableMetadata.Builder builder = + formatVersion.map(TableMetadata::buildFromEmpty).orElseGet(TableMetadata::buildFromEmpty); + request.updates().forEach(update -> update.applyTo(builder)); + // create transactions do not retry. if the table exists, retrying is not a solution + ops.commit(null, builder.build()); + + return ops.current(); + } + + static TableMetadata commit(TableOperations ops, UpdateTableRequest request) { + AtomicBoolean isRetry = new AtomicBoolean(false); + try { + Tasks.foreach(ops) + .retry(COMMIT_NUM_RETRIES_DEFAULT) + .exponentialBackoff( + COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, + COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, + COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, + 2.0 /* exponential */) + .onlyRetryOn(CommitFailedException.class) + .run( + taskOps -> { + TableMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); + isRetry.set(true); + + // validate requirements + try { + request.requirements().forEach(requirement -> requirement.validate(base)); + } catch (CommitFailedException e) { + // wrap and rethrow outside of tasks to avoid unnecessary retry + throw new ValidationFailureException(e); + } + + // apply changes + TableMetadata.Builder metadataBuilder = TableMetadata.buildFrom(base); + request.updates().forEach(update -> update.applyTo(metadataBuilder)); + + TableMetadata updated = metadataBuilder.build(); + if (updated.changes().isEmpty()) { + // do not commit if the metadata has not changed + return; + } + + // commit + taskOps.commit(base, updated); + }); + + } catch (ValidationFailureException e) { + throw e.wrapped(); + } + + return ops.current(); + } + + private static BaseView asBaseView(View view) { + Preconditions.checkState( + view instanceof BaseView, "Cannot wrap catalog that does not produce BaseView"); + return (BaseView) view; + } + + public static ListTablesResponse listViews(ViewCatalog catalog, Namespace namespace) { + return ListTablesResponse.builder().addAll(catalog.listViews(namespace)).build(); + } + + public static ListTablesResponse listViews( + ViewCatalog catalog, Namespace namespace, String pageToken, String pageSize) { + List results = catalog.listViews(namespace); + + Pair, String> page = + paginate(results, pageToken, Integer.parseInt(pageSize)); + + return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); + } + + public static LoadViewResponse createView( + ViewCatalog catalog, Namespace namespace, CreateViewRequest request) { + request.validate(); + + ViewBuilder viewBuilder = + catalog + .buildView(TableIdentifier.of(namespace, request.name())) + .withSchema(request.schema()) + .withProperties(request.properties()) + .withDefaultNamespace(request.viewVersion().defaultNamespace()) + .withDefaultCatalog(request.viewVersion().defaultCatalog()) + .withLocation(request.location()); + + Set unsupportedRepresentations = request.viewVersion().representations().stream() - .filter(SQLViewRepresentation.class::isInstance) - .map(SQLViewRepresentation.class::cast) - .forEach(r -> viewBuilder.withQuery(r.dialect(), r.sql())); - - View view = viewBuilder.create(); - - return viewResponse(view); - } - - private static LoadViewResponse viewResponse(View view) { - ViewMetadata metadata = asBaseView(view).operations().current(); - return ImmutableLoadViewResponse.builder() - .metadata(metadata) - .metadataLocation(metadata.metadataFileLocation()) - .build(); - } - - public static void viewExists(ViewCatalog catalog, TableIdentifier viewIdentifier) { - if (!catalog.viewExists(viewIdentifier)) { - throw new NoSuchViewException("View does not exist: %s", viewIdentifier); - } - } - - public static LoadViewResponse loadView(ViewCatalog catalog, TableIdentifier viewIdentifier) { - View view = catalog.loadView(viewIdentifier); - return viewResponse(view); - } - - public static LoadViewResponse updateView( - ViewCatalog catalog, TableIdentifier ident, UpdateTableRequest request) { - View view = catalog.loadView(ident); - ViewMetadata metadata = commit(asBaseView(view).operations(), request); - - return ImmutableLoadViewResponse.builder() - .metadata(metadata) - .metadataLocation(metadata.metadataFileLocation()) - .build(); - } - - public static void renameView(ViewCatalog catalog, RenameTableRequest request) { - catalog.renameView(request.source(), request.destination()); - } - - public static void dropView(ViewCatalog catalog, TableIdentifier viewIdentifier) { - boolean dropped = catalog.dropView(viewIdentifier); - if (!dropped) { - throw new NoSuchViewException("View does not exist: %s", viewIdentifier); - } - } - - static ViewMetadata commit(ViewOperations ops, UpdateTableRequest request) { - AtomicBoolean isRetry = new AtomicBoolean(false); - try { - Tasks.foreach(ops) - .retry(COMMIT_NUM_RETRIES_DEFAULT) - .exponentialBackoff( - COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, - COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, - COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, - 2.0 /* exponential */) - .onlyRetryOn(CommitFailedException.class) - .run( - taskOps -> { - ViewMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); - isRetry.set(true); - - // validate requirements - try { - request.requirements().forEach(requirement -> requirement.validate(base)); - } catch (CommitFailedException e) { - // wrap and rethrow outside of tasks to avoid unnecessary retry - throw new ValidationFailureException(e); - } - - // apply changes - ViewMetadata.Builder metadataBuilder = ViewMetadata.buildFrom(base); - request.updates().forEach(update -> update.applyTo(metadataBuilder)); - - ViewMetadata updated = metadataBuilder.build(); - - if (updated.changes().isEmpty()) { - // do not commit if the metadata has not changed - return; - } - - // commit - taskOps.commit(base, updated); - }); - - } catch (ValidationFailureException e) { - throw e.wrapped(); - } - - return ops.current(); - } -} \ No newline at end of file + .filter(r -> !(r instanceof SQLViewRepresentation)) + .map(ViewRepresentation::type) + .collect(Collectors.toSet()); + + if (!unsupportedRepresentations.isEmpty()) { + throw new IllegalStateException( + String.format("Found unsupported view representations: %s", unsupportedRepresentations)); + } + + request.viewVersion().representations().stream() + .filter(SQLViewRepresentation.class::isInstance) + .map(SQLViewRepresentation.class::cast) + .forEach(r -> viewBuilder.withQuery(r.dialect(), r.sql())); + + View view = viewBuilder.create(); + + return viewResponse(view); + } + + private static LoadViewResponse viewResponse(View view) { + ViewMetadata metadata = asBaseView(view).operations().current(); + return ImmutableLoadViewResponse.builder() + .metadata(metadata) + .metadataLocation(metadata.metadataFileLocation()) + .build(); + } + + public static void viewExists(ViewCatalog catalog, TableIdentifier viewIdentifier) { + if (!catalog.viewExists(viewIdentifier)) { + throw new NoSuchViewException("View does not exist: %s", viewIdentifier); + } + } + + public static LoadViewResponse loadView(ViewCatalog catalog, TableIdentifier viewIdentifier) { + View view = catalog.loadView(viewIdentifier); + return viewResponse(view); + } + + public static LoadViewResponse updateView( + ViewCatalog catalog, TableIdentifier ident, UpdateTableRequest request) { + View view = catalog.loadView(ident); + ViewMetadata metadata = commit(asBaseView(view).operations(), request); + + return ImmutableLoadViewResponse.builder() + .metadata(metadata) + .metadataLocation(metadata.metadataFileLocation()) + .build(); + } + + public static void renameView(ViewCatalog catalog, RenameTableRequest request) { + catalog.renameView(request.source(), request.destination()); + } + + public static void dropView(ViewCatalog catalog, TableIdentifier viewIdentifier) { + boolean dropped = catalog.dropView(viewIdentifier); + if (!dropped) { + throw new NoSuchViewException("View does not exist: %s", viewIdentifier); + } + } + + static ViewMetadata commit(ViewOperations ops, UpdateTableRequest request) { + AtomicBoolean isRetry = new AtomicBoolean(false); + try { + Tasks.foreach(ops) + .retry(COMMIT_NUM_RETRIES_DEFAULT) + .exponentialBackoff( + COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, + COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, + COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, + 2.0 /* exponential */) + .onlyRetryOn(CommitFailedException.class) + .run( + taskOps -> { + ViewMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); + isRetry.set(true); + + // validate requirements + try { + request.requirements().forEach(requirement -> requirement.validate(base)); + } catch (CommitFailedException e) { + // wrap and rethrow outside of tasks to avoid unnecessary retry + throw new ValidationFailureException(e); + } + + // apply changes + ViewMetadata.Builder metadataBuilder = ViewMetadata.buildFrom(base); + request.updates().forEach(update -> update.applyTo(metadataBuilder)); + + ViewMetadata updated = metadataBuilder.build(); + + if (updated.changes().isEmpty()) { + // do not commit if the metadata has not changed + return; + } + + // commit + taskOps.commit(base, updated); + }); + + } catch (ValidationFailureException e) { + throw e.wrapped(); + } + + return ops.current(); + } +} From ba263af047b5d4b3b9e8ffb77f6f4c7b47ed4198 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 16:33:12 -0700 Subject: [PATCH 20/27] no dots in props --- .../polaris/core/config/PolarisConfiguration.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 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 bcb3809881..658ba03b20 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 @@ -97,17 +97,22 @@ public Builder catalogConfig(String catalogConfig) { return this; } - public FeatureConfiguration buildFeatureConfiguration() { + private void validateOrThrow() { if (key == null || description == null || defaultValue == null) { throw new IllegalArgumentException("key, description, and defaultValue are required"); } + if (key.contains(".")) { + throw new IllegalArgumentException("key cannot contain `.`"); + } + } + + public FeatureConfiguration buildFeatureConfiguration() { + validateOrThrow(); return new FeatureConfiguration<>(key, description, defaultValue, catalogConfig); } public BehaviorChangeConfiguration buildBehaviorChangeConfiguration() { - if (key == null || description == null || defaultValue == null) { - throw new IllegalArgumentException("key, description, and defaultValue are required"); - } + validateOrThrow(); if (catalogConfig.isPresent()) { throw new IllegalArgumentException( "catalogConfig is not valid for behavior change configs"); From d90a4f8bf3985632b5f5c0c743868791b305a055 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 12 May 2025 23:50:09 -0700 Subject: [PATCH 21/27] remove accidental file --- .../catalog/iceberg/CatalogHandlerUtils.java | 598 ------------------ 1 file changed, 598 deletions(-) delete mode 100644 service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java diff --git a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java deleted file mode 100644 index 0c4420ce8a..0000000000 --- a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java +++ /dev/null @@ -1,598 +0,0 @@ -/* - * 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.service.catalog.iceberg; - -import static org.apache.iceberg.TableProperties.COMMIT_MAX_RETRY_WAIT_MS_DEFAULT; -import static org.apache.iceberg.TableProperties.COMMIT_MIN_RETRY_WAIT_MS_DEFAULT; -import static org.apache.iceberg.TableProperties.COMMIT_NUM_RETRIES_DEFAULT; -import static org.apache.iceberg.TableProperties.COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT; - -import com.google.common.base.Preconditions; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; -import java.time.OffsetDateTime; -import java.time.ZoneOffset; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; -import org.apache.iceberg.BaseMetadataTable; -import org.apache.iceberg.BaseTable; -import org.apache.iceberg.BaseTransaction; -import org.apache.iceberg.MetadataUpdate.UpgradeFormatVersion; -import org.apache.iceberg.PartitionSpec; -import org.apache.iceberg.Schema; -import org.apache.iceberg.SortOrder; -import org.apache.iceberg.Table; -import org.apache.iceberg.TableMetadata; -import org.apache.iceberg.TableOperations; -import org.apache.iceberg.Transaction; -import org.apache.iceberg.UpdateRequirement; -import org.apache.iceberg.catalog.Catalog; -import org.apache.iceberg.catalog.Namespace; -import org.apache.iceberg.catalog.SupportsNamespaces; -import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.catalog.ViewCatalog; -import org.apache.iceberg.exceptions.AlreadyExistsException; -import org.apache.iceberg.exceptions.CommitFailedException; -import org.apache.iceberg.exceptions.NoSuchNamespaceException; -import org.apache.iceberg.exceptions.NoSuchTableException; -import org.apache.iceberg.exceptions.NoSuchViewException; -import org.apache.iceberg.rest.requests.CreateNamespaceRequest; -import org.apache.iceberg.rest.requests.CreateTableRequest; -import org.apache.iceberg.rest.requests.CreateViewRequest; -import org.apache.iceberg.rest.requests.RegisterTableRequest; -import org.apache.iceberg.rest.requests.RenameTableRequest; -import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest; -import org.apache.iceberg.rest.requests.UpdateTableRequest; -import org.apache.iceberg.rest.responses.CreateNamespaceResponse; -import org.apache.iceberg.rest.responses.GetNamespaceResponse; -import org.apache.iceberg.rest.responses.ImmutableLoadViewResponse; -import org.apache.iceberg.rest.responses.ListNamespacesResponse; -import org.apache.iceberg.rest.responses.ListTablesResponse; -import org.apache.iceberg.rest.responses.LoadTableResponse; -import org.apache.iceberg.rest.responses.LoadViewResponse; -import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse; -import org.apache.iceberg.util.Pair; -import org.apache.iceberg.util.Tasks; -import org.apache.iceberg.view.BaseView; -import org.apache.iceberg.view.SQLViewRepresentation; -import org.apache.iceberg.view.View; -import org.apache.iceberg.view.ViewBuilder; -import org.apache.iceberg.view.ViewMetadata; -import org.apache.iceberg.view.ViewOperations; -import org.apache.iceberg.view.ViewRepresentation; - -/** - * A collection of utilities related to or copied from Iceberg's CatalogHandlers - * CODE_COPIED_TO_POLARIS Iceberg 1.8.0 - */ -public class CatalogHandlerUtils { - private static final Schema EMPTY_SCHEMA = new Schema(); - private static final String INITIAL_PAGE_TOKEN = ""; - - private CatalogHandlerUtils() {} - - /** - * Exception used to avoid retrying commits when assertions fail. - * - *

When a REST assertion fails, it will throw CommitFailedException to send back to the client. - * But the assertion checks happen in the block that is retried if {@link - * TableOperations#commit(TableMetadata, TableMetadata)} throws CommitFailedException. This is - * used to avoid retries for assertion failures, which are unwrapped and rethrown outside of the - * commit loop. - */ - private static class ValidationFailureException extends RuntimeException { - private final CommitFailedException wrapped; - - private ValidationFailureException(CommitFailedException cause) { - super(cause); - this.wrapped = cause; - } - - public CommitFailedException wrapped() { - return wrapped; - } - } - - private static Pair, String> paginate(List list, String pageToken, int pageSize) { - int pageStart = INITIAL_PAGE_TOKEN.equals(pageToken) ? 0 : Integer.parseInt(pageToken); - if (pageStart >= list.size()) { - return Pair.of(Collections.emptyList(), null); - } - - int end = Math.min(pageStart + pageSize, list.size()); - List subList = list.subList(pageStart, end); - String nextPageToken = end >= list.size() ? null : String.valueOf(end); - - return Pair.of(subList, nextPageToken); - } - - public static ListNamespacesResponse listNamespaces( - SupportsNamespaces catalog, Namespace parent) { - List results; - if (parent.isEmpty()) { - results = catalog.listNamespaces(); - } else { - results = catalog.listNamespaces(parent); - } - - return ListNamespacesResponse.builder().addAll(results).build(); - } - - public static ListNamespacesResponse listNamespaces( - SupportsNamespaces catalog, Namespace parent, String pageToken, String pageSize) { - List results; - - if (parent.isEmpty()) { - results = catalog.listNamespaces(); - } else { - results = catalog.listNamespaces(parent); - } - - Pair, String> page = paginate(results, pageToken, Integer.parseInt(pageSize)); - - return ListNamespacesResponse.builder() - .addAll(page.first()) - .nextPageToken(page.second()) - .build(); - } - - public static CreateNamespaceResponse createNamespace( - SupportsNamespaces catalog, CreateNamespaceRequest request) { - Namespace namespace = request.namespace(); - catalog.createNamespace(namespace, request.properties()); - return CreateNamespaceResponse.builder() - .withNamespace(namespace) - .setProperties(catalog.loadNamespaceMetadata(namespace)) - .build(); - } - - public static void namespaceExists(SupportsNamespaces catalog, Namespace namespace) { - if (!catalog.namespaceExists(namespace)) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } - } - - public static GetNamespaceResponse loadNamespace( - SupportsNamespaces catalog, Namespace namespace) { - Map properties = catalog.loadNamespaceMetadata(namespace); - return GetNamespaceResponse.builder() - .withNamespace(namespace) - .setProperties(properties) - .build(); - } - - public static void dropNamespace(SupportsNamespaces catalog, Namespace namespace) { - boolean dropped = catalog.dropNamespace(namespace); - if (!dropped) { - throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); - } - } - - public static UpdateNamespacePropertiesResponse updateNamespaceProperties( - SupportsNamespaces catalog, Namespace namespace, UpdateNamespacePropertiesRequest request) { - request.validate(); - - Set removals = Sets.newHashSet(request.removals()); - Map updates = request.updates(); - - Map startProperties = catalog.loadNamespaceMetadata(namespace); - Set missing = Sets.difference(removals, startProperties.keySet()); - - if (!updates.isEmpty()) { - catalog.setProperties(namespace, updates); - } - - if (!removals.isEmpty()) { - // remove the original set just in case there was an update just after loading properties - catalog.removeProperties(namespace, removals); - } - - return UpdateNamespacePropertiesResponse.builder() - .addMissing(missing) - .addUpdated(updates.keySet()) - .addRemoved(Sets.difference(removals, missing)) - .build(); - } - - public static ListTablesResponse listTables(Catalog catalog, Namespace namespace) { - List idents = catalog.listTables(namespace); - return ListTablesResponse.builder().addAll(idents).build(); - } - - public static ListTablesResponse listTables( - Catalog catalog, Namespace namespace, String pageToken, String pageSize) { - List results = catalog.listTables(namespace); - - Pair, String> page = - paginate(results, pageToken, Integer.parseInt(pageSize)); - - return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); - } - - public static LoadTableResponse stageTableCreate( - Catalog catalog, Namespace namespace, CreateTableRequest request) { - request.validate(); - - TableIdentifier ident = TableIdentifier.of(namespace, request.name()); - if (catalog.tableExists(ident)) { - throw new AlreadyExistsException("Table already exists: %s", ident); - } - - Map properties = Maps.newHashMap(); - properties.put("created-at", OffsetDateTime.now(ZoneOffset.UTC).toString()); - properties.putAll(request.properties()); - - String location; - if (request.location() != null) { - location = request.location(); - } else { - location = - catalog - .buildTable(ident, request.schema()) - .withPartitionSpec(request.spec()) - .withSortOrder(request.writeOrder()) - .withProperties(properties) - .createTransaction() - .table() - .location(); - } - - TableMetadata metadata = - TableMetadata.newTableMetadata( - request.schema(), - request.spec() != null ? request.spec() : PartitionSpec.unpartitioned(), - request.writeOrder() != null ? request.writeOrder() : SortOrder.unsorted(), - location, - properties); - - return LoadTableResponse.builder().withTableMetadata(metadata).build(); - } - - public static LoadTableResponse createTable( - Catalog catalog, Namespace namespace, CreateTableRequest request) { - request.validate(); - - TableIdentifier ident = TableIdentifier.of(namespace, request.name()); - Table table = - catalog - .buildTable(ident, request.schema()) - .withLocation(request.location()) - .withPartitionSpec(request.spec()) - .withSortOrder(request.writeOrder()) - .withProperties(request.properties()) - .create(); - - if (table instanceof BaseTable) { - return LoadTableResponse.builder() - .withTableMetadata(((BaseTable) table).operations().current()) - .build(); - } - - throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - } - - public static LoadTableResponse registerTable( - Catalog catalog, Namespace namespace, RegisterTableRequest request) { - request.validate(); - - TableIdentifier identifier = TableIdentifier.of(namespace, request.name()); - Table table = catalog.registerTable(identifier, request.metadataLocation()); - if (table instanceof BaseTable) { - return LoadTableResponse.builder() - .withTableMetadata(((BaseTable) table).operations().current()) - .build(); - } - - throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - } - - public static void dropTable(Catalog catalog, TableIdentifier ident) { - boolean dropped = catalog.dropTable(ident, false); - if (!dropped) { - throw new NoSuchTableException("Table does not exist: %s", ident); - } - } - - public static void purgeTable(Catalog catalog, TableIdentifier ident) { - boolean dropped = catalog.dropTable(ident, true); - if (!dropped) { - throw new NoSuchTableException("Table does not exist: %s", ident); - } - } - - public static void tableExists(Catalog catalog, TableIdentifier ident) { - boolean exists = catalog.tableExists(ident); - if (!exists) { - throw new NoSuchTableException("Table does not exist: %s", ident); - } - } - - public static LoadTableResponse loadTable(Catalog catalog, TableIdentifier ident) { - Table table = catalog.loadTable(ident); - - if (table instanceof BaseTable) { - return LoadTableResponse.builder() - .withTableMetadata(((BaseTable) table).operations().current()) - .build(); - } else if (table instanceof BaseMetadataTable) { - // metadata tables are loaded on the client side, return NoSuchTableException for now - throw new NoSuchTableException("Table does not exist: %s", ident.toString()); - } - - throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - } - - public static LoadTableResponse updateTable( - Catalog catalog, TableIdentifier ident, UpdateTableRequest request) { - TableMetadata finalMetadata; - if (isCreate(request)) { - // this is a hacky way to get TableOperations for an uncommitted table - Transaction transaction = - catalog.buildTable(ident, EMPTY_SCHEMA).createOrReplaceTransaction(); - if (transaction instanceof BaseTransaction) { - BaseTransaction baseTransaction = (BaseTransaction) transaction; - finalMetadata = create(baseTransaction.underlyingOps(), request); - } else { - throw new IllegalStateException( - "Cannot wrap catalog that does not produce BaseTransaction"); - } - - } else { - Table table = catalog.loadTable(ident); - if (table instanceof BaseTable) { - TableOperations ops = ((BaseTable) table).operations(); - finalMetadata = commit(ops, request); - } else { - throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable"); - } - } - - return LoadTableResponse.builder().withTableMetadata(finalMetadata).build(); - } - - public static void renameTable(Catalog catalog, RenameTableRequest request) { - catalog.renameTable(request.source(), request.destination()); - } - - private static boolean isCreate(UpdateTableRequest request) { - boolean isCreate = - request.requirements().stream() - .anyMatch(UpdateRequirement.AssertTableDoesNotExist.class::isInstance); - - if (isCreate) { - List invalidRequirements = - request.requirements().stream() - .filter(req -> !(req instanceof UpdateRequirement.AssertTableDoesNotExist)) - .collect(Collectors.toList()); - Preconditions.checkArgument( - invalidRequirements.isEmpty(), "Invalid create requirements: %s", invalidRequirements); - } - - return isCreate; - } - - private static TableMetadata create(TableOperations ops, UpdateTableRequest request) { - // the only valid requirement is that the table will be created - request.requirements().forEach(requirement -> requirement.validate(ops.current())); - Optional formatVersion = - request.updates().stream() - .filter(update -> update instanceof UpgradeFormatVersion) - .map(update -> ((UpgradeFormatVersion) update).formatVersion()) - .findFirst(); - - TableMetadata.Builder builder = - formatVersion.map(TableMetadata::buildFromEmpty).orElseGet(TableMetadata::buildFromEmpty); - request.updates().forEach(update -> update.applyTo(builder)); - // create transactions do not retry. if the table exists, retrying is not a solution - ops.commit(null, builder.build()); - - return ops.current(); - } - - static TableMetadata commit(TableOperations ops, UpdateTableRequest request) { - AtomicBoolean isRetry = new AtomicBoolean(false); - try { - Tasks.foreach(ops) - .retry(COMMIT_NUM_RETRIES_DEFAULT) - .exponentialBackoff( - COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, - COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, - COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, - 2.0 /* exponential */) - .onlyRetryOn(CommitFailedException.class) - .run( - taskOps -> { - TableMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); - isRetry.set(true); - - // validate requirements - try { - request.requirements().forEach(requirement -> requirement.validate(base)); - } catch (CommitFailedException e) { - // wrap and rethrow outside of tasks to avoid unnecessary retry - throw new ValidationFailureException(e); - } - - // apply changes - TableMetadata.Builder metadataBuilder = TableMetadata.buildFrom(base); - request.updates().forEach(update -> update.applyTo(metadataBuilder)); - - TableMetadata updated = metadataBuilder.build(); - if (updated.changes().isEmpty()) { - // do not commit if the metadata has not changed - return; - } - - // commit - taskOps.commit(base, updated); - }); - - } catch (ValidationFailureException e) { - throw e.wrapped(); - } - - return ops.current(); - } - - private static BaseView asBaseView(View view) { - Preconditions.checkState( - view instanceof BaseView, "Cannot wrap catalog that does not produce BaseView"); - return (BaseView) view; - } - - public static ListTablesResponse listViews(ViewCatalog catalog, Namespace namespace) { - return ListTablesResponse.builder().addAll(catalog.listViews(namespace)).build(); - } - - public static ListTablesResponse listViews( - ViewCatalog catalog, Namespace namespace, String pageToken, String pageSize) { - List results = catalog.listViews(namespace); - - Pair, String> page = - paginate(results, pageToken, Integer.parseInt(pageSize)); - - return ListTablesResponse.builder().addAll(page.first()).nextPageToken(page.second()).build(); - } - - public static LoadViewResponse createView( - ViewCatalog catalog, Namespace namespace, CreateViewRequest request) { - request.validate(); - - ViewBuilder viewBuilder = - catalog - .buildView(TableIdentifier.of(namespace, request.name())) - .withSchema(request.schema()) - .withProperties(request.properties()) - .withDefaultNamespace(request.viewVersion().defaultNamespace()) - .withDefaultCatalog(request.viewVersion().defaultCatalog()) - .withLocation(request.location()); - - Set unsupportedRepresentations = - request.viewVersion().representations().stream() - .filter(r -> !(r instanceof SQLViewRepresentation)) - .map(ViewRepresentation::type) - .collect(Collectors.toSet()); - - if (!unsupportedRepresentations.isEmpty()) { - throw new IllegalStateException( - String.format("Found unsupported view representations: %s", unsupportedRepresentations)); - } - - request.viewVersion().representations().stream() - .filter(SQLViewRepresentation.class::isInstance) - .map(SQLViewRepresentation.class::cast) - .forEach(r -> viewBuilder.withQuery(r.dialect(), r.sql())); - - View view = viewBuilder.create(); - - return viewResponse(view); - } - - private static LoadViewResponse viewResponse(View view) { - ViewMetadata metadata = asBaseView(view).operations().current(); - return ImmutableLoadViewResponse.builder() - .metadata(metadata) - .metadataLocation(metadata.metadataFileLocation()) - .build(); - } - - public static void viewExists(ViewCatalog catalog, TableIdentifier viewIdentifier) { - if (!catalog.viewExists(viewIdentifier)) { - throw new NoSuchViewException("View does not exist: %s", viewIdentifier); - } - } - - public static LoadViewResponse loadView(ViewCatalog catalog, TableIdentifier viewIdentifier) { - View view = catalog.loadView(viewIdentifier); - return viewResponse(view); - } - - public static LoadViewResponse updateView( - ViewCatalog catalog, TableIdentifier ident, UpdateTableRequest request) { - View view = catalog.loadView(ident); - ViewMetadata metadata = commit(asBaseView(view).operations(), request); - - return ImmutableLoadViewResponse.builder() - .metadata(metadata) - .metadataLocation(metadata.metadataFileLocation()) - .build(); - } - - public static void renameView(ViewCatalog catalog, RenameTableRequest request) { - catalog.renameView(request.source(), request.destination()); - } - - public static void dropView(ViewCatalog catalog, TableIdentifier viewIdentifier) { - boolean dropped = catalog.dropView(viewIdentifier); - if (!dropped) { - throw new NoSuchViewException("View does not exist: %s", viewIdentifier); - } - } - - static ViewMetadata commit(ViewOperations ops, UpdateTableRequest request) { - AtomicBoolean isRetry = new AtomicBoolean(false); - try { - Tasks.foreach(ops) - .retry(COMMIT_NUM_RETRIES_DEFAULT) - .exponentialBackoff( - COMMIT_MIN_RETRY_WAIT_MS_DEFAULT, - COMMIT_MAX_RETRY_WAIT_MS_DEFAULT, - COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT, - 2.0 /* exponential */) - .onlyRetryOn(CommitFailedException.class) - .run( - taskOps -> { - ViewMetadata base = isRetry.get() ? taskOps.refresh() : taskOps.current(); - isRetry.set(true); - - // validate requirements - try { - request.requirements().forEach(requirement -> requirement.validate(base)); - } catch (CommitFailedException e) { - // wrap and rethrow outside of tasks to avoid unnecessary retry - throw new ValidationFailureException(e); - } - - // apply changes - ViewMetadata.Builder metadataBuilder = ViewMetadata.buildFrom(base); - request.updates().forEach(update -> update.applyTo(metadataBuilder)); - - ViewMetadata updated = metadataBuilder.build(); - - if (updated.changes().isEmpty()) { - // do not commit if the metadata has not changed - return; - } - - // commit - taskOps.commit(base, updated); - }); - - } catch (ValidationFailureException e) { - throw e.wrapped(); - } - - return ops.current(); - } -} From c04b2d4817afcd7d4315f110ecb9e14cbc9c6494 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 13:20:34 -0700 Subject: [PATCH 22/27] small changes per review --- helm/polaris/README.md | 2 +- .../service/quarkus/config/DefaultConfigurationStoreTest.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/helm/polaris/README.md b/helm/polaris/README.md index 886cd4ed3f..e6707091ff 100644 --- a/helm/polaris/README.md +++ b/helm/polaris/README.md @@ -285,7 +285,7 @@ kubectl delete namespace polaris | persistence.eclipseLink.secret | object | `{"key":"persistence.xml","name":null}` | The secret name to pull persistence.xml from. | | persistence.eclipseLink.secret.key | string | `"persistence.xml"` | The key in the secret to pull persistence.xml from. | | persistence.eclipseLink.secret.name | string | `nil` | The name of the secret to pull persistence.xml from. If not provided, the default built-in persistence.xml will be used. This is probably not what you want. | -| persistence.type | string | `"eclipse-link"` | The type of persistence to use. Two built-in types are supported: in-memory and eclipse-link. | +| persistence.type | string | `"relational-jdbc"` | Three built-in types are available: "relational-jdbc", "in-memory", "eclipse-link". The in-memory type is not recommended for production use. The eclipse-link type is deprecated and will be unsupported in a future release. | | podAnnotations | object | `{}` | Annotations to apply to polaris pods. | | podLabels | object | `{}` | Additional Labels to apply to polaris pods. | | podSecurityContext | object | `{"fsGroup":10001,"seccompProfile":{"type":"RuntimeDefault"}}` | Security context for the polaris pod. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/. | diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index 26eb26c26b..e01b0fb7b5 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -209,5 +209,7 @@ public void testInjectedConfigurationStore() { setCurrentRealm(realmOne); realmOverrideValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); assertThat(realmOverrideValue).isFalse(); + + assertThat(configurationStore).isInstanceOf(DefaultConfigurationStore.class); } } From b7462adcbd3b3f4aa28c8fc454d1e0294d57c75c Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 13:32:29 -0700 Subject: [PATCH 23/27] clean out defaults --- .../QuarkusResolvedFeaturesConfiguration.java | 59 +++++++++++++++++++ .../config/DefaultConfigurationStoreTest.java | 18 ++++++ 2 files changed, 77 insertions(+) create mode 100644 quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedFeaturesConfiguration.java diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedFeaturesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedFeaturesConfiguration.java new file mode 100644 index 0000000000..567c30972e --- /dev/null +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedFeaturesConfiguration.java @@ -0,0 +1,59 @@ +/* + * 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.service.quarkus.config; + +import jakarta.annotation.Priority; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.inject.Alternative; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.polaris.service.config.FeaturesConfiguration; + +/** + * Wraps around {@link QuarkusFeaturesConfiguration} but removes properties from `defaults` that + * shouldn't be there + */ +@ApplicationScoped +@Alternative +@Priority(1) +public class QuarkusResolvedFeaturesConfiguration implements FeaturesConfiguration { + + private final Map cleanedDefaults; + private final Map realmOverrides; + + public QuarkusResolvedFeaturesConfiguration(QuarkusFeaturesConfiguration raw) { + this.realmOverrides = raw.realmOverrides(); + + // Filter out any keys that look like realm overrides + this.cleanedDefaults = + raw.defaults().entrySet().stream() + .filter(e -> e.getKey().split("\\.").length == 1) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } + + @Override + public Map defaults() { + return cleanedDefaults; + } + + @Override + public Map realmOverrides() { + return realmOverrides; + } +} diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index e01b0fb7b5..e533d52142 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -35,6 +35,7 @@ import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.service.config.DefaultConfigurationStore; +import org.apache.polaris.service.config.FeaturesConfiguration; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -69,6 +70,7 @@ public Map getConfigOverrides() { @Inject MetaStoreManagerFactory managerFactory; @Inject PolarisConfigurationStore configurationStore; @Inject PolarisDiagnostics diagServices; + @Inject FeaturesConfiguration featuresConfiguration; @BeforeEach public void before(TestInfo testInfo) { @@ -212,4 +214,20 @@ public void testInjectedConfigurationStore() { assertThat(configurationStore).isInstanceOf(DefaultConfigurationStore.class); } + + @Test + public void testInjectedFeaturesConfiguration() { + assertThat(featuresConfiguration).isInstanceOf(QuarkusResolvedFeaturesConfiguration.class); + + assertThat(featuresConfiguration.defaults()) + .containsKeys(falseByDefaultKey, trueByDefaultKey) + .allSatisfy((key, value) -> assertThat(value).doesNotContain(realmOne)); + + assertThat(featuresConfiguration.realmOverrides()).hasSize(1); + assertThat(featuresConfiguration.realmOverrides()).containsKey(realmOne); + + assertThat(featuresConfiguration.realmOverrides().get(realmOne).overrides()).hasSize(1); + assertThat(featuresConfiguration.realmOverrides().get(realmOne).overrides()) + .containsKey(falseByDefaultKey); + } } From 1713f389be743abfea86e62f601ef58e08ddf9cc Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 13:34:40 -0700 Subject: [PATCH 24/27] BCC fix --- ...sResolvedBehaviorChangesConfiguration.java | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java new file mode 100644 index 0000000000..c189a2bf19 --- /dev/null +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java @@ -0,0 +1,60 @@ +/* + * 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.service.quarkus.config; + +import jakarta.annotation.Priority; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.inject.Alternative; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.polaris.service.config.FeaturesConfiguration; + +/** + * Wraps around {@link QuarkusBehaviorChangesConfiguration} but removes properties from `defaults` that + * shouldn't be there + */ +@ApplicationScoped +@Alternative +@Priority(1) +public class QuarkusResolvedBehaviorChangesConfiguration implements FeaturesConfiguration { + + private final Map cleanedDefaults; + private final Map realmOverrides; + + public QuarkusResolvedBehaviorChangesConfiguration(QuarkusBehaviorChangesConfiguration raw) { + this.realmOverrides = raw.realmOverrides(); + + // Filter out any keys that look like realm overrides + this.cleanedDefaults = + raw.defaults().entrySet().stream() + .filter(e -> e.getKey().split("\\.").length == 1) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } + + @Override + public Map defaults() { + return cleanedDefaults; + } + + @Override + public Map realmOverrides() { + return realmOverrides; + } +} From a0adc9ea25155e6b1338af2eebf32ecaaf337605 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 13:34:43 -0700 Subject: [PATCH 25/27] autolint --- ...sResolvedBehaviorChangesConfiguration.java | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java index c189a2bf19..1abf4dca3e 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ - package org.apache.polaris.service.quarkus.config; import jakarta.annotation.Priority; @@ -27,34 +26,34 @@ import org.apache.polaris.service.config.FeaturesConfiguration; /** - * Wraps around {@link QuarkusBehaviorChangesConfiguration} but removes properties from `defaults` that - * shouldn't be there + * Wraps around {@link QuarkusBehaviorChangesConfiguration} but removes properties from `defaults` + * that shouldn't be there */ @ApplicationScoped @Alternative @Priority(1) public class QuarkusResolvedBehaviorChangesConfiguration implements FeaturesConfiguration { - private final Map cleanedDefaults; - private final Map realmOverrides; + private final Map cleanedDefaults; + private final Map realmOverrides; - public QuarkusResolvedBehaviorChangesConfiguration(QuarkusBehaviorChangesConfiguration raw) { - this.realmOverrides = raw.realmOverrides(); + public QuarkusResolvedBehaviorChangesConfiguration(QuarkusBehaviorChangesConfiguration raw) { + this.realmOverrides = raw.realmOverrides(); - // Filter out any keys that look like realm overrides - this.cleanedDefaults = - raw.defaults().entrySet().stream() - .filter(e -> e.getKey().split("\\.").length == 1) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - } + // Filter out any keys that look like realm overrides + this.cleanedDefaults = + raw.defaults().entrySet().stream() + .filter(e -> e.getKey().split("\\.").length == 1) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } - @Override - public Map defaults() { - return cleanedDefaults; - } + @Override + public Map defaults() { + return cleanedDefaults; + } - @Override - public Map realmOverrides() { - return realmOverrides; - } + @Override + public Map realmOverrides() { + return realmOverrides; + } } From 04fea8fbcd31fe1ff65b13a1000ac763dc016ed6 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 13:40:02 -0700 Subject: [PATCH 26/27] typefix --- .../QuarkusResolvedBehaviorChangesConfiguration.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java index 1abf4dca3e..19c6dab8f5 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java @@ -23,6 +23,8 @@ import jakarta.enterprise.inject.Alternative; import java.util.Map; import java.util.stream.Collectors; + +import org.apache.polaris.core.config.BehaviorChangeConfiguration; import org.apache.polaris.service.config.FeaturesConfiguration; /** @@ -32,10 +34,10 @@ @ApplicationScoped @Alternative @Priority(1) -public class QuarkusResolvedBehaviorChangesConfiguration implements FeaturesConfiguration { +public class QuarkusResolvedBehaviorChangesConfiguration implements QuarkusBehaviorChangesConfiguration { private final Map cleanedDefaults; - private final Map realmOverrides; + private final Map realmOverrides; public QuarkusResolvedBehaviorChangesConfiguration(QuarkusBehaviorChangesConfiguration raw) { this.realmOverrides = raw.realmOverrides(); @@ -53,7 +55,7 @@ public Map defaults() { } @Override - public Map realmOverrides() { + public Map realmOverrides() { return realmOverrides; } } From 5b3a11ba88ec66357ee1a739426c1430f694dd15 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 13:40:05 -0700 Subject: [PATCH 27/27] autolint --- .../QuarkusResolvedBehaviorChangesConfiguration.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java index 19c6dab8f5..b4a4078a38 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusResolvedBehaviorChangesConfiguration.java @@ -24,9 +24,6 @@ import java.util.Map; import java.util.stream.Collectors; -import org.apache.polaris.core.config.BehaviorChangeConfiguration; -import org.apache.polaris.service.config.FeaturesConfiguration; - /** * Wraps around {@link QuarkusBehaviorChangesConfiguration} but removes properties from `defaults` * that shouldn't be there @@ -34,10 +31,12 @@ @ApplicationScoped @Alternative @Priority(1) -public class QuarkusResolvedBehaviorChangesConfiguration implements QuarkusBehaviorChangesConfiguration { +public class QuarkusResolvedBehaviorChangesConfiguration + implements QuarkusBehaviorChangesConfiguration { private final Map cleanedDefaults; - private final Map realmOverrides; + private final Map + realmOverrides; public QuarkusResolvedBehaviorChangesConfiguration(QuarkusBehaviorChangesConfiguration raw) { this.realmOverrides = raw.realmOverrides(); @@ -55,7 +54,8 @@ public Map defaults() { } @Override - public Map realmOverrides() { + public Map + realmOverrides() { return realmOverrides; } }