xanderbailey commented on code in PR #2701:
URL: https://github.com/apache/iceberg-rust/pull/2701#discussion_r3596020405
##########
crates/iceberg/src/encryption/key_metadata.rs:
##########
@@ -101,6 +104,22 @@ impl StandardKeyMetadata {
}
}
+/// AAD prefix length in bytes.
+const AAD_PREFIX_LENGTH: usize = 16;
+
+/// Generate a [`StandardKeyMetadata`] with a fresh random DEK and AAD prefix,
+/// sized to `key_size`.
+pub(crate) fn generate_standard_key_metadata(key_size: AesKeySize) ->
StandardKeyMetadata {
+ let dek = SecureKey::generate(key_size);
+
StandardKeyMetadata::new(dek.as_bytes()).with_aad_prefix(&generate_aad_prefix())
Review Comment:
FYI: This will break after https://github.com/apache/iceberg-rust/pull/2834
merges.
##########
crates/iceberg/src/writer/file_writer/parquet_writer.rs:
##########
@@ -507,6 +538,34 @@ impl ParquetWriter {
}
}
+fn resolve_writer_properties(
+ writer_properties: &WriterProperties,
Review Comment:
I would error if these `WriterProperties` already have encryption properties
set.
##########
crates/iceberg/src/writer/file_writer/parquet_writer.rs:
##########
Review Comment:
I would add a comment in the doc here that encrypted writing is not
supported with this constructor. I would honestly be in favour of removing this
constructor since it's not used in this crate and nor should it be. I don't
think we should do that in this PR but it would be a good follow-up IMO.
--
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]