Rename SUPPORTED_CATALOG_CONNECTION_TYPES

CHANGELOG.md

@@ -38,6 +38,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
 ### Changes
 
 ### Deprecations
+- Deprecates the `SUPPORTED_CATALOG_CONNECTION_TYPES` configuration property and replaces it with `SUPPORTED_EXTERNAL_CATALOG_CONNECTION_TYPES`. 
 
 ### Fixes
 

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

@@ -19,6 +19,7 @@
 package org.apache.polaris.core.config;
 
 import java.util.Optional;
+import java.util.Set;
 
 /**
  * Internal configuration flags for non-feature behavior changes in Polaris. These flags control
@@ -34,11 +35,12 @@ public class BehaviorChangeConfiguration<T> extends PolarisConfiguration<T> {
 
   protected BehaviorChangeConfiguration(
       String key,
+      Set<String> legacyKeys,
       String description,
       T defaultValue,
       Optional<String> catalogConfig,
       Optional<String> catalogConfigUnsafe) {
-    super(key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
+    super(key, legacyKeys, description, defaultValue, catalogConfig, catalogConfigUnsafe);
   }
 
   public static final BehaviorChangeConfiguration<Boolean> VALIDATE_VIEW_LOCATION_OVERLAP =

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

@@ -20,6 +20,7 @@
 
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import org.apache.polaris.core.admin.model.AuthenticationParameters;
 import org.apache.polaris.core.admin.model.StorageConfigInfo;
 import org.apache.polaris.core.connection.ConnectionType;
@@ -36,11 +37,12 @@
 public class FeatureConfiguration<T> extends PolarisConfiguration<T> {
   protected FeatureConfiguration(
       String key,
+      Set<String> legacyKeys,
       String description,
       T defaultValue,
       Optional<String> catalogConfig,
       Optional<String> catalogConfigUnsafe) {
-    super(key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
+    super(key, legacyKeys, description, defaultValue, catalogConfig, catalogConfigUnsafe);
   }
 
   /**
@@ -263,12 +265,14 @@ public static void enforceFeatureEnabledOrThrow(
           .defaultValue(true)
           .buildFeatureConfiguration();
 
-  public static final FeatureConfiguration<List<String>> SUPPORTED_CATALOG_CONNECTION_TYPES =
-      PolarisConfiguration.<List<String>>builder()
-          .key("SUPPORTED_CATALOG_CONNECTION_TYPES")
-          .description("The list of supported catalog connection types for federation")
-          .defaultValue(List.of(ConnectionType.ICEBERG_REST.name()))
-          .buildFeatureConfiguration();
+  public static final FeatureConfiguration<List<String>>
+      SUPPORTED_EXTERNAL_CATALOG_CONNECTION_TYPES =
+          PolarisConfiguration.<List<String>>builder()
+              .key("SUPPORTED_EXTERNAL_CATALOG_CONNECTION_TYPES")
+              .legacyKey("SUPPORTED_CATALOG_CONNECTION_TYPES")
+              .description("The list of supported external catalog connection types for federation")
+              .defaultValue(List.of(ConnectionType.ICEBERG_REST.name()))
+              .buildFeatureConfiguration();
 
   public static final FeatureConfiguration<List<String>>
       SUPPORTED_EXTERNAL_CATALOG_AUTHENTICATION_TYPES =

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

@@ -19,8 +19,10 @@
 package org.apache.polaris.core.config;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -41,6 +43,7 @@ public abstract class PolarisConfiguration<T> {
 
   public final String key;
   public final String description;
+  public final Set<String> legacyKeys;
   public final T defaultValue;
   private final Optional<String> catalogConfigImpl;
   private final Optional<String> catalogConfigUnsafeImpl;
@@ -55,9 +58,22 @@ public abstract class PolarisConfiguration<T> {
    */
   private static void registerConfiguration(PolarisConfiguration<?> configuration) {
     for (PolarisConfiguration<?> existingConfiguration : allConfigurations) {
-      if (existingConfiguration.key.equals(configuration.key)) {
+      if (existingConfiguration.key.equals(configuration.key)
+          || configuration.legacyKeys.contains(existingConfiguration.key)) {
+        throw new IllegalArgumentException(
+            String.format("Config '%s' is already in use", existingConfiguration.key));
+      } else if (existingConfiguration.legacyKeys.contains(configuration.key)) {
         throw new IllegalArgumentException(
             String.format("Config '%s' is already in use", configuration.key));
+      } else if (!configuration.legacyKeys.isEmpty()) {
+        Set<String> legacyKeys = new HashSet<>(existingConfiguration.legacyKeys);
+        legacyKeys.retainAll(configuration.legacyKeys);
+        if (!legacyKeys.isEmpty()) {
+          throw new IllegalArgumentException(
+              String.format(
+                  "Config '%s' is already in use",
+                  legacyKeys.stream().collect(Collectors.joining(","))));
+        }
       } else {
         var configs =
             Stream.of(
@@ -86,18 +102,28 @@ public static List<PolarisConfiguration<?>> getAllConfigurations() {
   @SuppressWarnings("unchecked")
   protected PolarisConfiguration(
       String key,
+      Set<String> legacyKeys,
       String description,
       T defaultValue,
       Optional<String> catalogConfig,
       Optional<String> catalogConfigUnsafe) {
     this.key = key;
+    this.legacyKeys = legacyKeys;
     this.description = description;
     this.defaultValue = defaultValue;
     this.catalogConfigImpl = catalogConfig;
     this.catalogConfigUnsafeImpl = catalogConfigUnsafe;
     this.typ = (Class<T>) defaultValue.getClass();
   }
 
+  public boolean hasLegacyKeys() {
+    return !legacyKeys.isEmpty();
+  }
+
+  public Set<String> legacyKeys() {
+    return legacyKeys;
+  }
+
   public boolean hasCatalogConfig() {
     return catalogConfigImpl.isPresent();
   }
@@ -126,6 +152,7 @@ T cast(Object value) {
 
   public static class Builder<T> {
     private String key;
+    private Set<String> legacyKeys = new HashSet<>();
     private String description;
     private T defaultValue;
     private Optional<String> catalogConfig = Optional.empty();
@@ -136,6 +163,16 @@ public Builder<T> key(String key) {
       return this;
     }
 
+    public Builder<T> legacyKey(String legacyKey) {
+      legacyKeys.add(legacyKey);
+      return this;
+    }
+
+    public Builder<T> legacyKeys(Set<String> legacyKeys) {
+      this.legacyKeys.addAll(legacyKeys);
+      return this;
+    }
+
     public Builder<T> description(String description) {
       this.description = description;
       return this;
@@ -190,7 +227,7 @@ public FeatureConfiguration<T> buildFeatureConfiguration() {
       validateOrThrow();
       FeatureConfiguration<T> config =
           new FeatureConfiguration<>(
-              key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
+              key, legacyKeys, description, defaultValue, catalogConfig, catalogConfigUnsafe);
       PolarisConfiguration.registerConfiguration(config);
       return config;
     }
@@ -203,7 +240,7 @@ public BehaviorChangeConfiguration<T> buildBehaviorChangeConfiguration() {
       }
       BehaviorChangeConfiguration<T> config =
           new BehaviorChangeConfiguration<>(
-              key, description, defaultValue, catalogConfig, catalogConfigUnsafe);
+              key, legacyKeys, description, defaultValue, catalogConfig, catalogConfigUnsafe);
       PolarisConfiguration.registerConfiguration(config);
       return config;
     }

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

@@ -99,7 +99,27 @@ public interface PolarisConfigurationStore {
    */
   default <T> @Nonnull T getConfiguration(
       @Nonnull RealmContext realmContext, PolarisConfiguration<T> config) {
-    T result = getConfiguration(realmContext, config.key, config.defaultValue);
+    T result = getConfiguration(realmContext, config.key);
+
+    if (result == null && config.hasLegacyKeys()) {
+      for (String legacyKey : config.legacyKeys()) {
+        T legacyResult = getConfiguration(realmContext, legacyKey);
+        if (legacyResult != null) {
+          LOGGER.warn(
+              "Legacy configuration key '{}' is in use. Please use '{}' instead.",
+              legacyKey,
+              config.key);
+          if (result == null) {
+            result = legacyResult;
+          }
+        }
+      }
+    }
+
+    if (result == null) {
+      result = config.defaultValue;
+    }
+
     return tryCast(config, result);
   }
 

polaris-core/src/test/java/org/apache/polaris/core/config/PolarisConfigurationTest.java

@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.core.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+
+public class PolarisConfigurationTest {
+
+  @Test
+  void testFeatureConfigurationWithLegacyKey() {
+    FeatureConfiguration<Boolean> config =
+        PolarisConfiguration.<Boolean>builder()
+            .key("FEATURE_CONFIG_WITH_LEGACY_KEY")
+            .legacyKey("OLD_FEATURE_CONFIG_WITH_LEGACY_KEY")
+            .description("Test configuration with deprecated key")
+            .defaultValue(true)
+            .buildFeatureConfiguration();
+
+    assertThat(config.hasLegacyKeys()).isTrue();
+    assertThat(config.legacyKeys().contains("OLD_FEATURE_CONFIG_WITH_LEGACY_KEY")).isTrue();
+    assertThat(config.key).isEqualTo("FEATURE_CONFIG_WITH_LEGACY_KEY");
+    assertThat(config.description).isEqualTo("Test configuration with deprecated key");
+    assertThat(config.defaultValue).isTrue();
+  }
+
+  @Test
+  void testBehaviorChangeConfigurationWithLegacyKey() {
+    BehaviorChangeConfiguration<String> config =
+        PolarisConfiguration.<String>builder()
+            .key("BEHAVIOR_CONFIG_WITH_LEGACY_KEY")
+            .legacyKey("OLD_BEHAVIOR_CONFIG_WITH_LEGACY_KEY")
+            .description("Test behavior configuration with deprecated key")
+            .defaultValue("test-value")
+            .buildBehaviorChangeConfiguration();
+
+    assertThat(config.hasLegacyKeys()).isTrue();
+    assertThat(config.legacyKeys().contains("OLD_BEHAVIOR_CONFIG_WITH_LEGACY_KEY")).isTrue();
+    assertThat(config.key).isEqualTo("BEHAVIOR_CONFIG_WITH_LEGACY_KEY");
+    assertThat(config.defaultValue).isEqualTo("test-value");
+  }
+
+  @Test
+  void testDuplicateKeyValidation() {
+    PolarisConfiguration.<Boolean>builder()
+        .key("DUPLICATE_KEY_TEST_1")
+        .description("First configuration")
+        .defaultValue(true)
+        .buildFeatureConfiguration();
+
+    assertThatThrownBy(
+            () ->
+                PolarisConfiguration.<Boolean>builder()
+                    .key("DUPLICATE_KEY_TEST_1")
+                    .description("Second configuration")
+                    .defaultValue(false)
+                    .buildFeatureConfiguration())
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Config 'DUPLICATE_KEY_TEST_1' is already in use");
+  }
+
+  @Test
+  void testDuplicateLegacyKeysValidation() {
+    PolarisConfiguration.<Boolean>builder()
+        .key("NEW_KEY_DEPRECATED_TEST_2")
+        .legacyKey("LEGACY_KEY_TEST_2_1")
+        .legacyKey("LEGACY_KEY_TEST_2_2")
+        .legacyKey("LEGACY_KEY_TEST_2_3")
+        .description("First configuration with deprecated key")
+        .defaultValue(true)
+        .buildFeatureConfiguration();
+
+    assertThatThrownBy(
+            () ->
+                PolarisConfiguration.<Boolean>builder()
+                    .key("ANOTHER_NEW_KEY_2")
+                    .legacyKey("LEGACY_KEY_TEST_2_1")
+                    .legacyKey("LEGACY_KEY_TEST_2_3")
+                    .description("Second configuration with same deprecated key")
+                    .defaultValue(false)
+                    .buildFeatureConfiguration())
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Config 'LEGACY_KEY_TEST_2_3,LEGACY_KEY_TEST_2_1' is already in use");
+  }
+
+  @Test
+  void testNewKeyMatchingExistingLegacyKeyValidation() {
+    PolarisConfiguration.<Boolean>builder()
+        .key("ORIGINAL_KEY_3")
+        .legacyKey("OLD_LEGACY_KEY_3")
+        .description("First configuration with deprecated key")
+        .defaultValue(true)
+        .buildFeatureConfiguration();
+
+    assertThatThrownBy(
+            () ->
+                PolarisConfiguration.<Boolean>builder()
+                    .key("OLD_LEGACY_KEY_3")
+                    .description("Configuration with key matching existing deprecated key")
+                    .defaultValue(false)
+                    .buildFeatureConfiguration())
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Config 'OLD_LEGACY_KEY_3' is already in use");
+  }
+
+  @Test
+  void testLegacyKeyMatchingExistingKeyValidation() {
+    PolarisConfiguration.<Boolean>builder()
+        .key("EXISTING_KEY_4")
+        .description("First configuration")
+        .defaultValue(true)
+        .buildFeatureConfiguration();
+
+    assertThatThrownBy(
+            () ->
+                PolarisConfiguration.<Boolean>builder()
+                    .key("NEW_KEY_FOR_VALIDATION_4")
+                    .legacyKey("EXISTING_KEY_4")
+                    .legacyKey("EXISTING_KEY_4_1")
+                    .description("Configuration with deprecated key matching existing key")
+                    .defaultValue(false)
+                    .buildFeatureConfiguration())
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Config 'EXISTING_KEY_4' is already in use");
+  }
+}