This is an automated email from the ASF dual-hosted git repository.
emaynard pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 21efdf6d Adjustable limit on the number of locations per storage
config (#1068)
21efdf6d is described below
commit 21efdf6d4e3f93c8a6b5220089e220c922b2a0cb
Author: Eric Maynard <[email protected]>
AuthorDate: Thu Mar 13 11:31:51 2025 -0700
Adjustable limit on the number of locations per storage config (#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
---
.../core/config/BehaviorChangeConfiguration.java | 9 +++++++++
.../apache/polaris/core/entity/CatalogEntity.java | 16 ++++++++++++++++
.../storage/PolarisStorageConfigurationInfo.java | 8 --------
.../core/storage/StorageConfigurationOverride.java | 5 -----
.../storage/aws/AwsStorageConfigurationInfo.java | 5 -----
.../azure/AzureStorageConfigurationInfo.java | 5 -----
.../storage/gcp/GcpStorageConfigurationInfo.java | 7 -------
.../storage/PolarisConfigurationStoreTest.java | 8 ++++++--
.../service/quarkus/entity/CatalogEntityTest.java | 22 +++++++++++++++++++---
9 files changed, 50 insertions(+), 35 deletions(-)
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/config/BehaviorChangeConfiguration.java
b/polaris-core/src/main/java/org/apache/polaris/core/config/BehaviorChangeConfiguration.java
index a64c5940..6048872b 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/config/BehaviorChangeConfiguration.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/config/BehaviorChangeConfiguration.java
@@ -43,4 +43,13 @@ public class BehaviorChangeConfiguration<T> extends
PolarisConfiguration<T> {
.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();
}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
index f3bfd6ed..dbc31c0f 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
@@ -23,6 +23,7 @@ import static
org.apache.polaris.core.admin.model.StorageConfigInfo.StorageTypeE
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -39,6 +40,7 @@ import
org.apache.polaris.core.admin.model.FileStorageConfigInfo;
import org.apache.polaris.core.admin.model.GcpStorageConfigInfo;
import org.apache.polaris.core.admin.model.PolarisCatalog;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.core.config.BehaviorChangeConfiguration;
import org.apache.polaris.core.storage.FileStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
@@ -237,6 +239,7 @@ public class CatalogEntity extends PolarisEntity {
throw new BadRequestException("Must specify default base location");
}
allowedLocations.add(defaultBaseLocation);
+ validateMaxAllowedLocations(allowedLocations);
switch (storageConfigModel.getStorageType()) {
case S3:
AwsStorageConfigInfo awsConfigModel = (AwsStorageConfigInfo)
storageConfigModel;
@@ -279,6 +282,19 @@ public class CatalogEntity extends PolarisEntity {
return this;
}
+ /** Validate the number of allowed locations not exceeding the max value.
*/
+ private void validateMaxAllowedLocations(Collection<String>
allowedLocations) {
+ int maxAllowedLocations =
+ BehaviorChangeConfiguration.loadConfig(
+ BehaviorChangeConfiguration.STORAGE_CONFIGURATION_MAX_LOCATIONS);
+ if (maxAllowedLocations != -1 && allowedLocations.size() >
maxAllowedLocations) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Number of configured locations (%s) exceeds the limit of %s",
+ allowedLocations.size(), maxAllowedLocations));
+ }
+ }
+
@Override
public CatalogEntity build() {
return new CatalogEntity(buildBase());
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java
index 9631d95b..00f6ef6b 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java
@@ -230,14 +230,6 @@ public abstract class PolarisStorageConfigurationInfo {
}
}
- /** Validate the number of allowed locations not exceeding the max value. */
- public void validateMaxAllowedLocations(int maxAllowedLocations) {
- if (allowedLocations.size() > maxAllowedLocations) {
- throw new IllegalArgumentException(
- "Number of allowed locations exceeds " + maxAllowedLocations);
- }
- }
-
/** Polaris' storage type, each has a fixed prefix for its location */
public enum StorageType {
S3("s3://"),
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigurationOverride.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigurationOverride.java
index bf5a6cbf..1d695dbb 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigurationOverride.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigurationOverride.java
@@ -49,9 +49,4 @@ public class StorageConfigurationOverride extends
PolarisStorageConfigurationInf
protected void validatePrefixForStorageType(String loc) {
parentStorageConfiguration.validatePrefixForStorageType(loc);
}
-
- @Override
- public void validateMaxAllowedLocations(int maxAllowedLocations) {
-
parentStorageConfiguration.validateMaxAllowedLocations(maxAllowedLocations);
- }
}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java
index ebd24540..bc0fe437 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java
@@ -32,10 +32,6 @@ import
org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
/** Aws Polaris Storage Configuration information */
public class AwsStorageConfigurationInfo extends
PolarisStorageConfigurationInfo {
- // 5 is the approximate max allowed locations for the size of AccessPolicy
when LIST is required
- // for allowed read and write locations for subscoping creds.
- @JsonIgnore private static final int MAX_ALLOWED_LOCATIONS = 5;
-
// Technically, it should be
^arn:(aws|aws-cn|aws-us-gov):iam::(\d{12}):role/.+$,
@JsonIgnore
public static final String ROLE_ARN_PATTERN =
"^arn:(aws|aws-us-gov):iam::(\\d{12}):role/.+$";
@@ -75,7 +71,6 @@ public class AwsStorageConfigurationInfo extends
PolarisStorageConfigurationInfo
this.roleARN = roleARN;
this.externalId = externalId;
this.region = region;
- validateMaxAllowedLocations(MAX_ALLOWED_LOCATIONS);
}
@Override
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureStorageConfigurationInfo.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureStorageConfigurationInfo.java
index 99e7f3e3..53695b48 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureStorageConfigurationInfo.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureStorageConfigurationInfo.java
@@ -19,7 +19,6 @@
package org.apache.polaris.core.storage.azure;
import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.MoreObjects;
import jakarta.annotation.Nonnull;
@@ -30,9 +29,6 @@ import
org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
/** Azure storage configuration information. */
public class AzureStorageConfigurationInfo extends
PolarisStorageConfigurationInfo {
- // technically there is no limitation since expectation for Azure locations
are for the same
- // storage account and same container
- @JsonIgnore private static final int MAX_ALLOWED_LOCATIONS = 20;
// Azure tenant id
private final @Nonnull String tenantId;
@@ -52,7 +48,6 @@ public class AzureStorageConfigurationInfo extends
PolarisStorageConfigurationIn
@JsonProperty(value = "tenantId", required = true) @Nonnull String
tenantId) {
super(StorageType.AZURE, allowedLocations);
this.tenantId = tenantId;
- validateMaxAllowedLocations(MAX_ALLOWED_LOCATIONS);
}
@Override
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpStorageConfigurationInfo.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpStorageConfigurationInfo.java
index 7b0a9f6a..e664d4bd 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpStorageConfigurationInfo.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpStorageConfigurationInfo.java
@@ -19,7 +19,6 @@
package org.apache.polaris.core.storage.gcp;
import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.MoreObjects;
import jakarta.annotation.Nonnull;
@@ -30,11 +29,6 @@ import
org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
/** Gcp storage storage configuration information. */
public class GcpStorageConfigurationInfo extends
PolarisStorageConfigurationInfo {
- // 8 is an experimental result from generating GCP accessBoundaryRules when
subscoping creds,
- // when the rule is too large, GCS only returns error: 400 bad request
"Invalid arguments
- // provided in the request"
- @JsonIgnore private static final int MAX_ALLOWED_LOCATIONS = 8;
-
/** The gcp service account */
@JsonProperty(value = "gcpServiceAccount", required = false)
private @Nullable String gcpServiceAccount = null;
@@ -44,7 +38,6 @@ public class GcpStorageConfigurationInfo extends
PolarisStorageConfigurationInfo
@JsonProperty(value = "allowedLocations", required = true) @Nonnull
List<String> allowedLocations) {
super(StorageType.GCS, allowedLocations);
- validateMaxAllowedLocations(MAX_ALLOWED_LOCATIONS);
}
@Override
diff --git
a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
index 6abb76b8..5a1cfed3 100644
---
a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
+++
b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
@@ -28,6 +28,7 @@ import org.apache.polaris.core.config.PolarisConfiguration;
import org.apache.polaris.core.config.PolarisConfigurationStore;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
/** Unit test for the default behaviors of the PolarisConfigurationStore
interface. */
public class PolarisConfigurationStoreTest {
@@ -64,8 +65,9 @@ public class PolarisConfigurationStoreTest {
// Ensure that we can fetch all the configs and that the value is what we
expect, which
// is the config's default value based on how we've implemented
PolarisConfigurationStore above.
+ PolarisCallContext polarisCallContext =
Mockito.mock(PolarisCallContext.class);
for (PolarisConfiguration<?> c : configs) {
- Assertions.assertEquals(c.defaultValue, store.getConfiguration(null, c));
+ Assertions.assertEquals(c.defaultValue,
store.getConfiguration(polarisCallContext, c));
}
}
@@ -84,8 +86,10 @@ public class PolarisConfigurationStoreTest {
}
};
+ PolarisCallContext polarisCallContext =
Mockito.mock(PolarisCallContext.class);
for (PolarisConfiguration<?> c : configs) {
- Assertions.assertThrows(NumberFormatException.class, () ->
store.getConfiguration(null, c));
+ Assertions.assertThrows(
+ NumberFormatException.class, () ->
store.getConfiguration(polarisCallContext, c));
}
}
diff --git
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java
index 4a485925..8d8caabf 100644
---
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java
+++
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/entity/CatalogEntityTest.java
@@ -19,6 +19,8 @@
package org.apache.polaris.service.quarkus.entity;
import java.util.List;
+import org.apache.polaris.core.PolarisCallContext;
+import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
import org.apache.polaris.core.admin.model.AzureStorageConfigInfo;
import org.apache.polaris.core.admin.model.Catalog;
@@ -26,14 +28,29 @@ import
org.apache.polaris.core.admin.model.CatalogProperties;
import org.apache.polaris.core.admin.model.GcpStorageConfigInfo;
import org.apache.polaris.core.admin.model.PolarisCatalog;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.entity.CatalogEntity;
+import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
+import
org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory;
import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
public class CatalogEntityTest {
+ @BeforeAll
+ public static void setup() {
+ MetaStoreManagerFactory metaStoreManagerFactory = new
InMemoryPolarisMetaStoreManagerFactory();
+ PolarisCallContext polarisCallContext =
+ new PolarisCallContext(
+ metaStoreManagerFactory.getOrCreateSessionSupplier(() ->
"realm").get(),
+ new PolarisDefaultDiagServiceImpl());
+ CallContext callContext = CallContext.of(() -> "realm",
polarisCallContext);
+ CallContext.setCurrentContext(callContext);
+ }
+
@Test
public void testInvalidAllowedLocationPrefix() {
String storageLocation = "unsupportPrefix://mybucket/path";
@@ -123,9 +140,8 @@ public class CatalogEntityTest {
.setProperties(prop)
.setStorageConfigInfo(awsStorageConfigModel)
.build();
- Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(awsCatalog))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Number of allowed locations exceeds 5");
+ Assertions.assertThatCode(() -> CatalogEntity.fromCatalog(awsCatalog))
+ .doesNotThrowAnyException();
}
@Test
