Cherry picks for "Adjustable limit on the number of locations per storage config" (apache#26)

* b93e97b

* Adjustable limit on the number of locations per storage config (apache#1068)

* initial commit

* autolint

* change the config store access

* autolint

* add support for 01

* autolint

* fix test

* autolint

* retest

* rebase

* autolint

* change the config store access

* autolint

* add support for 01

* autolint

* fix test

* autolint

* retest

* fix a test

* autolint

* fix another test

* autolint

* remove catalog config for now as it's not used

* changes

* autolint

* update test to reflect -1 default

* autolint

* autolint

* move the check

* changes per review

* ready

* autolint

* spotless

---------

Co-authored-by: Eric Maynard <[email protected]>

Author: sfc-gh-emaynard

extension/persistence/eclipselink/src/test/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreManagerTest.java

@@ -36,9 +36,9 @@
 import java.util.Objects;
 import java.util.stream.Stream;
 import org.apache.polaris.core.PolarisCallContext;
-import org.apache.polaris.core.PolarisConfigurationStore;
 import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
 import org.apache.polaris.core.PolarisDiagnostics;
+import org.apache.polaris.core.config.PolarisConfigurationStore;
 import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
 import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest;
 import org.apache.polaris.core.persistence.PolarisMetaStoreManagerImpl;

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisApplicationIntegrationTest.java

@@ -94,10 +94,11 @@
 /**
  * @implSpec This test expects the server to be configured with the following features configured:
  *     <ul>
- *       <li>{@link org.apache.polaris.core.PolarisConfiguration#ALLOW_OVERLAPPING_CATALOG_URLS}:
+ *       <li>{@link
+ *           org.apache.polaris.core.config.FeatureConfiguration#ALLOW_OVERLAPPING_CATALOG_URLS}:
  *           {@code true}
  *       <li>{@link
- *           org.apache.polaris.core.PolarisConfiguration#SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION}:
+ *           org.apache.polaris.core.config.FeatureConfiguration#SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION}:
  *           {@code true}
  *     </ul>
  *     The server must also be configured to reject request body sizes larger than 1MB (1000000

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java

@@ -90,10 +90,11 @@
  * @implSpec @implSpec This test expects the server to be configured with the following features
  *     configured:
  *     <ul>
- *       <li>{@link org.apache.polaris.core.PolarisConfiguration#ALLOW_OVERLAPPING_CATALOG_URLS}:
+ *       <li>{@link
+ *           org.apache.polaris.core.config.FeatureConfiguration#ALLOW_OVERLAPPING_CATALOG_URLS}:
  *           {@code true}
  *       <li>{@link
- *           org.apache.polaris.core.PolarisConfiguration#ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING}:
+ *           org.apache.polaris.core.config.FeatureConfiguration#ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING}:
  *           {@code true}
  *     </ul>
  */

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java

@@ -58,7 +58,6 @@
 import org.apache.iceberg.rest.RESTCatalog;
 import org.apache.iceberg.rest.responses.ErrorResponse;
 import org.apache.iceberg.types.Types;
-import org.apache.polaris.core.PolarisConfiguration;
 import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
 import org.apache.polaris.core.admin.model.Catalog;
 import org.apache.polaris.core.admin.model.CatalogGrant;
@@ -76,6 +75,8 @@
 import org.apache.polaris.core.admin.model.TablePrivilege;
 import org.apache.polaris.core.admin.model.ViewGrant;
 import org.apache.polaris.core.admin.model.ViewPrivilege;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.PolarisConfiguration;
 import org.apache.polaris.core.entity.CatalogEntity;
 import org.apache.polaris.core.entity.PolarisEntityConstants;
 import org.apache.polaris.service.it.env.CatalogApi;
@@ -449,7 +450,7 @@ public void testCreateTableWithOverriddenBaseLocation() {
     Catalog catalog = managementApi.getCatalog(currentCatalogName);
     Map<String, String> catalogProps = new HashMap<>(catalog.getProperties().toMap());
     catalogProps.put(
-        PolarisConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false");
+        FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false");
     managementApi.updateCatalog(catalog, catalogProps);
 
     restCatalog.createNamespace(Namespace.of("ns1"));
@@ -477,7 +478,7 @@ public void testCreateTableWithOverriddenBaseLocationCannotOverlapSibling() {
     Catalog catalog = managementApi.getCatalog(currentCatalogName);
     Map<String, String> catalogProps = new HashMap<>(catalog.getProperties().toMap());
     catalogProps.put(
-        PolarisConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false");
+        FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false");
     managementApi.updateCatalog(catalog, catalogProps);
 
     restCatalog.createNamespace(Namespace.of("ns1"));
@@ -514,7 +515,7 @@ public void testCreateTableWithOverriddenBaseLocationMustResideInNsDirectory() {
     Catalog catalog = managementApi.getCatalog(currentCatalogName);
     Map<String, String> catalogProps = new HashMap<>(catalog.getProperties().toMap());
     catalogProps.put(
-        PolarisConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false");
+        FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false");
     managementApi.updateCatalog(catalog, catalogProps);
 
     restCatalog.createNamespace(Namespace.of("ns1"));
@@ -563,7 +564,7 @@ public void testLoadTableWithAccessDelegationForExternalCatalogWithConfigDisable
             .isInstanceOf(ForbiddenException.class)
             .hasMessageContaining("Access Delegation is not enabled for this catalog")
             .hasMessageContaining(
-                PolarisConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING.catalogConfig());
+                FeatureConfiguration.ALLOW_EXTERNAL_CATALOG_CREDENTIAL_VENDING.catalogConfig());
       } finally {
         resolvingFileIO.deleteFile(fileLocation);
       }

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java

@@ -24,12 +24,12 @@
 import java.util.Map;
 import org.apache.iceberg.rest.RESTCatalog;
 import org.apache.iceberg.view.ViewCatalogTests;
-import org.apache.polaris.core.PolarisConfiguration;
 import org.apache.polaris.core.admin.model.Catalog;
 import org.apache.polaris.core.admin.model.CatalogProperties;
 import org.apache.polaris.core.admin.model.PolarisCatalog;
 import org.apache.polaris.core.admin.model.PrincipalWithCredentials;
 import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.core.config.FeatureConfiguration;
 import org.apache.polaris.core.entity.CatalogEntity;
 import org.apache.polaris.service.it.env.ClientCredentials;
 import org.apache.polaris.service.it.env.IcebergHelper;
@@ -50,8 +50,8 @@
  * client.
  *
  * @implSpec This test expects the server to be configured with {@link
- *     org.apache.polaris.core.PolarisConfiguration#SUPPORTED_CATALOG_STORAGE_TYPES} set to the
- *     appropriate storage type.
+ *     org.apache.polaris.core.config.FeatureConfiguration#SUPPORTED_CATALOG_STORAGE_TYPES} set to
+ *     the appropriate storage type.
  */
 @ExtendWith(PolarisIntegrationTestExtension.class)
 public abstract class PolarisRestCatalogViewIntegrationBase extends ViewCatalogTests<RESTCatalog> {
@@ -99,9 +99,9 @@ public void before(TestInfo testInfo) {
         CatalogProperties.builder(defaultBaseLocation)
             .addProperty(
                 CatalogEntity.REPLACE_NEW_LOCATION_PREFIX_WITH_CATALOG_DEFAULT_KEY, "file:")
-            .addProperty(PolarisConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true")
+            .addProperty(FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true")
             .addProperty(
-                PolarisConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")
+                FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")
             .build();
     Catalog catalog =
         PolarisCatalog.builder()

integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisSparkIntegrationTest.java

@@ -64,9 +64,10 @@
  * @implSpec This test expects the server to be configured with the following features enabled:
  *     <ul>
  *       <li>{@link
- *           org.apache.polaris.core.PolarisConfiguration#SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION}:
+ *           org.apache.polaris.core.config.FeatureConfiguration#SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION}:
  *           {@code true}
- *       <li>{@link org.apache.polaris.core.PolarisConfiguration#ALLOW_OVERLAPPING_CATALOG_URLS}:
+ *       <li>{@link
+ *           org.apache.polaris.core.config.FeatureConfiguration#ALLOW_OVERLAPPING_CATALOG_URLS}:
  *           {@code true}
  *     </ul>
  */

polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java

@@ -21,6 +21,7 @@
 import jakarta.annotation.Nonnull;
 import java.time.Clock;
 import java.time.ZoneId;
+import org.apache.polaris.core.config.PolarisConfigurationStore;
 import org.apache.polaris.core.persistence.PolarisMetaStoreSession;
 
 /**

polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java

@@ -98,8 +98,8 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.apache.iceberg.exceptions.ForbiddenException;
-import org.apache.polaris.core.PolarisConfiguration;
-import org.apache.polaris.core.PolarisConfigurationStore;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.config.PolarisConfigurationStore;
 import org.apache.polaris.core.context.CallContext;
 import org.apache.polaris.core.entity.PolarisBaseEntity;
 import org.apache.polaris.core.entity.PolarisEntityConstants;
@@ -510,7 +510,7 @@ public void authorizeOrThrow(
     boolean enforceCredentialRotationRequiredState =
         featureConfig.getConfiguration(
             CallContext.getCurrentContext().getPolarisCallContext(),
-            PolarisConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING);
+            FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING);
     if (enforceCredentialRotationRequiredState
         && authenticatedPrincipal
             .getPrincipalEntity()

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

@@ -0,0 +1,55 @@
+/*
+ * 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 java.util.Optional;
+
+/**
+ * Internal configuration flags for non-feature behavior changes in Polaris. These flags control
+ * subtle behavior adjustments and bug fixes, not user-facing catalog settings. They are intended
+ * for internal use only, are inherently unstable, and may be removed at any time. When introducing
+ * a new flag, consider the trade-off between maintenance burden and the risk of an unguarded
+ * behavior change. Flags here are generally short-lived and should either be removed or promoted to
+ * stable feature flags before the next release.
+ *
+ * @param <T> The type of the configuration
+ */
+public class BehaviorChangeConfiguration<T> extends PolarisConfiguration<T> {
+
+  protected BehaviorChangeConfiguration(
+      String key, String description, T defaultValue, Optional<String> catalogConfig) {
+    super(key, description, defaultValue, catalogConfig);
+  }
+
+  public static final BehaviorChangeConfiguration<Boolean> VALIDATE_VIEW_LOCATION_OVERLAP =
+      PolarisConfiguration.<Boolean>builder()
+          .key("STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS")
+          .description("If true, validate that view locations don't overlap when views are created")
+          .defaultValue(true)
+          .buildBehaviorChangeConfiguration();
+
+  public static final BehaviorChangeConfiguration<Integer> STORAGE_CONFIGURATION_MAX_LOCATIONS =
+      PolarisConfiguration.<Integer>builder()
+          .key("STORAGE_CONFIGURATION_MAX_LOCATIONS")
+          .description(
+              "How many locations can be associated with a storage configuration, or -1 for"
+                  + " unlimited locations")
+          .defaultValue(-1)
+          .buildBehaviorChangeConfiguration();
+}