kevinjqliu commented on code in PR #3930:
URL: https://github.com/apache/iceberg-python/pull/3930#discussion_r3991323954
##########
pyiceberg/table/metadata.py:
##########
@@ -125,6 +126,33 @@ def construct_refs(table_metadata: TableMetadata) ->
TableMetadata:
return table_metadata
+class EncryptedKey(IcebergBaseModel):
+ """A key used for table encryption, tracked in v3 metadata under
`encryption-keys`.
+
+ https://iceberg.apache.org/spec/#encryption-keys
+ """
+
+ key_id: str = Field(alias="key-id")
+ """ID of the encryption key."""
+
+ encrypted_key_metadata: bytes = Field(alias="encrypted-key-metadata")
Review Comment:
curious if theres a reason this is `bytes` here instead of `string`.
https://iceberg.apache.org/spec/#encryption-keys mentions its string type
is this just an optimization for pydantic?
##########
pyiceberg/table/metadata.py:
##########
@@ -125,6 +126,33 @@ def construct_refs(table_metadata: TableMetadata) ->
TableMetadata:
return table_metadata
+class EncryptedKey(IcebergBaseModel):
+ """A key used for table encryption, tracked in v3 metadata under
`encryption-keys`.
+
+ https://iceberg.apache.org/spec/#encryption-keys
+ """
+
+ key_id: str = Field(alias="key-id")
+ """ID of the encryption key."""
+
+ encrypted_key_metadata: bytes = Field(alias="encrypted-key-metadata")
+ """The encrypted key and metadata, base64 encoded in JSON."""
+
+ encrypted_by_id: str | None = Field(alias="encrypted-by-id", default=None)
+ """ID of the key used to encrypt or wrap `encrypted-key-metadata`."""
+
+ properties: dict[str, str] = Field(default_factory=dict)
+ """Additional metadata used by the table's encryption scheme."""
+
+ @field_validator("encrypted_key_metadata", mode="before")
+ def decode_encrypted_key_metadata(cls, encrypted_key_metadata: Any) -> Any:
+ return base64.b64decode(encrypted_key_metadata) if
isinstance(encrypted_key_metadata, str) else encrypted_key_metadata
Review Comment:
`base64.b64decode` without `validate=True` discards invalid characters,
potentially turning malformed encryption metadata into altered or empty bytes.
Use `base64.b64decode(value, validate=True)` and add malformed-input tests.
##########
pyiceberg/table/metadata.py:
##########
@@ -584,6 +612,9 @@ def construct_refs(self) -> TableMetadata:
next_row_id: int | None = Field(alias="next-row-id", default=None)
"""A long higher than all assigned row IDs; the next snapshot's
`first-row-id`."""
+ encryption_keys: list[EncryptedKey] = Field(alias="encryption-keys",
default_factory=list)
Review Comment:
nit: can we add a test that `encryption_keys` is not serialized when not set?
Something like:
```
def
test_v3_metadata_without_encryption_keys_omits_it(example_table_metadata_v3:
dict[str, Any]) -> None:
table_metadata = TableMetadataUtil.parse_obj(example_table_metadata_v3)
assert table_metadata.encryption_keys == []
assert "encryption-keys" not in table_metadata.model_dump(mode="json")
```
--
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]
