Relax ARN validation logic (#3071)
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.
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 b62265f..197da69 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
@@ -169,10 +169,6 @@
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 2732577..e742746 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 @@
});
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 =
@@ -598,24 +580,6 @@
});
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 =
@@ -659,6 +623,7 @@
});
switch (awsPartition) {
case AWS_PARTITION:
+ case "aws-cn":
StorageAccessConfig storageAccessConfig =
new AwsCredentialsStorageIntegration(
AwsStorageConfigurationInfo.builder()
@@ -677,7 +642,6 @@
.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 24c7814..fb7182c 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 @@
}
@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 @@
.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);
