adnanhemani commented on code in PR #1424:
URL: https://github.com/apache/polaris/pull/1424#discussion_r2110151239
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java:
##########
@@ -185,7 +190,32 @@ private IamPolicy policyString(
bucketGetLocationStatementBuilder
.values()
.forEach(statementBuilder ->
policyBuilder.addStatement(statementBuilder.build()));
- return
policyBuilder.addStatement(allowGetObjectStatementBuilder.build()).build();
+
+ policyBuilder.addStatement(allowGetObjectStatementBuilder.build());
+
+ policyBuilder.addStatement(
+ IamStatement.builder()
+ .effect(IamEffect.ALLOW)
+ .addAction("kms:GenerateDataKey")
+ .addAction("kms:Decrypt")
+ .addAction("kms:DescribeKey")
+ .addResource("arn:aws:kms:" + region + ":" + awsAccountId +
":key/*")
+ .addCondition(IamConditionOperator.STRING_EQUALS,
"aws:PrincipalArn", roleARN)
+ .addCondition(
+ IamConditionOperator.STRING_LIKE,
+ "kms:EncryptionContext:aws:s3:arn",
+ getArnPrefixFor(roleARN)
+ + StorageUtil.getBucket(
+ URI.create(
+
awsStorageConfigurationInfo.getAllowedLocations().iterator().next()))
Review Comment:
That makes sense - and I could be wrong, but I'm not sure we can make the
assumption that the first value in `getAllowedLocations()` will always be the
default base location?
Don't we need to get that information directly from the catalog? i.e.
through calling `CatalogEntity.getDefaultBaseLocation()` or something similar?
In fact, there are some callers where I see the default base location being
added to the end of the array list - but we shouldn't rely on the ordering of
the allowed locations list ideally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
