Skip to content

Commit d7d8990

Browse files
snazysnazy
authored
Make S3 roleARN optional (#2329)
Fixes #2325
1 parent cc03796 commit d7d8990

File tree

15 files changed

+122
-140
lines changed

15 files changed

+122
-140
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ at locations that better optimize for object storage.
5151

5252
- Polaris Management API clients must be prepared to deal with new attributes in `AwsStorageConfigInfo` objects.
5353

54+
- S3 configuration property role-ARN is no longer mandatory.
55+
5456
### Deprecations
5557

5658
* The property `polaris.active-roles-provider.type` is deprecated in favor of

‎api/management-model/src/test/java/org/apache/polaris/core/admin/model/CatalogSerializationTest.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ public void testJsonFormat() throws JsonProcessingException {
5757
Catalog.TypeEnum.INTERNAL,
5858
TEST_CATALOG_NAME,
5959
new CatalogProperties(TEST_LOCATION),
60-
new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3));
60+
AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
61+
.setRoleArn(TEST_ROLE_ARN)
62+
.build());
6163

6264
String json = mapper.writeValueAsString(catalog);
6365

@@ -83,7 +85,9 @@ private static Stream<Arguments> catalogTestCases() {
8385
Catalog.TypeEnum.INTERNAL,
8486
TEST_CATALOG_NAME,
8587
new CatalogProperties(TEST_LOCATION),
86-
new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3))),
88+
AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
89+
.setRoleArn(TEST_ROLE_ARN)
90+
.build())),
8791
Arguments.of("Null fields", new Catalog(Catalog.TypeEnum.INTERNAL, null, null, null)),
8892
Arguments.of(
8993
"Long name",
@@ -102,14 +106,16 @@ private static Stream<Arguments> catalogTestCases() {
102106
Catalog.TypeEnum.INTERNAL,
103107
"",
104108
new CatalogProperties(""),
105-
new AwsStorageConfigInfo("", StorageConfigInfo.StorageTypeEnum.S3))),
109+
new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))),
106110
Arguments.of(
107111
"Special characters",
108112
new Catalog(
109113
Catalog.TypeEnum.INTERNAL,
110114
"test\"catalog",
111115
new CatalogProperties(TEST_LOCATION),
112-
new AwsStorageConfigInfo(TEST_ROLE_ARN, StorageConfigInfo.StorageTypeEnum.S3))),
116+
AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
117+
.setRoleArn(TEST_ROLE_ARN)
118+
.build())),
113119
Arguments.of(
114120
"Whitespace",
115121
new Catalog(
@@ -131,7 +137,9 @@ private static Stream<Arguments> catalogTestCases() {
131137
Catalog.TypeEnum.INTERNAL,
132138
TEST_CATALOG_NAME,
133139
new CatalogProperties(TEST_LOCATION),
134-
new AwsStorageConfigInfo(arn, StorageConfigInfo.StorageTypeEnum.S3))));
140+
AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
141+
.setRoleArn(arn)
142+
.build())));
135143

136144
return Stream.concat(basicCases, arnCases);
137145
}

‎integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java

Lines changed: 23 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -605,8 +605,9 @@ public void testCreateAndUpdateAzureCatalog() {
605605
@Test
606606
public void testCreateListUpdateAndDeleteCatalog() {
607607
StorageConfigInfo storageConfig =
608-
new AwsStorageConfigInfo(
609-
"arn:aws:iam::123456789011:role/role1", StorageConfigInfo.StorageTypeEnum.S3);
608+
AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
609+
.setRoleArn("arn:aws:iam::123456789011:role/role1")
610+
.build();
610611
String catalogName = client.newEntityName("mycatalog");
611612
Catalog catalog =
612613
PolarisCatalog.builder()
@@ -649,8 +650,9 @@ public void testCreateListUpdateAndDeleteCatalog() {
649650

650651
// Reject update of fields that can't be currently updated
651652
StorageConfigInfo invalidModifiedStorageConfig =
652-
new AwsStorageConfigInfo(
653-
"arn:aws:iam::123456789012:role/newrole", StorageConfigInfo.StorageTypeEnum.S3);
653+
AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
654+
.setRoleArn("arn:aws:iam::123456789012:role/newrole")
655+
.build();
654656
UpdateCatalogRequest badUpdateRequest =
655657
new UpdateCatalogRequest(
656658
fetchedCatalog.getEntityVersion(),
@@ -674,8 +676,9 @@ public void testCreateListUpdateAndDeleteCatalog() {
674676
// AWS
675677
// account IDs are same)
676678
StorageConfigInfo validModifiedStorageConfig =
677-
new AwsStorageConfigInfo(
678-
"arn:aws:iam::123456789011:role/newrole", StorageConfigInfo.StorageTypeEnum.S3);
679+
AwsStorageConfigInfo.builder(StorageConfigInfo.StorageTypeEnum.S3)
680+
.setRoleArn("arn:aws:iam::123456789011:role/newrole")
681+
.build();
679682
UpdateCatalogRequest updateRequest =
680683
new UpdateCatalogRequest(
681684
fetchedCatalog.getEntityVersion(),
@@ -772,9 +775,7 @@ public void testCatalogRoleInvalidName() {
772775
.setType(Catalog.TypeEnum.INTERNAL)
773776
.setName(catalogName)
774777
.setProperties(new CatalogProperties("s3://required/base/location"))
775-
.setStorageConfigInfo(
776-
new AwsStorageConfigInfo(
777-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
778+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
778779
.build();
779780
managementApi.createCatalog(catalog);
780781

@@ -1204,9 +1205,7 @@ public void testCreateListUpdateAndDeleteCatalogRole() {
12041205
.setType(Catalog.TypeEnum.INTERNAL)
12051206
.setName(catalogName)
12061207
.setProperties(new CatalogProperties("s3://required/base/location"))
1207-
.setStorageConfigInfo(
1208-
new AwsStorageConfigInfo(
1209-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1208+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
12101209
.build();
12111210
managementApi.createCatalog(catalog);
12121211

@@ -1215,9 +1214,7 @@ public void testCreateListUpdateAndDeleteCatalogRole() {
12151214
PolarisCatalog.builder()
12161215
.setType(Catalog.TypeEnum.INTERNAL)
12171216
.setName(catalogName2)
1218-
.setStorageConfigInfo(
1219-
new AwsStorageConfigInfo(
1220-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1217+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
12211218
.setProperties(new CatalogProperties("s3://required/base/other_location"))
12221219
.build();
12231220
managementApi.createCatalog(catalog2);
@@ -1512,9 +1509,7 @@ public void testAssignListAndRevokeCatalogRoles() {
15121509
PolarisCatalog.builder()
15131510
.setType(Catalog.TypeEnum.INTERNAL)
15141511
.setName(client.newEntityName("mycatalog"))
1515-
.setStorageConfigInfo(
1516-
new AwsStorageConfigInfo(
1517-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1512+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
15181513
.setProperties(new CatalogProperties("s3://bucket1/"))
15191514
.build();
15201515
managementApi.createCatalog(catalog);
@@ -1534,9 +1529,7 @@ public void testAssignListAndRevokeCatalogRoles() {
15341529
.setType(Catalog.TypeEnum.INTERNAL)
15351530
.setName(client.newEntityName("othercatalog"))
15361531
.setProperties(new CatalogProperties("s3://path/to/data"))
1537-
.setStorageConfigInfo(
1538-
new AwsStorageConfigInfo(
1539-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1532+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
15401533
.build();
15411534
managementApi.createCatalog(otherCatalog);
15421535

@@ -1677,9 +1670,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRoles() {
16771670
PolarisCatalog.builder()
16781671
.setType(Catalog.TypeEnum.INTERNAL)
16791672
.setName(catalogName)
1680-
.setStorageConfigInfo(
1681-
new AwsStorageConfigInfo(
1682-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1673+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
16831674
.setProperties(new CatalogProperties("s3://bucket1/"))
16841675
.build();
16851676
managementApi.createCatalog(catalog);
@@ -1766,9 +1757,7 @@ public void testServiceAdminCanTransferCatalogAdmin() {
17661757
PolarisCatalog.builder()
17671758
.setType(Catalog.TypeEnum.INTERNAL)
17681759
.setName(catalogName)
1769-
.setStorageConfigInfo(
1770-
new AwsStorageConfigInfo(
1771-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1760+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
17721761
.setProperties(new CatalogProperties("s3://bucket1/"))
17731762
.build();
17741763
managementApi.createCatalog(catalog);
@@ -1838,9 +1827,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRolesFromWrongCatalog() {
18381827
PolarisCatalog.builder()
18391828
.setType(Catalog.TypeEnum.INTERNAL)
18401829
.setName(catalogName)
1841-
.setStorageConfigInfo(
1842-
new AwsStorageConfigInfo(
1843-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1830+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
18441831
.setProperties(new CatalogProperties("s3://bucket1/"))
18451832
.build();
18461833
managementApi.createCatalog(catalog);
@@ -1851,9 +1838,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRolesFromWrongCatalog() {
18511838
PolarisCatalog.builder()
18521839
.setType(Catalog.TypeEnum.INTERNAL)
18531840
.setName(catalogName2)
1854-
.setStorageConfigInfo(
1855-
new AwsStorageConfigInfo(
1856-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1841+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
18571842
.setProperties(new CatalogProperties("s3://bucket1/"))
18581843
.build();
18591844
managementApi.createCatalog(catalog2);
@@ -1905,9 +1890,7 @@ public void testTableManageAccessCanGrantAndRevokeFromCatalogRoles() {
19051890
PolarisCatalog.builder()
19061891
.setType(Catalog.TypeEnum.INTERNAL)
19071892
.setName(catalogName)
1908-
.setStorageConfigInfo(
1909-
new AwsStorageConfigInfo(
1910-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1893+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
19111894
.setProperties(new CatalogProperties("s3://bucket1/"))
19121895
.build();
19131896
managementApi.createCatalog(catalog);
@@ -1921,9 +1904,7 @@ public void testTableManageAccessCanGrantAndRevokeFromCatalogRoles() {
19211904
PolarisCatalog.builder()
19221905
.setType(Catalog.TypeEnum.INTERNAL)
19231906
.setName(catalogName2)
1924-
.setStorageConfigInfo(
1925-
new AwsStorageConfigInfo(
1926-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
1907+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
19271908
.setProperties(new CatalogProperties("s3://bucket1/"))
19281909
.build();
19291910
managementApi.createCatalog(catalog2);
@@ -2095,9 +2076,7 @@ public void testNamespaceExistsStatus() {
20952076
PolarisCatalog.builder()
20962077
.setType(Catalog.TypeEnum.INTERNAL)
20972078
.setName(catalogName)
2098-
.setStorageConfigInfo(
2099-
new AwsStorageConfigInfo(
2100-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
2079+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
21012080
.setProperties(new CatalogProperties("s3://bucket1/"))
21022081
.build();
21032082
managementApi.createCatalog(catalog);
@@ -2121,9 +2100,7 @@ public void testDropNamespaceStatus() {
21212100
PolarisCatalog.builder()
21222101
.setType(Catalog.TypeEnum.INTERNAL)
21232102
.setName(catalogName)
2124-
.setStorageConfigInfo(
2125-
new AwsStorageConfigInfo(
2126-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
2103+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
21272104
.setProperties(new CatalogProperties("s3://bucket1/"))
21282105
.build();
21292106
managementApi.createCatalog(catalog);
@@ -2147,9 +2124,7 @@ public void testCreateAndUpdateCatalogRoleWithReservedProperties() {
21472124
.setType(Catalog.TypeEnum.INTERNAL)
21482125
.setName(catalogName)
21492126
.setProperties(new CatalogProperties("s3://required/base/location"))
2150-
.setStorageConfigInfo(
2151-
new AwsStorageConfigInfo(
2152-
"arn:aws:iam::012345678901:role/jdoe", StorageConfigInfo.StorageTypeEnum.S3))
2127+
.setStorageConfigInfo(new AwsStorageConfigInfo(StorageConfigInfo.StorageTypeEnum.S3))
21532128
.build();
21542129
managementApi.createCatalog(catalog);
21552130

‎integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,6 @@ public void before(TestInfo testInfo) {
184184
currentCatalogName = client.newEntityName(method.getName());
185185
AwsStorageConfigInfo awsConfigModel =
186186
AwsStorageConfigInfo.builder()
187-
.setRoleArn(TEST_ROLE_ARN)
188-
.setExternalId("externalId")
189-
.setUserArn("a:user:arn")
190187
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
191188
.setAllowedLocations(List.of("s3://my-old-bucket/path/to/data"))
192189
.build();

‎polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,6 @@ public Builder setStorageConfigurationInfo(
279279
.pathStyleAccess(awsConfigModel.getPathStyleAccess())
280280
.endpointInternal(awsConfigModel.getEndpointInternal())
281281
.build();
282-
awsConfig.validateArn(awsConfigModel.getRoleArn());
283282
config = awsConfig;
284283
break;
285284
case AZURE:

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public AccessConfig getSubscopedCreds(
8585
.roleSessionName("PolarisAwsCredentialsStorageIntegration")
8686
.policy(
8787
policyString(
88-
storageConfig.getRoleARN(),
88+
storageConfig.getAwsPartition(),
8989
allowListOperation,
9090
allowedReadLocations,
9191
allowedWriteLocations)
@@ -134,7 +134,7 @@ public AccessConfig getSubscopedCreds(
134134
accessConfig.put(StorageAccessProperty.AWS_PATH_STYLE_ACCESS, Boolean.TRUE.toString());
135135
}
136136

137-
if (storageConfig.getAwsPartition().equals("aws-us-gov") && region == null) {
137+
if ("aws-us-gov".equals(storageConfig.getAwsPartition()) && region == null) {
138138
throw new IllegalArgumentException(
139139
String.format(
140140
"AWS region must be set when using partition %s", storageConfig.getAwsPartition()));
@@ -152,7 +152,10 @@ public AccessConfig getSubscopedCreds(
152152
*/
153153
// TODO - add KMS key access
154154
private IamPolicy policyString(
155-
String roleArn, boolean allowList, Set<String> readLocations, Set<String> writeLocations) {
155+
String awsPartition,
156+
boolean allowList,
157+
Set<String> readLocations,
158+
Set<String> writeLocations) {
156159
IamPolicy.Builder policyBuilder = IamPolicy.builder();
157160
IamStatement.Builder allowGetObjectStatementBuilder =
158161
IamStatement.builder()
@@ -162,7 +165,7 @@ private IamPolicy policyString(
162165
Map<String, IamStatement.Builder> bucketListStatementBuilder = new HashMap<>();
163166
Map<String, IamStatement.Builder> bucketGetLocationStatementBuilder = new HashMap<>();
164167

165-
String arnPrefix = getArnPrefixFor(roleArn);
168+
String arnPrefix = arnPrefixForPartition(awsPartition);
166169
Stream.concat(readLocations.stream(), writeLocations.stream())
167170
.distinct()
168171
.forEach(
@@ -226,14 +229,8 @@ private IamPolicy policyString(
226229
return policyBuilder.addStatement(allowGetObjectStatementBuilder.build()).build();
227230
}
228231

229-
private String getArnPrefixFor(String roleArn) {
230-
if (roleArn.contains("aws-cn")) {
231-
return "arn:aws-cn:s3:::";
232-
} else if (roleArn.contains("aws-us-gov")) {
233-
return "arn:aws-us-gov:s3:::";
234-
} else {
235-
return "arn:aws:s3:::";
236-
}
232+
private static String arnPrefixForPartition(String awsPartition) {
233+
return String.format("arn:%s:s3:::", awsPartition != null ? awsPartition : "aws");
237234
}
238235

239236
private static @Nonnull String parseS3Path(URI uri) {

0 commit comments

Comments
 (0)