Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: S3 storage configuration property [...]


### Deprecations

* The property `polaris.active-roles-provider.type` is deprecated in favor of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ public void testJsonFormat() throws JsonProcessingException {
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);

Expand All @@ -83,7 +85,9 @@ private static Stream<Arguments> catalogTestCases() {
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",
Expand All @@ -102,14 +106,16 @@ private static Stream<Arguments> catalogTestCases() {
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(
Expand All @@ -131,7 +137,9 @@ private static Stream<Arguments> catalogTestCases() {
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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,9 @@ public void testCreateAndUpdateAzureCatalog() {
@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()
Expand Down Expand Up @@ -649,8 +650,9 @@ public void testCreateListUpdateAndDeleteCatalog() {

// 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(),
Expand All @@ -674,8 +676,9 @@ public void testCreateListUpdateAndDeleteCatalog() {
// 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(),
Expand Down Expand Up @@ -772,9 +775,7 @@ public void testCatalogRoleInvalidName() {
.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);

Expand Down Expand Up @@ -1204,9 +1205,7 @@ public void testCreateListUpdateAndDeleteCatalogRole() {
.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);

Expand All @@ -1215,9 +1214,7 @@ public void testCreateListUpdateAndDeleteCatalogRole() {
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);
Expand Down Expand Up @@ -1512,9 +1509,7 @@ public void testAssignListAndRevokeCatalogRoles() {
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);
Expand All @@ -1534,9 +1529,7 @@ public void testAssignListAndRevokeCatalogRoles() {
.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);

Expand Down Expand Up @@ -1677,9 +1670,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRoles() {
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);
Expand Down Expand Up @@ -1766,9 +1757,7 @@ public void testServiceAdminCanTransferCatalogAdmin() {
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);
Expand Down Expand Up @@ -1838,9 +1827,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRolesFromWrongCatalog() {
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);
Expand All @@ -1851,9 +1838,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRolesFromWrongCatalog() {
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);
Expand Down Expand Up @@ -1905,9 +1890,7 @@ public void testTableManageAccessCanGrantAndRevokeFromCatalogRoles() {
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);
Expand All @@ -1921,9 +1904,7 @@ public void testTableManageAccessCanGrantAndRevokeFromCatalogRoles() {
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);
Expand Down Expand Up @@ -2095,9 +2076,7 @@ public void testNamespaceExistsStatus() {
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);
Expand All @@ -2121,9 +2100,7 @@ public void testDropNamespaceStatus() {
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);
Expand All @@ -2147,9 +2124,7 @@ public void testCreateAndUpdateCatalogRoleWithReservedProperties() {
.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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,6 @@ public void before(TestInfo testInfo) {
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ public Builder setStorageConfigurationInfo(
.pathStyleAccess(awsConfigModel.getPathStyleAccess())
.endpointInternal(awsConfigModel.getEndpointInternal())
.build();
awsConfig.validateArn(awsConfigModel.getRoleArn());
config = awsConfig;
break;
case AZURE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public AccessConfig getSubscopedCreds(
.roleSessionName("PolarisAwsCredentialsStorageIntegration")
.policy(
policyString(
storageConfig.getRoleARN(),
storageConfig.getAwsPartition(),
allowListOperation,
allowedReadLocations,
allowedWriteLocations)
Expand Down Expand Up @@ -134,7 +134,7 @@ public AccessConfig getSubscopedCreds(
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()));
Expand All @@ -152,7 +152,10 @@ public AccessConfig getSubscopedCreds(
*/
// 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()
Expand All @@ -162,7 +165,7 @@ private IamPolicy policyString(
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(
Expand Down Expand Up @@ -226,14 +229,8 @@ private IamPolicy policyString(
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) {
Expand Down
Loading