This is an automated email from the ASF dual-hosted git repository.
yzheng 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 c2785bd61 Remove KMS policies when KMS is not configured and improved
default KMS permission for RO/RW (#3493)
c2785bd61 is described below
commit c2785bd61732d2df16e8d32e49017710965fff6d
Author: Yong Zheng <[email protected]>
AuthorDate: Fri Jan 23 20:33:17 2026 -0600
Remove KMS policies when KMS is not configured and improved default KMS
permission for RO/RW (#3493)
---
CHANGELOG.md | 6 ++-
.../aws/AwsCredentialsStorageIntegration.java | 36 ++++++++-----
.../aws/AwsCredentialsStorageIntegrationTest.java | 62 +++++++++++++++++-----
3 files changed, 76 insertions(+), 28 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1eaec3be4..991155bff 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -53,8 +53,10 @@ request adding CHANGELOG notes for breaking (!) changes and
possibly other secti
- The `gcpServiceAccount` configuration value now affects Polaris behavior
(enables service account impersonation). This value was previously defined but
unused. This change may affect existing deployments that have populated this
property.
- (Before/After)UpdateTableEvent is emitted for all table updates within a
transaction.
-- Added KMS options to Polaris CLI
-- Changed from Poetry to UV for Python package management
+- Added KMS options to Polaris CLI.
+- Changed from Poetry to UV for Python package management.
+- Exclude KMS policies when KMS is not being used for S3.
+- Improved default KMS permission handling to better distinguish read-only and
read-write access.
### Deprecations
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 8fce267af..265b227e0 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
@@ -299,9 +299,16 @@ public class AwsCredentialsStorageIntegration
String region,
String accountId) {
- IamStatement.Builder allowKms = buildBaseKmsStatement(canWrite);
boolean hasCurrentKey = kmsKeyArn != null;
boolean hasAllowedKeys = hasAllowedKmsKeys(allowedKmsKeys);
+ boolean isAwsS3 = region != null && accountId != null;
+
+ // Nothing to do if no keys are configured and not AWS S3
+ if (!hasCurrentKey && !hasAllowedKeys && !isAwsS3) {
+ return;
+ }
+
+ IamStatement.Builder allowKms = buildBaseKmsStatement(canWrite);
if (hasCurrentKey) {
addKmsKeyResource(kmsKeyArn, allowKms);
@@ -311,16 +318,16 @@ public class AwsCredentialsStorageIntegration
addAllowedKmsKeyResources(allowedKmsKeys, allowKms);
}
- // Add KMS statement if we have any KMS key configuration
- if (hasCurrentKey || hasAllowedKeys) {
+ // Only add wildcard KMS access for read-only operations on AWS S3 when no
specific keys are
+ // configured. This does not apply to services like Minio where region and
accountId are not
+ // available.
+ boolean shouldAddWildcard = !hasCurrentKey && !hasAllowedKeys && !canWrite
&& isAwsS3;
+ if (shouldAddWildcard) {
+ addAllKeysResource(region, accountId, allowKms);
+ }
+
+ if (hasCurrentKey || hasAllowedKeys || shouldAddWildcard) {
policyBuilder.addStatement(allowKms.build());
- } else if (!canWrite) {
- // Only add wildcard KMS access for read-only operations when no
specific keys are configured
- // this check is for minio because it doesn't have region or account id
- if (region != null && accountId != null) {
- addAllKeysResource(region, accountId, allowKms);
- policyBuilder.addStatement(allowKms.build());
- }
}
}
@@ -328,13 +335,14 @@ public class AwsCredentialsStorageIntegration
IamStatement.Builder allowKms =
IamStatement.builder()
.effect(IamEffect.ALLOW)
- .addAction("kms:GenerateDataKeyWithoutPlaintext")
.addAction("kms:DescribeKey")
- .addAction("kms:Decrypt")
- .addAction("kms:GenerateDataKey");
+ .addAction("kms:Decrypt");
if (canEncrypt) {
- allowKms.addAction("kms:Encrypt");
+ allowKms
+ .addAction("kms:Encrypt")
+ .addAction("kms:GenerateDataKey")
+ .addAction("kms:GenerateDataKeyWithoutPlaintext");
}
return allowKms;
diff --git
a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
index 082eed3ec..16e0893f6 100644
---
a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
+++
b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
@@ -468,11 +468,8 @@ class AwsCredentialsStorageIntegrationTest extends
BaseStorageIntegrationTest {
.returns(IamEffect.ALLOW,
IamStatement::effect)
.returns(
List.of(
- IamAction.create(
-
"kms:GenerateDataKeyWithoutPlaintext"),
IamAction.create("kms:DescribeKey"),
-
IamAction.create("kms:Decrypt"),
-
IamAction.create("kms:GenerateDataKey")),
+
IamAction.create("kms:Decrypt")),
IamStatement::actions)
.returns(
List.of(
@@ -583,11 +580,8 @@ class AwsCredentialsStorageIntegrationTest extends
BaseStorageIntegrationTest {
.returns(IamEffect.ALLOW,
IamStatement::effect)
.returns(
List.of(
- IamAction.create(
-
"kms:GenerateDataKeyWithoutPlaintext"),
IamAction.create("kms:DescribeKey"),
-
IamAction.create("kms:Decrypt"),
-
IamAction.create("kms:GenerateDataKey")),
+
IamAction.create("kms:Decrypt")),
IamStatement::actions)
.returns(
List.of(
@@ -824,11 +818,14 @@ class AwsCredentialsStorageIntegrationTest extends
BaseStorageIntegrationTest {
assertThat(stmt.actions())
.containsAll(
List.of(
-
IamAction.create("kms:GenerateDataKeyWithoutPlaintext"),
IamAction.create("kms:DescribeKey"),
- IamAction.create("kms:Decrypt"),
- IamAction.create("kms:GenerateDataKey")));
-
assertThat(stmt.actions()).doesNotContain(IamAction.create("kms:Encrypt"));
+ IamAction.create("kms:Decrypt")));
+ assertThat(stmt.actions())
+ .doesNotContainAnyElementsOf(
+ List.of(
+ IamAction.create("kms:Encrypt"),
+ IamAction.create("kms:GenerateDataKey"),
+
IamAction.create("kms:GenerateDataKeyWithoutPlaintext")));
assertThat(stmt.resources())
.containsExactlyInAnyOrder(
IamResource.create(allowedKmsKeys.get(0)),
@@ -1473,4 +1470,45 @@ class AwsCredentialsStorageIntegrationTest extends
BaseStorageIntegrationTest {
.isInstanceOf(software.amazon.awssdk.services.sts.model.StsException.class)
.hasMessageContaining("sts:TagSession");
}
+
+ @Test
+ public void testNoKmsForNonAwsReadOnly() {
+ StsClient stsClient = Mockito.mock(StsClient.class);
+ String roleARN = "arn:aws:iam::012345678901:role/jdoe";
+ String externalId = "externalId";
+ String bucket = "bucket";
+ String warehouseKeyPrefix = "path/to/warehouse";
+
+ // Test with no KMS keys and read-only for non-AWS (should not add any KMS
statement)
+ Mockito.when(stsClient.assumeRole(Mockito.isA(AssumeRoleRequest.class)))
+ .thenAnswer(
+ invocation -> {
+ AssumeRoleRequest request = invocation.getArgument(0);
+ IamPolicy policy = IamPolicy.fromJson(request.policy());
+
+ // Verify no KMS statement exists
+ assertThat(policy.statements())
+ .noneMatch(
+ stmt ->
+ stmt.actions().stream()
+ .anyMatch(action ->
action.value().startsWith("kms:")));
+ return ASSUME_ROLE_RESPONSE;
+ });
+
+ new AwsCredentialsStorageIntegration(
+ AwsStorageConfigurationInfo.builder()
+ .addAllowedLocation(s3Path(bucket, warehouseKeyPrefix))
+ .roleARN(roleARN)
+ .externalId(externalId)
+ .build(),
+ stsClient)
+ .getSubscopedCreds(
+ EMPTY_REALM_CONFIG,
+ true,
+ Set.of(s3Path(bucket, warehouseKeyPrefix + "/table")),
+ Set.of(),
+ POLARIS_PRINCIPAL,
+ Optional.empty(),
+ CredentialVendingContext.empty());
+ }
}
