You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by di...@apache.org on 2025/08/13 13:21:26 UTC
(polaris) branch main updated: Make S3 `roleARN` optional (#2329)
This is an automated email from the ASF dual-hosted git repository.
dimas pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new d7d8990db Make S3 `roleARN` optional (#2329)
d7d8990db is described below
commit d7d8990db8fb8a91fec4289553b010c36775afc7
Author: Robert Stupp <sn...@snazy.de>
AuthorDate: Wed Aug 13 15:21:22 2025 +0200
Make S3 `roleARN` optional (#2329)
Fixes #2325
---
CHANGELOG.md | 2 +
.../core/admin/model/CatalogSerializationTest.java | 18 ++++--
.../PolarisManagementServiceIntegrationTest.java | 71 +++++++---------------
.../test/PolarisPolicyServiceIntegrationTest.java | 3 -
.../apache/polaris/core/entity/CatalogEntity.java | 1 -
.../aws/AwsCredentialsStorageIntegration.java | 21 +++----
.../storage/aws/AwsStorageConfigurationInfo.java | 70 ++++++++++++---------
.../storage/InMemoryStorageIntegrationTest.java | 15 -----
.../aws/AwsStorageConfigurationInfoTest.java | 21 ++++++-
.../PolarisStorageConfigurationInfoTest.java | 12 ++--
.../service/it/PolarisRestCatalogMinIOIT.java | 3 -
.../service/it/RestCatalogMinIOSpecialIT.java | 3 -
.../polaris/service/admin/PolarisAdminService.java | 2 +-
.../polaris/service/entity/CatalogEntityTest.java | 18 ++----
spec/polaris-management-service.yml | 2 -
15 files changed, 122 insertions(+), 140 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 108eb0004..fdbe0c1b0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -51,6 +51,8 @@ at locations that better optimize for object storage.
- Polaris Management API clients must be prepared to deal with new attributes in `AwsStorageConfigInfo` objects.
+- S3 configuration property role-ARN is no longer mandatory.
+
### Deprecations
* The property `polaris.active-roles-provider.type` is deprecated in favor of
diff --git a/api/management-model/src/test/java/org/apache/polaris/core/admin/model/CatalogSerializationTest.java b/api/management-model/src/test/java/org/apache/polaris/core/admin/model/CatalogSerializationTest.java
index 38076b1ec..c4210486b 100644
--- a/api/management-model/src/test/java/org/apache/polaris/core/admin/model/CatalogSerializationTest.java
+++ b/api/management-model/src/test/java/org/apache/polaris/core/admin/model/CatalogSerializationTest.java
@@ -57,7 +57,9 @@ public class CatalogSerializationTest {
Catalog.TypeEnum.INTERNAL,
TEST_CATALOG_NAME,
new CatalogProperties(TEST_LOCATION),
- new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3));
+ AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
+ .setRoleArn(TEST_ROLE_ARN)
+ .build());
String json = mapper.writeValueAsString(catalog);
@@ -83,7 +85,9 @@ public class CatalogSerializationTest {
Catalog.TypeEnum.INTERNAL,
TEST_CATALOG_NAME,
new CatalogProperties(TEST_LOCATION),
- new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3))),
+ AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
+ .setRoleArn(TEST_ROLE_ARN)
+ .build())),
Arguments.of("Null fields", new Catalog(Catalog.TypeEnum.INTERNAL, null, null, null)),
Arguments.of(
"Long name",
@@ -102,14 +106,16 @@ public class CatalogSerializationTest {
Catalog.TypeEnum.INTERNAL,
"",
new CatalogProperties(""),
- new AwsStorageConfigInfo("", StorageConfigInfo.StorageTypeEnum.S3))),
+ new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))),
Arguments.of(
"Special characters",
new Catalog(
Catalog.TypeEnum.INTERNAL,
"test\"catalog",
new CatalogProperties(TEST_LOCATION),
- new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3))),
+ AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
+ .setRoleArn(TEST_ROLE_ARN)
+ .build())),
Arguments.of(
"Whitespace",
new Catalog(
@@ -131,7 +137,9 @@ public class CatalogSerializationTest {
Catalog.TypeEnum.INTERNAL,
TEST_CATALOG_NAME,
new CatalogProperties(TEST_LOCATION),
- new AwsStorageConfigInfo(arn, StorageConfigInfo.StorageTypeEnum.S3))));
+ AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
+ .setRoleArn(arn)
+ .build())));
return Stream.concat(basicCases, arnCases);
}
diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
index 2bbd3830c..1d969c6bc 100644
--- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
+++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
@@ -605,8 +605,9 @@ public class PolarisManagementServiceIntegrationTest {
@Test
public void testCreateListUpdateAndDeleteCatalog() {
StorageConfigInfo storageConfig =
- new AwsStorageConfigInfo(
- "arn:aws:iam::123456789011:role/role1", StorageConfigInfo.StorageTypeEnum.S3);
+ AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
+ .setRoleArn("arn:aws:iam::123456789011:role/role1")
+ .build();
String catalogName = client.newEntityName("mycatalog");
Catalog catalog =
PolarisCatalog.builder()
@@ -649,8 +650,9 @@ public class PolarisManagementServiceIntegrationTest {
// Reject update of fields that can't be currently updated
StorageConfigInfo invalidModifiedStorageConfig =
- new AwsStorageConfigInfo(
- "arn:aws:iam::123456789012:role/newrole", StorageConfigInfo.StorageTypeEnum.S3);
+ AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
+ .setRoleArn("arn:aws:iam::123456789012:role/newrole")
+ .build();
UpdateCatalogRequest badUpdateRequest =
new UpdateCatalogRequest(
fetchedCatalog.getEntityVersion(),
@@ -674,8 +676,9 @@ public class PolarisManagementServiceIntegrationTest {
// AWS
// account IDs are same)
StorageConfigInfo validModifiedStorageConfig =
- new AwsStorageConfigInfo(
- "arn:aws:iam::123456789011:role/newrole", StorageConfigInfo.StorageTypeEnum.S3);
+ AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
+ .setRoleArn("arn:aws:iam::123456789011:role/newrole")
+ .build();
UpdateCatalogRequest updateRequest =
new UpdateCatalogRequest(
fetchedCatalog.getEntityVersion(),
@@ -772,9 +775,7 @@ public class PolarisManagementServiceIntegrationTest {
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
.setProperties(new CatalogProperties("s3://required/base/location"))
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.build();
managementApi.createCatalog(catalog);
@@ -1204,9 +1205,7 @@ public class PolarisManagementServiceIntegrationTest {
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
.setProperties(new CatalogProperties("s3://required/base/location"))
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.build();
managementApi.createCatalog(catalog);
@@ -1215,9 +1214,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName2)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://required/base/other_location"))
.build();
managementApi.createCatalog(catalog2);
@@ -1512,9 +1509,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(client.newEntityName("mycatalog"))
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog);
@@ -1534,9 +1529,7 @@ public class PolarisManagementServiceIntegrationTest {
.setType(Catalog.TypeEnum.INTERNAL)
.setName(client.newEntityName("othercatalog"))
.setProperties(new CatalogProperties("s3://path/to/data"))
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.build();
managementApi.createCatalog(otherCatalog);
@@ -1677,9 +1670,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog);
@@ -1766,9 +1757,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog);
@@ -1838,9 +1827,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog);
@@ -1851,9 +1838,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName2)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog2);
@@ -1905,9 +1890,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog);
@@ -1921,9 +1904,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName2)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog2);
@@ -2095,9 +2076,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog);
@@ -2121,9 +2100,7 @@ public class PolarisManagementServiceIntegrationTest {
PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.setProperties(new CatalogProperties("s3://bucket1/"))
.build();
managementApi.createCatalog(catalog);
@@ -2147,9 +2124,7 @@ public class PolarisManagementServiceIntegrationTest {
.setType(Catalog.TypeEnum.INTERNAL)
.setName(catalogName)
.setProperties(new CatalogProperties("s3://required/base/location"))
- .setStorageConfigInfo(
- new AwsStorageConfigInfo(
- "arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
+ .setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
.build();
managementApi.createCatalog(catalog);
diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java
index 787215c4b..415f41806 100644
--- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java
+++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java
@@ -184,9 +184,6 @@ public class PolarisPolicyServiceIntegrationTest {
currentCatalogName = client.newEntityName(method.getName());
AwsStorageConfigInfo awsConfigModel =
AwsStorageConfigInfo.builder()
- .setRoleArn(TEST_ROLE_ARN)
- .setExternalId("externalId")
- .setUserArn("a:user:arn")
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
.setAllowedLocations(List.of("s3://my-old-bucket/path/to/data"))
.build();
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
index a3343c7a0..01e79b466 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java
@@ -279,7 +279,6 @@ public class CatalogEntity extends PolarisEntity implements LocationBasedEntity
.pathStyleAccess(awsConfigModel.getPathStyleAccess())
.endpointInternal(awsConfigModel.getEndpointInternal())
.build();
- awsConfig.validateArn(awsConfigModel.getRoleArn());
config = awsConfig;
break;
case AZURE:
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java
index e1ae09fe1..616fb1f4d 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java
@@ -85,7 +85,7 @@ public class AwsCredentialsStorageIntegration
.roleSessionName("PolarisAwsCredentialsStorageIntegration")
.policy(
policyString(
- storageConfig.getRoleARN(),
+ storageConfig.getAwsPartition(),
allowListOperation,
allowedReadLocations,
allowedWriteLocations)
@@ -134,7 +134,7 @@ public class AwsCredentialsStorageIntegration
accessConfig.put(StorageAccessProperty.AWS_PATH_STYLE_ACCESS, Boolean.TRUE.toString());
}
- if (storageConfig.getAwsPartition().equals("aws-us-gov") && region == null) {
+ if ("aws-us-gov".equals(storageConfig.getAwsPartition()) && region == null) {
throw new IllegalArgumentException(
String.format(
"AWS region must be set when using partition %s", storageConfig.getAwsPartition()));
@@ -152,7 +152,10 @@ public class AwsCredentialsStorageIntegration
*/
// TODO - add KMS key access
private IamPolicy policyString(
- String roleArn, boolean allowList, Set<String> readLocations, Set<String> writeLocations) {
+ String awsPartition,
+ boolean allowList,
+ Set<String> readLocations,
+ Set<String> writeLocations) {
IamPolicy.Builder policyBuilder = IamPolicy.builder();
IamStatement.Builder allowGetObjectStatementBuilder =
IamStatement.builder()
@@ -162,7 +165,7 @@ public class AwsCredentialsStorageIntegration
Map<String, IamStatement.Builder> bucketListStatementBuilder = new HashMap<>();
Map<String, IamStatement.Builder> bucketGetLocationStatementBuilder = new HashMap<>();
- String arnPrefix = getArnPrefixFor(roleArn);
+ String arnPrefix = arnPrefixForPartition(awsPartition);
Stream.concat(readLocations.stream(), writeLocations.stream())
.distinct()
.forEach(
@@ -226,14 +229,8 @@ public class AwsCredentialsStorageIntegration
return policyBuilder.addStatement(allowGetObjectStatementBuilder.build()).build();
}
- private String getArnPrefixFor(String roleArn) {
- if (roleArn.contains("aws-cn")) {
- return "arn:aws-cn:s3:::";
- } else if (roleArn.contains("aws-us-gov")) {
- return "arn:aws-us-gov:s3:::";
- } else {
- return "arn:aws:s3:::";
- }
+ private static String arnPrefixForPartition(String awsPartition) {
+ return String.format("arn:%s:s3:::", awsPartition != null ? awsPartition : "aws");
}
private static @Nonnull String parseS3Path(URI uri) {
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 4181d205e..3a2d70663 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
@@ -18,6 +18,9 @@
*/
package org.apache.polaris.core.storage.aws;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonTypeName;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
@@ -28,6 +31,7 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.immutables.PolarisImmutable;
+import org.immutables.value.Value;
/** Aws Polaris Storage Configuration information */
@PolarisImmutable
@@ -56,19 +60,7 @@ public abstract class AwsStorageConfigurationInfo extends PolarisStorageConfigur
return "org.apache.iceberg.aws.s3.S3FileIO";
}
- public static void validateArn(String arn) {
- if (arn == null || arn.isEmpty()) {
- throw new IllegalArgumentException("ARN cannot be null or empty");
- }
- // specifically throw errors for China
- if (arn.contains("aws-cn")) {
- throw new IllegalArgumentException("AWS China is temporarily not supported");
- }
- if (!Pattern.matches(ROLE_ARN_PATTERN, arn)) {
- throw new IllegalArgumentException("Invalid role ARN format");
- }
- }
-
+ @Nullable
public abstract String getRoleARN();
/** AWS external ID, optional */
@@ -118,32 +110,54 @@ public abstract class AwsStorageConfigurationInfo extends PolarisStorageConfigur
}
@JsonIgnore
+ @Nullable
public String getAwsAccountId() {
- return parseAwsAccountId(getRoleARN());
+ String arn = getRoleARN();
+ if (arn != null) {
+ Matcher matcher = ROLE_ARN_PATTERN_COMPILED.matcher(arn);
+ checkState(matcher.matches());
+ return matcher.group(2);
+ }
+ return null;
}
@JsonIgnore
+ @Nullable
public String getAwsPartition() {
- return parseAwsPartition(getRoleARN());
+ String arn = getRoleARN();
+ if (arn != null) {
+ Matcher matcher = ROLE_ARN_PATTERN_COMPILED.matcher(arn);
+ checkState(matcher.matches());
+ return matcher.group(1);
+ }
+ return null;
}
- private static String parseAwsAccountId(String arn) {
+ @Value.Check
+ @Override
+ protected void check() {
+ super.check();
+ String arn = getRoleARN();
validateArn(arn);
- Matcher matcher = ROLE_ARN_PATTERN_COMPILED.matcher(arn);
- if (matcher.matches()) {
- return matcher.group(2);
- } else {
- throw new IllegalArgumentException("ARN does not match the expected role ARN pattern");
+ if (arn != null) {
+ Matcher matcher = ROLE_ARN_PATTERN_COMPILED.matcher(arn);
+ if (!matcher.matches()) {
+ throw new IllegalArgumentException("ARN does not match the expected role ARN pattern");
+ }
}
}
- private static String parseAwsPartition(String arn) {
- validateArn(arn);
- Matcher matcher = ROLE_ARN_PATTERN_COMPILED.matcher(arn);
- if (matcher.matches()) {
- return matcher.group(1);
- } else {
- throw new IllegalArgumentException("ARN does not match the expected role ARN pattern");
+ public static void validateArn(String arn) {
+ if (arn == null) {
+ return;
+ }
+ 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/core/storage/InMemoryStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java
index 1bf6e4bb2..fa7777814 100644
--- a/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java
+++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java
@@ -79,21 +79,6 @@ class InMemoryStorageIntegrationTest {
new PolarisStorageIntegration.ValidationResult(false, "")));
}
- @Test
- public void testAwsAccountIdParsing() {
- AwsStorageConfigurationInfo awsConfig =
- AwsStorageConfigurationInfo.builder()
- .addAllowedLocation("s3://bucket/path/to/warehouse")
- .roleARN("arn:aws:iam::012345678901:role/jdoe")
- .region("us-east-2")
- .build();
-
- String expectedAccountId = "012345678901";
- String actualAccountId = awsConfig.getAwsAccountId();
-
- Assertions.assertThat(actualAccountId).isEqualTo(expectedAccountId);
- }
-
@ParameterizedTest
@ValueSource(strings = {"s3", "s3a"})
public void testValidateAccessToLocationsWithWildcard(String s3Scheme) {
diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfoTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfoTest.java
index faca7193f..0e238775b 100644
--- a/polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfoTest.java
+++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfoTest.java
@@ -22,6 +22,7 @@ package org.apache.polaris.core.storage.aws;
import static org.assertj.core.api.Assertions.assertThat;
import java.net.URI;
+import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
public class AwsStorageConfigurationInfoTest {
@@ -68,7 +69,8 @@ public class AwsStorageConfigurationInfoTest {
}
private static ImmutableAwsStorageConfigurationInfo.Builder newBuilder() {
- return AwsStorageConfigurationInfo.builder().roleARN("role");
+ return AwsStorageConfigurationInfo.builder()
+ .roleARN("arn:aws:iam::123456789012:role/polaris-test");
}
@Test
@@ -115,4 +117,21 @@ public class AwsStorageConfigurationInfoTest {
assertThat(newBuilder().pathStyleAccess(false).build().getPathStyleAccess()).isFalse();
assertThat(newBuilder().pathStyleAccess(true).build().getPathStyleAccess()).isTrue();
}
+
+ @Test
+ public void testRoleArnParsing() {
+ AwsStorageConfigurationInfo awsConfig =
+ AwsStorageConfigurationInfo.builder()
+ .addAllowedLocation("s3://bucket/path/to/warehouse")
+ .roleARN("arn:aws:iam::012345678901:role/jdoe")
+ .region("us-east-2")
+ .build();
+
+ Assertions.assertThat(awsConfig)
+ .extracting(
+ AwsStorageConfigurationInfo::getRoleARN,
+ AwsStorageConfigurationInfo::getAwsAccountId,
+ AwsStorageConfigurationInfo::getAwsPartition)
+ .containsExactly("arn:aws:iam::012345678901:role/jdoe", "012345678901", "aws");
+ }
}
diff --git a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisStorageConfigurationInfoTest.java b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisStorageConfigurationInfoTest.java
index fe15147ed..865cff17e 100644
--- a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisStorageConfigurationInfoTest.java
+++ b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisStorageConfigurationInfoTest.java
@@ -71,30 +71,30 @@ public class PolarisStorageConfigurationInfoTest {
arguments(
AwsStorageConfigurationInfo.builder()
.addAllowedLocations("s3://foo/bar", "s3://no/where")
- .roleARN("arn:foo")
+ .roleARN("arn:aws:iam::123456789012:role/polaris-test")
.region("no-where-1")
.build(),
- "{\"@type\":\"AwsStorageConfigurationInfo\",\"storageType\":\"S3\",\"allowedLocations\":[\"s3://foo/bar\",\"s3://no/where\"],\"roleARN\":\"arn:foo\",\"region\":\"no-where-1\",\"fileIoImplClassName\":\"org.apache.iceberg.aws.s3.S3FileIO\"}"),
+ "{\"@type\":\"AwsStorageConfigurationInfo\",\"storageType\":\"S3\",\"allowedLocations\":[\"s3://foo/bar\",\"s3://no/where\"],\"roleARN\":\"arn:aws:iam::123456789012:role/polaris-test\",\"region\":\"no-where-1\",\"fileIoImplClassName\":\"org.apache.iceberg.aws.s3.S3FileIO\"}"),
arguments(
AwsStorageConfigurationInfo.builder()
.addAllowedLocations("s3://foo/bar", "s3://no/where")
.region("no-where-1")
- .roleARN("arn:foo")
+ .roleARN("arn:aws:iam::123456789012:role/polaris-test")
.externalId("external-id")
.build(),
- "{\"@type\":\"AwsStorageConfigurationInfo\",\"storageType\":\"S3\",\"allowedLocations\":[\"s3://foo/bar\",\"s3://no/where\"],\"roleARN\":\"arn:foo\",\"externalId\":\"external-id\",\"region\":\"no-where-1\",\"fileIoImplClassName\":\"org.apache.iceberg.aws.s3.S3FileIO\"}"),
+ "{\"@type\":\"AwsStorageConfigurationInfo\",\"storageType\":\"S3\",\"allowedLocations\":[\"s3://foo/bar\",\"s3://no/where\"],\"roleARN\":\"arn:aws:iam::123456789012:role/polaris-test\",\"externalId\":\"external-id\",\"region\":\"no-where-1\",\"fileIoImplClassName\":\"org.apache.iceberg.aws.s3.S3FileIO\"}"),
arguments(
AwsStorageConfigurationInfo.builder()
.addAllowedLocations("s3://foo/bar", "s3://no/where")
.region("no-where-1")
- .roleARN("arn:foo")
+ .roleARN("arn:aws:iam::123456789012:role/polaris-test")
.externalId("external-id")
.endpoint("http://127.9.9.9/")
.stsEndpoint("http://127.9.9.9/sts/")
.endpointInternal("http://127.8.8.8/internal/")
.pathStyleAccess(true)
.build(),
- "{\"@type\":\"AwsStorageConfigurationInfo\",\"storageType\":\"S3\",\"allowedLocations\":[\"s3://foo/bar\",\"s3://no/where\"],\"roleARN\":\"arn:foo\",\"externalId\":\"external-id\",\"region\":\"no-where-1\",\"endpoint\":\"http://127.9.9.9/\",\"stsEndpoint\":\"http://127.9.9.9/sts/\",\"endpointInternal\":\"http://127.8.8.8/internal/\",\"pathStyleAccess\":true,\"fileIoImplClassName\":\"org.apache.iceberg.aws.s3.S3FileIO\"}"),
+ "{\"@type\":\"AwsStorageConfigurationInfo\",\"storageType\":\"S3\",\"allowedLocations\":[\"s3://foo/bar\",\"s3://no/where\"],\"roleARN\":\"arn:aws:iam::123456789012:role/polaris-test\",\"externalId\":\"external-id\",\"region\":\"no-where-1\",\"endpoint\":\"http://127.9.9.9/\",\"stsEndpoint\":\"http://127.9.9.9/sts/\",\"endpointInternal\":\"http://127.8.8.8/internal/\",\"pathStyleAccess\":true,\"fileIoImplClassName\":\"org.apache.iceberg.aws.s3.S3FileIO\"}"),
//
arguments(
GcpStorageConfigurationInfo.builder()
diff --git a/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java b/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java
index d7f764dd5..bbd66512d 100644
--- a/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java
+++ b/runtime/service/src/intTest/java/org/apache/polaris/service/it/PolarisRestCatalogMinIOIT.java
@@ -84,9 +84,6 @@ public class PolarisRestCatalogMinIOIT extends PolarisRestCatalogIntegrationBase
protected StorageConfigInfo getStorageConfigInfo() {
AwsStorageConfigInfo.Builder storageConfig =
AwsStorageConfigInfo.builder()
- .setRoleArn("arn:aws:iam::123456789012:role/polaris-test")
- .setExternalId("externalId123")
- .setUserArn("arn:aws:iam::123456789012:user/polaris-test")
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
.setPathStyleAccess(true)
.setEndpoint(endpoint)
diff --git a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java
index b4d171d1b..726faa163 100644
--- a/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java
+++ b/runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java
@@ -165,9 +165,6 @@ public class RestCatalogMinIOSpecialIT {
Optional<String> endpointInternal) {
AwsStorageConfigInfo.Builder storageConfig =
AwsStorageConfigInfo.builder()
- .setRoleArn("arn:aws:iam::123456789012:role/polaris-test")
- .setExternalId("externalId123")
- .setUserArn("arn:aws:iam::123456789012:user/polaris-test")
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
.setPathStyleAccess(pathStyleAccess)
.setAllowedLocations(List.of(storageBase.toString()));
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
index bfb77ac26..b6c849985 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
@@ -893,7 +893,7 @@ public class PolarisAdminService {
if (currentStorageConfig instanceof AwsStorageConfigurationInfo currentAwsConfig
&& newStorageConfig instanceof AwsStorageConfigurationInfo newAwsConfig) {
- if (!currentAwsConfig.getAwsAccountId().equals(newAwsConfig.getAwsAccountId())) {
+ if (!Objects.equals(currentAwsConfig.getAwsAccountId(), newAwsConfig.getAwsAccountId())) {
throw new BadRequestException(
"Cannot modify Role ARN in storage config from %s to %s",
currentStorageConfig, newStorageConfig);
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 a9969d04a..4d774f8b3 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
@@ -259,18 +259,12 @@ public class CatalogEntityTest {
.setProperties(prop)
.setStorageConfigInfo(awsStorageConfigModel)
.build();
- String expectedMessage = "";
- switch (roleArn) {
- case "":
- expectedMessage = "ARN cannot be null or empty";
- break;
- case "aws-cn":
- expectedMessage = "AWS China is temporarily not supported";
- break;
- default:
- expectedMessage = "Invalid role ARN format";
- }
- ;
+ 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";
+ };
Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(callContext, awsCatalog))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(expectedMessage);
diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml
index a7398bd39..00c2a8809 100644
--- a/spec/polaris-management-service.yml
+++ b/spec/polaris-management-service.yml
@@ -1073,8 +1073,6 @@ components:
Whether S3 requests to files in this catalog should use 'path-style addressing for buckets'.
example: true
default: false
- required:
- - roleArn
AzureStorageConfigInfo:
type: object