blackmwk commented on code in PR #2183:
URL: https://github.com/apache/iceberg-rust/pull/2183#discussion_r3135006871


##########
docs/rfcs/0003_table_encryption.md:
##########
@@ -410,56 +455,67 @@ block to its position in the stream to prevent reordering 
attacks.
 random-access reads. **`AesGcmFileWrite`** implements `FileWrite` for 
transparent AGS1
 encryption with block buffering.
 
-### InputFile and OutputFile Enums
+### EncryptedInputFile and EncryptedOutputFile Wrappers
 
-`InputFile` and `OutputFile` are enums with three variants each:
+Rather than adding encryption variants to `InputFile`/`OutputFile` (which 
would change the
+public API of those core types), encryption uses dedicated wrapper types. 
`EncryptedInputFile`
+and `EncryptedOutputFile` wrap a plain `InputFile`/`OutputFile` and add 
transparent
+encryption/decryption. The plain `InputFile`/`OutputFile` types remain 
unchanged.
 
 ```rust
-pub enum InputFile {
-    /// Standard unencrypted file.
-    Plain { storage: Arc<dyn Storage>, path: String },
-
-    /// AGS1 stream-encrypted file. Transparent decryption on read.
-    Encrypted { storage: Arc<dyn Storage>, path: String, decryptor: 
Arc<FileDecryptor> },
-
-    /// Parquet Modular Encryption. Raw reads; Parquet reader handles 
decryption.
-    NativeEncrypted { storage: Arc<dyn Storage>, path: String, key_material: 
NativeKeyMaterial },
+/// Wraps a plain InputFile with decryption capabilities.
+pub enum EncryptedInputFile {
+    /// AGS1 stream-encrypted file. Decrypted transparently on read.
+    Encrypted {
+        inner: InputFile,
+        decryptor: Arc<AesGcmFileDecryptor>,
+    },
+    /// Parquet Modular Encryption. The Parquet reader handles decryption.
+    NativeEncrypted {
+        inner: InputFile,
+        key_material: NativeKeyMaterial,
+    },
 }
 
-pub enum OutputFile {
-    Plain { storage: Arc<dyn Storage>, path: String },
-    Encrypted { storage: Arc<dyn Storage>, path: String, encryptor: 
Arc<FileEncryptor> },
-    NativeEncrypted { storage: Arc<dyn Storage>, path: String, key_material: 
NativeKeyMaterial },
+/// Wraps a plain OutputFile with encryption capabilities.
+pub enum EncryptedOutputFile {
+    /// AGS1 stream-encrypted output. Encrypted transparently on write.
+    Encrypted {
+        inner: OutputFile,
+        key_metadata: Box<[u8]>,
+        encryptor: Arc<AesGcmFileEncryptor>,
+    },
+    /// Parquet Modular Encryption. The Parquet writer handles encryption.
+    NativeEncrypted {
+        inner: OutputFile,
+        key_metadata: Box<[u8]>,
+        key_material: NativeKeyMaterial,
+    },
 }
 ```
 
+Both wrappers delegate standard operations (`location()`, `exists()`, 
`read()`, `reader()`,
+`write()`, `writer()`) to the inner file, with `Encrypted` variants 
transparently
+encrypting/decrypting via `AesGcmFileRead`/`AesGcmFileWrite`. `into_inner()` 
recovers
+the underlying plain file.
+
 `NativeKeyMaterial` carries the plaintext DEK and AAD prefix for Parquet's
 `FileEncryptionProperties` / `FileDecryptionProperties`.
 
-Common operations (`location()`, `exists()`, `read()`, `reader()`, `write()`, 
`writer()`)
-delegate to the appropriate variant, with `Encrypted` variants transparently 
encrypting/decrypting
-via `AesGcmFileRead`/`AesGcmFileWrite`.
-
-#### Adaptation for Storage Trait RFC
-
-Once RFC 0002 merges, `InputFile` will hold `Arc<dyn Storage>` instead of 
`Operator`. This is
-already the case in this implementation — the enum structure is stable. Only 
the underlying
-`Storage` trait implementation may change.
+This wrapper approach means `ManifestReader` and `ManifestListWriter` accept 
the encrypted
+wrapper types (or `Box<dyn FileWrite>`) where encryption is needed, rather 
than requiring
+changes to the `InputFile`/`OutputFile` enums themselves.
 
 ### FileIO Integration
 
-The `EncryptionManager` is stored as a type-safe `FileIOBuilder` extension. 
This integrates
-naturally with catalogs that support extensions (e.g. 
`RestCatalog.with_file_io_extension()`):
+The `EncryptionManager` is attached to `FileIO` via a convenience method. The 
encryption

Review Comment:
   Let's make it explicit first, and polish the api later. As long as we be 
careful with pub visibility, we could reduce user impact as much as possible.



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