This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-4173-9febaf3596cf90fab40252e902ea4fe8217cdafe in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
commit 1c800eb33bce585c73eb7056fb9acece8550ce6d Author: Alex Stephen <[email protected]> AuthorDate: Sat Nov 7 20:31:50 2026 +0000 Reject encryption properties on pre-v3 tables (#4173) Spec compliance indicates that encryption (and its properties) aren't allowable on pre-v3 tables. --- pyiceberg/table/metadata.py | 23 +++++++++++++++++++++++ pyiceberg/table/update/__init__.py | 3 +++ tests/catalog/test_catalog_behaviors.py | 16 ++++++++++++++++ tests/table/test_init.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+) diff --git a/pyiceberg/table/metadata.py b/pyiceberg/table/metadata.py index 8236f1222..9e983667e 100644 --- a/pyiceberg/table/metadata.py +++ b/pyiceberg/table/metadata.py @@ -636,6 +636,28 @@ class TableMetadataV3(TableMetadataCommonFields, IcebergBaseModel): TableMetadata = Annotated[TableMetadataV1 | TableMetadataV2 | TableMetadataV3, Field(discriminator="format_version")] +def _check_properties_format_version_compatibility(properties: Properties, format_version: int) -> None: + """Check that the table properties are compatible for the given table format version. + + Args: + properties: The table properties to check. + format_version: The Iceberg table format version. + + Raises: + ValueError: If the properties are not compatible for the format version. + """ + from pyiceberg.table import TableProperties + + if format_version >= 3: + return + + encryption_properties = sorted( + {TableProperties.ENCRYPTION_KEY_ID, TableProperties.ENCRYPTION_DATA_KEY_LENGTH} & properties.keys() + ) + if encryption_properties: + raise ValueError(f"Invalid properties for v{format_version}: {encryption_properties}") + + def new_table_metadata( schema: Schema, partition_spec: PartitionSpec, @@ -650,6 +672,7 @@ def new_table_metadata( format_version = int(properties.pop(TableProperties.FORMAT_VERSION, TableProperties.DEFAULT_FORMAT_VERSION)) schema.check_format_version_compatibility(format_version) + _check_properties_format_version_compatibility(properties, format_version) fresh_schema = assign_fresh_schema_ids(schema) fresh_partition_spec = assign_fresh_partition_spec_ids(partition_spec, schema, fresh_schema) diff --git a/pyiceberg/table/update/__init__.py b/pyiceberg/table/update/__init__.py index aa1a09ebb..755e51911 100644 --- a/pyiceberg/table/update/__init__.py +++ b/pyiceberg/table/update/__init__.py @@ -35,6 +35,7 @@ from pyiceberg.table.metadata import ( TableMetadata, TableMetadataUtil, TableMetadataV3, + _check_properties_format_version_compatibility, ) from pyiceberg.table.refs import MAIN_BRANCH, SnapshotRef, SnapshotRefType from pyiceberg.table.snapshots import ( @@ -350,6 +351,8 @@ def _(update: SetPropertiesUpdate, base_metadata: TableMetadata, context: _Table if len(update.updates) == 0: return base_metadata + _check_properties_format_version_compatibility(update.updates, base_metadata.format_version) + properties = dict(base_metadata.properties) properties.update(update.updates) diff --git a/tests/catalog/test_catalog_behaviors.py b/tests/catalog/test_catalog_behaviors.py index 5bd756d74..1ed9ea409 100644 --- a/tests/catalog/test_catalog_behaviors.py +++ b/tests/catalog/test_catalog_behaviors.py @@ -210,6 +210,22 @@ def test_create_v1_table(catalog: Catalog, table_schema_nested: Schema, test_tab catalog.drop_table(test_table_identifier) [email protected]("format_version", [1, 2]) +def test_create_table_rejects_encryption_properties_before_v3( + catalog: Catalog, table_schema_nested: Schema, test_table_identifier: Identifier, format_version: int +) -> None: + catalog.create_namespace(Catalog.namespace_from(test_table_identifier)) + + with pytest.raises(ValueError, match=rf"Invalid properties for v{format_version}: \['encryption.key-id'\]"): + catalog.create_table( + test_table_identifier, + table_schema_nested, + properties={"format-version": str(format_version), "encryption.key-id": "key"}, + ) + + assert not catalog.table_exists(test_table_identifier) + + def test_create_table_custom_sort_order(catalog: Catalog, table_schema_nested: Schema, test_table_identifier: Identifier) -> None: namespace = Catalog.namespace_from(test_table_identifier) catalog.create_namespace(namespace) diff --git a/tests/table/test_init.py b/tests/table/test_init.py index 481326b44..db6dc00e3 100644 --- a/tests/table/test_init.py +++ b/tests/table/test_init.py @@ -1036,6 +1036,38 @@ def test_apply_set_properties_update(table_v2: Table) -> None: assert new_metadata_add_only.last_updated_ms > base_metadata.last_updated_ms [email protected]("table_fixture", ["table_v1", "table_v2"]) [email protected]("property_name", ["encryption.key-id", "encryption.data-key-length"]) +def test_apply_set_properties_update_rejects_encryption_before_v3( + table_fixture: str, property_name: str, request: pytest.FixtureRequest +) -> None: + table: Table = request.getfixturevalue(table_fixture) + + with pytest.raises(ValueError, match=rf"Invalid properties for v{table.format_version}: \['{property_name}'\]"): + update_table_metadata(table.metadata, (SetPropertiesUpdate(updates={property_name: "value"}),)) + + +def test_apply_set_properties_update_allows_encryption_on_v3(table_v3: Table) -> None: + new_metadata = update_table_metadata( + table_v3.metadata, + (SetPropertiesUpdate(updates={"encryption.key-id": "key", "encryption.data-key-length": "32"}),), + ) + + assert new_metadata.properties["encryption.key-id"] == "key" + assert new_metadata.properties["encryption.data-key-length"] == "32" + + +def test_apply_remove_encryption_property_before_v3(table_v2: Table) -> None: + # A pre-v3 table that already has encryption.key-id must be repairable + base_metadata = table_v2.metadata.model_copy( + update={"properties": {**table_v2.metadata.properties, "encryption.key-id": "key"}} + ) + + new_metadata = update_table_metadata(base_metadata, (RemovePropertiesUpdate(removals=["encryption.key-id"]),)) + + assert "encryption.key-id" not in new_metadata.properties + + def test_apply_remove_properties_update(table_v2: Table) -> None: base_metadata = update_table_metadata( table_v2.metadata,
