gaborkaszab commented on code in PR #17533:
URL: https://github.com/apache/iceberg/pull/17533#discussion_r4036791710
##########
format/spec.md:
##########
@@ -1216,7 +1214,8 @@ Statistics files metadata within `statistics` table
metadata field is a struct w
| _required_ | _required_ | **`statistics-path`** | `string`
| Path of the statistics file. See [Puffin file
format](puffin-spec.md). |
| _required_ | _required_ | **`file-size-in-bytes`** | `long`
| Size of the statistics file. |
| _required_ | _required_ | **`file-footer-size-in-bytes`** | `long`
| Total size of the statistics file's footer (not the footer payload
size). See [Puffin file format](puffin-spec.md) for footer definition. |
- | _optional_ | _optional_ | **`key-metadata`** |
| Base64-encoded implementation-specific key metadata for encryption. |
+ | _optional_ | _optional_ | **`key-metadata`** | `string`
| Base64-encoded implementation-specific key metadata for encryption.
(**Deprecated**: use `key-id` instead) |
Review Comment:
Thanks for looking into this @rdblue and @ggershinsky !
I think the main questions here are 1) reusability of keys and 2) whether
reusability requires any special care in the spec.
1) I did an experiment the other day and for me it seems that each manifest
list file adds a new `EncryptedKey` into the `encryption-keys` list. There is
(are?) additional KEK entry(ies) on top of that in the list. I'm not sure about
custom implementations but for me it seems that there is no reuse of
`EncryptedKey` entries across different manifest lists, apart from the KEK used
to encrypt the `EncryptedKey`s.
Keys for stat files fit into this pattern, they could behave the same way as
keys for manifest lists. Let me know if I miss something here.
2) I think the spec changes in this PR can express both the reused key and
the non-reused key scenario. `key-id` for statistics files is just an ID to be
used for finding the `EncryptedKey` in the `encryption-keys` list that in turn
can be used to decrypt the statistics files (after decrypting the found
`EncryptedFile` itself using the KEK). Whether this is a reused `EncryptedFile`
across files or not, I don't think matters in this context.
While I'm still learning this area of the project, let me know where my
assumptions go wrong :)
--
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]
