rdblue commented on PR #9359: URL: https://github.com/apache/iceberg/pull/9359#issuecomment-1874744448
@ggershinsky, I'm still working through this review, so don't feel like you need to address or respond to my comments yet! Also, here are a few notes for myself when I pick this up tomorrow: * How do we ensure that all files that are committed to a table are encrypted? I think we should have a validation in `FastAppend` and `MergingSnapshotProducer` that verifies the encryption key metadata is non-null if the standard encryption manager is used. * There are places like `BaseTaskWriter.openCurrent` that also call `EncryptedOutputFile.encryptingOutputFile()` to get the location. We should look into whether those should use the underlying file path or need to be updated. * I think the way to solve the problem with `StandardEncryptionManager.decrypt` is to make `StandardDecryptedInputFile` implement both `InputFile` (that runs AES GCM decryption) and `EncryptedInputFile` (to provide access to key metadata and the encrypted underlying `InputFile`). Then the read path continues to pass around `InputFile`, but can check whether the file can be used via `EncryptedInputFile` for native decryption. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
