From fa7b282df0ff3037c4003f761dfdb20befffc5d9 Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 10:21:24 -0500 Subject: [PATCH 1/8] made the regex more generic --- .../polaris/core/storage/aws/AwsStorageConfigurationInfo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..45904b9bf5 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 @@ -46,7 +46,7 @@ public static ImmutableAwsStorageConfigurationInfo.Builder 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/.+$"; + public static final String ROLE_ARN_PATTERN = "^.+:(.*):iam:.*:(.*):role/.+$"; private static final Pattern ROLE_ARN_PATTERN_COMPILED = Pattern.compile(ROLE_ARN_PATTERN); From c340ab3e4634bc1b4c65a386bb4da8977f8997d4 Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 10:47:13 -0500 Subject: [PATCH 2/8] cleanup aisle 2 --- .../polaris/core/storage/aws/AwsStorageConfigurationInfo.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 45904b9bf5..eaae6f4c35 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 @@ -45,8 +45,7 @@ public static ImmutableAwsStorageConfigurationInfo.Builder builder() { } // Technically, it should be ^arn:(aws|aws-cn|aws-us-gov):iam::(\d{12}):role/.+$, - @JsonIgnore - public static final String ROLE_ARN_PATTERN = "^.+:(.*):iam:.*:(.*):role/.+$"; + @JsonIgnore public static final String ROLE_ARN_PATTERN = "^.+:(.*):iam:.*:(.*):role/.+$"; private static final Pattern ROLE_ARN_PATTERN_COMPILED = Pattern.compile(ROLE_ARN_PATTERN); From bd2c3f871cb115dc911a0060c6497553bf3d88d2 Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 10:59:01 -0500 Subject: [PATCH 3/8] updated ROLE_ARN_PATTERN comment --- .../polaris/core/storage/aws/AwsStorageConfigurationInfo.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 eaae6f4c35..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,7 +44,8 @@ public static ImmutableAwsStorageConfigurationInfo.Builder builder() { return ImmutableAwsStorageConfigurationInfo.builder(); } - // Technically, it should be ^arn:(aws|aws-cn|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); From 79fd42443036ce200de48e8813696679c94db62c Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 11:24:18 -0500 Subject: [PATCH 4/8] removed test-case that would not be considered valid --- .../org/apache/polaris/service/entity/CatalogEntityTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..c99b52dddc 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 = {"", "aws-cn"}) public void testInvalidArn(String roleArn) { String basedLocation = "s3://externally-owned-bucket"; AwsStorageConfigInfo awsStorageConfigModel = From ec4eb9a8e3df51cb9f815de9a39b31f35f1debf4 Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 13:06:35 -0500 Subject: [PATCH 5/8] Added this change to the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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 From 9dc3aee55ad977b12efd18fa78642c5052c26206 Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 13:07:00 -0500 Subject: [PATCH 6/8] added a test case --- .../org/apache/polaris/service/entity/CatalogEntityTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c99b52dddc..485deff644 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 = {"", "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,7 +278,7 @@ 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) From 101fcf3dc917cba9bc5d9247bb8f2600de86df30 Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 13:32:26 -0500 Subject: [PATCH 7/8] added more tests for s3 ARN --- .../service/entity/CatalogEntityTest.java | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) 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 485deff644..01ffd055d0 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,8 @@ public void testValidAllowedLocationPrefix() { } @ParameterizedTest - @ValueSource(strings = {"", "arn:aws:iam:0123456:role/jdoe", "aws-cn"}) + @ValueSource( + strings = {"", ":aws:iam:0123456:role/jdoe", "arn:aws:iam:0123456:role/jdoe", "aws-cn"}) public void testInvalidArn(String roleArn) { String basedLocation = "s3://externally-owned-bucket"; AwsStorageConfigInfo awsStorageConfigModel = @@ -285,6 +286,35 @@ public void testInvalidArn(String roleArn) { .hasMessage(expectedMessage); } + @ParameterizedTest + @ValueSource( + strings = { + "", + "arn:aws:iam::012345678911:role/rollerblade", + "test:test:iam:region:accountid: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"; From 2e0a1b931eba71a75cdb37265a3832978f333342 Mon Sep 17 00:00:00 2001 From: cccs-cat001 Date: Fri, 7 Nov 2025 14:59:01 -0500 Subject: [PATCH 8/8] tests work properly --- .../apache/polaris/service/entity/CatalogEntityTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 01ffd055d0..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,8 +255,7 @@ public void testValidAllowedLocationPrefix() { } @ParameterizedTest - @ValueSource( - strings = {"", ":aws:iam:0123456:role/jdoe", "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 = @@ -289,9 +288,9 @@ public void testInvalidArn(String roleArn) { @ParameterizedTest @ValueSource( strings = { - "", "arn:aws:iam::012345678911:role/rollerblade", - "test:test:iam:region:accountid:role/rollerblade" + "test:test:iam:region:accountid:role/rollerblade", + "a::iam:::role/rollerblade" }) public void testValidArn(String roleArn) { String basedLocation = "s3://externally-owned-bucket";