From faac5acde8a3976b71bfdd894b1c3d5b796227dc Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Mon, 17 Nov 2025 13:52:54 -0500 Subject: [PATCH] Relax ARN validation logic Following up on #3005, which allowed a wide range of ARN values in the validation RegEx, remove an additional explicit check for `aws-cn` being present in the ARN as a sub-string. Update existing unit tests to process `aws-cn` ARNs as common `aws` ARNs. Note: the old validation code does not look correct because it used to check for `aws-cn` anywhere in the ARN string, not just in its "partition" component. --- .../aws/AwsStorageConfigurationInfo.java | 4 -- .../AwsCredentialsStorageIntegrationTest.java | 38 +------------------ .../service/entity/CatalogEntityTest.java | 8 +--- 3 files changed, 3 insertions(+), 47 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 69c669222d..46c369e3b9 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 @@ -160,10 +160,6 @@ public static void validateArn(String arn) { if (arn.isEmpty()) { throw new IllegalArgumentException("ARN must not be empty"); } - // specifically throw errors for China - if (arn.contains("aws-cn")) { - throw new IllegalArgumentException("AWS China is temporarily not supported"); - } checkArgument(Pattern.matches(ROLE_ARN_PATTERN, arn), "Invalid role ARN format: %s", arn); } } diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java index fb0c63c403..5ad9c77da8 100644 --- a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java @@ -234,24 +234,6 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) { }); switch (awsPartition) { case "aws-cn": - Assertions.assertThatThrownBy( - () -> - new AwsCredentialsStorageIntegration( - AwsStorageConfigurationInfo.builder() - .addAllowedLocation(s3Path(bucket, warehouseKeyPrefix)) - .roleARN(roleARN) - .externalId(externalId) - .region(region) - .build(), - stsClient) - .getSubscopedCreds( - EMPTY_REALM_CONFIG, - true, - Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)), - Set.of(s3Path(bucket, firstPath)), - null)) - .isInstanceOf(IllegalArgumentException.class); - break; case AWS_PARTITION: case "aws-us-gov": StorageAccessConfig storageAccessConfig = @@ -558,24 +540,6 @@ public void testClientRegion(String awsPartition) { }); switch (awsPartition) { case "aws-cn": - Assertions.assertThatThrownBy( - () -> - new AwsCredentialsStorageIntegration( - AwsStorageConfigurationInfo.builder() - .addAllowedLocation(s3Path(bucket, warehouseKeyPrefix)) - .roleARN(roleARN) - .externalId(externalId) - .region(clientRegion) - .build(), - stsClient) - .getSubscopedCreds( - EMPTY_REALM_CONFIG, - true, /* allowList = true */ - Set.of(), - Set.of(), - Optional.empty())) - .isInstanceOf(IllegalArgumentException.class); - break; case AWS_PARTITION: case "aws-us-gov": StorageAccessConfig storageAccessConfig = @@ -619,6 +583,7 @@ public void testNoClientRegion(String awsPartition) { }); switch (awsPartition) { case AWS_PARTITION: + case "aws-cn": StorageAccessConfig storageAccessConfig = new AwsCredentialsStorageIntegration( AwsStorageConfigurationInfo.builder() @@ -637,7 +602,6 @@ public void testNoClientRegion(String awsPartition) { .isNotEmpty() .doesNotContainKey(StorageAccessProperty.CLIENT_REGION.getPropertyName()); break; - case "aws-cn": case "aws-us-gov": Assertions.assertThatThrownBy( () -> 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 5c3c220c82..a215df31f0 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", "arn:aws-cn:iam:0123456:role/jdoe"}) public void testInvalidArn(String roleArn) { String basedLocation = "s3://externally-owned-bucket"; AwsStorageConfigInfo awsStorageConfigModel = @@ -275,11 +275,7 @@ public void testInvalidArn(String roleArn) { .setStorageConfigInfo(awsStorageConfigModel) .build(); String expectedMessage = - 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"; - }; + roleArn.isEmpty() ? "ARN must not be empty" : "Invalid role ARN format: " + roleArn; Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(realmConfig, awsCatalog)) .isInstanceOf(IllegalArgumentException.class) .hasMessage(expectedMessage);