abhishekmauryaKsolves commented on code in PR #18337:
URL: https://github.com/apache/iceberg/pull/18337#discussion_r4178601093


##########
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:
   Yes, it can be toggled. From reading the code, the property only affects how 
a new KEK is created. Existing KEKs are always unwrapped via 
kmsClient.unwrapKey(), whether they were created with wrapKey() or 
generateKey(), so flipping the property does not affect reading existing keys.
   
   Separately, while checking this I noticed HiveTableOperations builds the 
encryption properties by hand (key-id and data-key-length, both taken from 
HMS), so the new property is currently not passed to createEncryptionManager on 
the Hive path. Would you prefer it to be read from the table metadata 
properties there, or stored as an HMS parameter like the DEK length? Happy to 
follow whichever fits the tamper-protection design.



-- 
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]

Reply via email to