Skip to content

Commit d18aa7b

Browse files
authored
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.
1 parent 6a79605 commit d18aa7b

File tree

3 files changed

+3
-47
lines changed

3 files changed

+3
-47
lines changed

polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,6 @@ public static void validateArn(String arn) {
169169
if (arn.isEmpty()) {
170170
throw new IllegalArgumentException("ARN must not be empty");
171171
}
172-
// specifically throw errors for China
173-
if (arn.contains("aws-cn")) {
174-
throw new IllegalArgumentException("AWS China is temporarily not supported");
175-
}
176172
checkArgument(Pattern.matches(ROLE_ARN_PATTERN, arn), "Invalid role ARN format: %s", arn);
177173
}
178174
}

polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -234,24 +234,6 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) {
234234
});
235235
switch (awsPartition) {
236236
case "aws-cn":
237-
Assertions.assertThatThrownBy(
238-
() ->
239-
new AwsCredentialsStorageIntegration(
240-
AwsStorageConfigurationInfo.builder()
241-
.addAllowedLocation(s3Path(bucket, warehouseKeyPrefix))
242-
.roleARN(roleARN)
243-
.externalId(externalId)
244-
.region(region)
245-
.build(),
246-
stsClient)
247-
.getSubscopedCreds(
248-
EMPTY_REALM_CONFIG,
249-
true,
250-
Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)),
251-
Set.of(s3Path(bucket, firstPath)),
252-
null))
253-
.isInstanceOf(IllegalArgumentException.class);
254-
break;
255237
case AWS_PARTITION:
256238
case "aws-us-gov":
257239
StorageAccessConfig storageAccessConfig =
@@ -598,24 +580,6 @@ public void testClientRegion(String awsPartition) {
598580
});
599581
switch (awsPartition) {
600582
case "aws-cn":
601-
Assertions.assertThatThrownBy(
602-
() ->
603-
new AwsCredentialsStorageIntegration(
604-
AwsStorageConfigurationInfo.builder()
605-
.addAllowedLocation(s3Path(bucket, warehouseKeyPrefix))
606-
.roleARN(roleARN)
607-
.externalId(externalId)
608-
.region(clientRegion)
609-
.build(),
610-
stsClient)
611-
.getSubscopedCreds(
612-
EMPTY_REALM_CONFIG,
613-
true, /* allowList = true */
614-
Set.of(),
615-
Set.of(),
616-
Optional.empty()))
617-
.isInstanceOf(IllegalArgumentException.class);
618-
break;
619583
case AWS_PARTITION:
620584
case "aws-us-gov":
621585
StorageAccessConfig storageAccessConfig =
@@ -659,6 +623,7 @@ public void testNoClientRegion(String awsPartition) {
659623
});
660624
switch (awsPartition) {
661625
case AWS_PARTITION:
626+
case "aws-cn":
662627
StorageAccessConfig storageAccessConfig =
663628
new AwsCredentialsStorageIntegration(
664629
AwsStorageConfigurationInfo.builder()
@@ -677,7 +642,6 @@ public void testNoClientRegion(String awsPartition) {
677642
.isNotEmpty()
678643
.doesNotContainKey(StorageAccessProperty.CLIENT_REGION.getPropertyName());
679644
break;
680-
case "aws-cn":
681645
case "aws-us-gov":
682646
Assertions.assertThatThrownBy(
683647
() ->

runtime/service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ public void testValidAllowedLocationPrefix() {
255255
}
256256

257257
@ParameterizedTest
258-
@ValueSource(strings = {"", "arn:aws:iam:0123456:role/jdoe", "aws-cn"})
258+
@ValueSource(strings = {"", "arn:aws:iam:0123456:role/jdoe", "arn:aws-cn:iam:0123456:role/jdoe"})
259259
public void testInvalidArn(String roleArn) {
260260
String basedLocation = "s3://externally-owned-bucket";
261261
AwsStorageConfigInfo awsStorageConfigModel =
@@ -275,11 +275,7 @@ public void testInvalidArn(String roleArn) {
275275
.setStorageConfigInfo(awsStorageConfigModel)
276276
.build();
277277
String expectedMessage =
278-
switch (roleArn) {
279-
case "" -> "ARN must not be empty";
280-
case "aws-cn" -> "AWS China is temporarily not supported";
281-
default -> "Invalid role ARN format: arn:aws:iam:0123456:role/jdoe";
282-
};
278+
roleArn.isEmpty() ? "ARN must not be empty" : "Invalid role ARN format: " + roleArn;
283279
Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(realmConfig, awsCatalog))
284280
.isInstanceOf(IllegalArgumentException.class)
285281
.hasMessage(expectedMessage);

0 commit comments

Comments
 (0)