Skip to content

Commit 9507ab0

Browse files
committed
addressing comments from nastra
1 parent 10a5c85 commit 9507ab0

File tree

2 files changed

+29
-37
lines changed

2 files changed

+29
-37
lines changed

aws/src/integration/java/org/apache/iceberg/aws/TestKeyManagementClient.java

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,33 @@ public static void beforeClass() {
6868
keyId = response.keyMetadata().keyId();
6969
}
7070

71+
@AfterAll
72+
public static void afterClass() {
73+
// AWS KMS doesn't allow instant deletion. Keys can be put to pendingDeletion state instead,
74+
// with a minimum of 7 days until final removal.
75+
ScheduleKeyDeletionRequest deletionRequest =
76+
ScheduleKeyDeletionRequest.builder().keyId(keyId).pendingWindowInDays(7).build();
77+
78+
ScheduleKeyDeletionResponse deletionResponse = kmsClient.scheduleKeyDeletion(deletionRequest);
79+
LOG.info(
80+
"Deletion of test key {} will be finalized at {}", keyId, deletionResponse.deletionDate());
81+
82+
try {
83+
kmsClient.close();
84+
} catch (Exception e) {
85+
LOG.error("Error closing KMS client", e);
86+
}
87+
}
88+
7189
@Test
7290
public void testKeyWrapping() {
73-
AwsKeyManagementClient keyManagementClient = new AwsKeyManagementClient();
74-
try {
91+
try (AwsKeyManagementClient keyManagementClient = new AwsKeyManagementClient()) {
7592
keyManagementClient.initialize(ImmutableMap.of());
7693

7794
ByteBuffer key = ByteBuffer.wrap(new String("super-secret-table-master-key").getBytes());
7895
ByteBuffer encryptedKey = keyManagementClient.wrapKey(key, keyId);
7996

8097
assertThat(keyManagementClient.unwrapKey(encryptedKey, keyId)).isEqualTo(key);
81-
} finally {
82-
keyManagementClient.close();
8398
}
8499
}
85100

@@ -89,8 +104,7 @@ public void testKeyWrapping() {
89104
value = DataKeySpec.class,
90105
names = {"AES_128", "AES_256"})
91106
public void testKeyGeneration(DataKeySpec dataKeySpec) {
92-
AwsKeyManagementClient keyManagementClient = new AwsKeyManagementClient();
93-
try {
107+
try (AwsKeyManagementClient keyManagementClient = new AwsKeyManagementClient()) {
94108
Map<String, String> properties =
95109
dataKeySpec == null
96110
? ImmutableMap.of()
@@ -100,8 +114,6 @@ public void testKeyGeneration(DataKeySpec dataKeySpec) {
100114

101115
assertThat(keyManagementClient.unwrapKey(result.wrappedKey(), keyId)).isEqualTo(result.key());
102116
assertThat(result.key().limit()).isEqualTo(expectedLength(dataKeySpec));
103-
} finally {
104-
keyManagementClient.close();
105117
}
106118
}
107119

@@ -112,22 +124,4 @@ private static int expectedLength(DataKeySpec spec) {
112124
return 256 / 8;
113125
}
114126
}
115-
116-
@AfterAll
117-
public static void afterClass() {
118-
// AWS KMS doesn't allow instant deletion. Keys can be put to pendingDeletion state instead,
119-
// with a minimum of 7 days until final removal.
120-
ScheduleKeyDeletionRequest deletionRequest =
121-
ScheduleKeyDeletionRequest.builder().keyId(keyId).pendingWindowInDays(7).build();
122-
123-
ScheduleKeyDeletionResponse deletionResponse = kmsClient.scheduleKeyDeletion(deletionRequest);
124-
LOG.info(
125-
"Deletion of test key {} will be finalized at {}", keyId, deletionResponse.deletionDate());
126-
127-
try {
128-
kmsClient.close();
129-
} catch (Exception e) {
130-
LOG.error("Error closing KMS client", e);
131-
}
132-
}
133127
}

aws/src/main/java/org/apache/iceberg/aws/AwsKeyManagementClient.java

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,15 @@ public class AwsKeyManagementClient implements KeyManagementClient {
4343
private EncryptionAlgorithmSpec encryptionAlgorithmSpec;
4444
private DataKeySpec dataKeySpec;
4545

46-
public AwsKeyManagementClient() {}
46+
@Override
47+
public void initialize(Map<String, String> properties) {
48+
AwsClientFactory clientFactory = AwsClientFactories.from(properties);
49+
this.kmsClient = clientFactory.kms();
50+
51+
AwsProperties awsProperties = new AwsProperties(properties);
52+
this.encryptionAlgorithmSpec = awsProperties.kmsEncryptionAlgorithmSpec();
53+
this.dataKeySpec = awsProperties.kmsDataKeySpec();
54+
}
4755

4856
@Override
4957
public ByteBuffer wrapKey(ByteBuffer key, String wrappingKeyId) {
@@ -88,16 +96,6 @@ public ByteBuffer unwrapKey(ByteBuffer wrappedKey, String wrappingKeyId) {
8896
return result.plaintext().asByteBuffer();
8997
}
9098

91-
@Override
92-
public void initialize(Map<String, String> properties) {
93-
AwsClientFactory clientFactory = AwsClientFactories.from(properties);
94-
this.kmsClient = clientFactory.kms();
95-
96-
AwsProperties awsProperties = new AwsProperties(properties);
97-
this.encryptionAlgorithmSpec = awsProperties.kmsEncryptionAlgorithmSpec();
98-
this.dataKeySpec = awsProperties.kmsDataKeySpec();
99-
}
100-
10199
@Override
102100
public void close() {
103101
if (kmsClient != null) {

0 commit comments

Comments
 (0)