diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a4d0c891b..0e6fb7e78d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti ### Changes - `client.region` is no longer considered a "credential" property (related to Iceberg REST Catalog API). +- Relaxed the requirements for S3 storage's ARN to allow Polaris to connect to more non-AWS S3 storage appliances. ### Deprecations 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 b3d7d60790..69c669222d 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 @@ -44,9 +44,9 @@ public static ImmutableAwsStorageConfigurationInfo.Builder builder() { return ImmutableAwsStorageConfigurationInfo.builder(); } - // 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/.+$"; + // Technically, it should be ^arn:(aws|aws-cn|aws-us-gov):iam::(\d{12}):role/.+$, but we've + // generalized it to support non-aws S3 implementations + @JsonIgnore public static final String ROLE_ARN_PATTERN = "^.+:(.*):iam:.*:(.*):role/.+$"; private static final Pattern ROLE_ARN_PATTERN_COMPILED = Pattern.compile(ROLE_ARN_PATTERN); diff --git a/runtime/service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java b/runtime/service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java index 47e61f5734..5c3c220c82 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java @@ -255,7 +255,7 @@ public void testValidAllowedLocationPrefix() { } @ParameterizedTest - @ValueSource(strings = {"", "arn:aws:iam::0123456:role/jdoe", "aws-cn"}) + @ValueSource(strings = {"", "arn:aws:iam:0123456:role/jdoe", "aws-cn"}) public void testInvalidArn(String roleArn) { String basedLocation = "s3://externally-owned-bucket"; AwsStorageConfigInfo awsStorageConfigModel = @@ -278,13 +278,42 @@ public void testInvalidArn(String roleArn) { switch (roleArn) { case "" -> "ARN must not be empty"; case "aws-cn" -> "AWS China is temporarily not supported"; - default -> "Invalid role ARN format: arn:aws:iam::0123456:role/jdoe"; + default -> "Invalid role ARN format: arn:aws:iam:0123456:role/jdoe"; }; Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(realmConfig, awsCatalog)) .isInstanceOf(IllegalArgumentException.class) .hasMessage(expectedMessage); } + @ParameterizedTest + @ValueSource( + strings = { + "arn:aws:iam::012345678911:role/rollerblade", + "test:test:iam:region:accountid:role/rollerblade", + "a::iam:::role/rollerblade" + }) + public void testValidArn(String roleArn) { + String basedLocation = "s3://externally-owned-bucket"; + AwsStorageConfigInfo awsStorageConfigModel = + AwsStorageConfigInfo.builder() + .setRoleArn(roleArn) + .setExternalId("externalId") + .setStorageType(StorageConfigInfo.StorageTypeEnum.S3) + .setAllowedLocations(List.of(basedLocation)) + .build(); + + CatalogProperties prop = new CatalogProperties(basedLocation); + Catalog awsCatalog = + PolarisCatalog.builder() + .setType(Catalog.TypeEnum.INTERNAL) + .setName("name") + .setProperties(prop) + .setStorageConfigInfo(awsStorageConfigModel) + .build(); + Assertions.assertThatNoException() + .isThrownBy(() -> CatalogEntity.fromCatalog(realmConfig, awsCatalog)); + } + @Test public void testCatalogTypeDefaultsToInternal() { String baseLocation = "s3://test-bucket/path";