singhpk234 commented on code in PR #18337:
URL: https://github.com/apache/iceberg/pull/18337#discussion_r4152512396
##########
core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java:
##########
@@ -64,6 +65,15 @@ public StandardEncryptionManager(
String tableKeyId,
int dataKeyLength,
KeyManagementClient kmsClient) {
+ this(keys, tableKeyId, dataKeyLength, kmsClient, false);
Review Comment:
wouldn;t we want to mark it as deprecated ?
##########
core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java:
##########
@@ -146,8 +157,17 @@ String keyEncryptionKeyID() {
}
// No unexpired key encryption keys; create one
- ByteBuffer unwrapped = newKey();
- ByteBuffer wrapped = kmsClient.wrapKey(unwrapped, tableKeyId);
+ ByteBuffer unwrapped;
+ ByteBuffer wrapped;
+ if (kekGenerationEnabled && kmsClient.supportsKeyGeneration()) {
+ KeyManagementClient.KeyGenerationResult result =
kmsClient.generateKey(tableKeyId);
+ unwrapped = result.key();
+ wrapped = result.wrappedKey();
+ } else {
Review Comment:
@ggershinsky do we know why is `kmsClient.supportsKeyGeneration()` and hence
kmsClient.generateKey(tableKeyId) already ? is there some historical context
for this ?
##########
core/src/main/java/org/apache/iceberg/encryption/StandardEncryptionManager.java:
##########
@@ -146,8 +157,17 @@ String keyEncryptionKeyID() {
}
// No unexpired key encryption keys; create one
- ByteBuffer unwrapped = newKey();
- ByteBuffer wrapped = kmsClient.wrapKey(unwrapped, tableKeyId);
+ ByteBuffer unwrapped;
Review Comment:
while we are it why not make KEK TTL configurable too ? it might be
orthogonal though
##########
core/src/main/java/org/apache/iceberg/TableProperties.java:
##########
@@ -461,5 +461,9 @@ private TableProperties() {}
public static final String ENCRYPTION_DEK_LENGTH =
"encryption.data-key-length";
public static final int ENCRYPTION_DEK_LENGTH_DEFAULT = 16;
+ public static final String ENCRYPTION_KEK_GENERATION_ENABLED =
+ "encryption.kek-generation-enabled";
+ public static final boolean ENCRYPTION_KEK_GENERATION_ENABLED_DEFAULT =
false;
Review Comment:
can this property be toggle on and off ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
