From f137852806cc84350ec41fffa68ff5bb65d9b0a8 Mon Sep 17 00:00:00 2001 From: Ankit Raj Date: Wed, 21 Oct 2026 17:56:07 +0000 Subject: [PATCH 1/2] REST: Allow disabling remote signing with an empty s3.signer Closes #3888 --- pyiceberg/catalog/rest/__init__.py | 5 ++++ tests/catalog/test_rest.py | 40 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/pyiceberg/catalog/rest/__init__.py b/pyiceberg/catalog/rest/__init__.py index 2b8b4496f8..5015f746d4 100644 --- a/pyiceberg/catalog/rest/__init__.py +++ b/pyiceberg/catalog/rest/__init__.py @@ -73,6 +73,7 @@ AWS_REGION, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, + S3_SIGNER, FileIO, load_file_io, ) @@ -645,6 +646,10 @@ def _load_file_io( table_properties: Properties = EMPTY_DICT, ) -> FileIO: merged_properties = {**table_properties, **self.properties, **properties} + # An explicitly empty s3.signer in the catalog config opts out of remote signing, + # even when the server config or vended credentials set a signer. + if self.properties.get(S3_SIGNER) == "": + merged_properties[S3_SIGNER] = "" if self._auth_manager: merged_properties[AUTH_MANAGER] = self._auth_manager return load_file_io(merged_properties, location) diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py index 4877ee2f73..48e6eae0a4 100644 --- a/tests/catalog/test_rest.py +++ b/tests/catalog/test_rest.py @@ -3481,6 +3481,46 @@ def test_load_table_catalog_config_outranks_table_properties( assert table.io.properties["s3.proxy-uri"] == "http://table-only-proxy" +def test_load_table_empty_catalog_signer_disables_remote_signing( + rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any] +) -> None: + metadata_location = "s3://warehouse/database/table/metadata/00001.metadata.json" + rest_mock.get( + f"{TEST_URI}v1/namespaces/fokko/tables/table", + json={ + "metadata-location": metadata_location, + "metadata": example_table_metadata_with_snapshot_v1, + "config": {"s3.signer": "S3V4RestSigner", "s3.signer.uri": "http://signer"}, + }, + status_code=200, + request_headers=TEST_HEADERS, + ) + catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **{"s3.signer": ""}) + table = catalog.load_table(("fokko", "table")) + + assert table.io.properties["s3.signer"] == "" + + +def test_load_table_server_signer_kept_without_catalog_override( + rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any] +) -> None: + metadata_location = "s3://warehouse/database/table/metadata/00001.metadata.json" + rest_mock.get( + f"{TEST_URI}v1/namespaces/fokko/tables/table", + json={ + "metadata-location": metadata_location, + "metadata": example_table_metadata_with_snapshot_v1, + "config": {"s3.signer": "S3V4RestSigner", "s3.signer.uri": "http://signer"}, + }, + status_code=200, + request_headers=TEST_HEADERS, + ) + catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) + table = catalog.load_table(("fokko", "table")) + + assert table.io.properties["s3.signer"] == "S3V4RestSigner" + + def test_load_credentials_with_longest_prefix(rest_mock: Mocker) -> None: rest_mock.get( f"{TEST_URI}v1/namespaces/fokko/tables/table/credentials", From 791e55f07aa4bab1a66ea8f3c8a7ad228b91e5f5 Mon Sep 17 00:00:00 2001 From: Ankit Raj Date: Thu, 22 Oct 2026 21:42:06 +0000 Subject: [PATCH 2/2] Use s3.remote-signing-enabled instead of an empty s3.signer --- mkdocs/docs/configuration.md | 1 + pyiceberg/catalog/rest/__init__.py | 7 ++++--- pyiceberg/io/__init__.py | 1 + tests/catalog/test_rest.py | 13 +++++++------ 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/mkdocs/docs/configuration.md b/mkdocs/docs/configuration.md index 18f9db973b..d542852d20 100644 --- a/mkdocs/docs/configuration.md +++ b/mkdocs/docs/configuration.md @@ -407,6 +407,7 @@ catalog: | warehouse | myWarehouse | Warehouse location or identifier to request from the catalog service. May be used to determine server-side overrides, such as the warehouse location. | | snapshot-loading-mode | refs | The snapshots to return in the body of the metadata. Setting the value to `all` would return the full set of snapshots currently valid for the table. Setting the value to `refs` would load all snapshots referenced by branches or tags. | | `header.X-Iceberg-Access-Delegation` | `vended-credentials` | Signal to the server that the client supports delegated access via a comma-separated list of access mechanisms. The server may choose to supply access via any or none of the requested mechanisms. When using `vended-credentials`, the server provides temporary credentials to the client. When using `remote-signing`, the server signs requests on behalf of the client. (default: `vended-credentials`) | +| s3.remote-signing-enabled | false | Set to `false` to turn off S3 remote signing for this catalog, even when the server returns `s3.signer` in its config. Useful when the client already has its own S3 credentials. (default: `true`) | | view-endpoints-supported | false | For backwards compatibility with older REST servers. Set to `true` if the server supports view endpoints but doesn't send the `endpoints` field in the ConfigResponse. | | scan-planning-mode | client | When set to `server`, and the catalog advertises the plan-table-scan endpoint, `table.scan()` uses REST server-side scan planning. May be set by the client, returned by the catalog via `GET /v1/config` defaults/overrides, or returned per table in the `config` of the `loadTable` response. The `loadTable` value takes precedence over the catalog-level value, which lets a server request server-side planning only for specific tables. Async plans (`status=submitted`) are polled via `GET .../plan/{plan-id}` until terminal state (completed / failed / cancelled). | | rest-scan-planning.poll-timeout-ms | 300000 | Maximum time to wait for an async scan plan to complete before failing (default: 5 minutes). | diff --git a/pyiceberg/catalog/rest/__init__.py b/pyiceberg/catalog/rest/__init__.py index 5015f746d4..3b91749170 100644 --- a/pyiceberg/catalog/rest/__init__.py +++ b/pyiceberg/catalog/rest/__init__.py @@ -73,6 +73,7 @@ AWS_REGION, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, + S3_REMOTE_SIGNING_ENABLED, S3_SIGNER, FileIO, load_file_io, @@ -646,10 +647,10 @@ def _load_file_io( table_properties: Properties = EMPTY_DICT, ) -> FileIO: merged_properties = {**table_properties, **self.properties, **properties} - # An explicitly empty s3.signer in the catalog config opts out of remote signing, + # s3.remote-signing-enabled=false in the catalog config opts out of remote signing, # even when the server config or vended credentials set a signer. - if self.properties.get(S3_SIGNER) == "": - merged_properties[S3_SIGNER] = "" + if not property_as_bool(self.properties, S3_REMOTE_SIGNING_ENABLED, True): + merged_properties.pop(S3_SIGNER, None) if self._auth_manager: merged_properties[AUTH_MANAGER] = self._auth_manager return load_file_io(merged_properties, location) diff --git a/pyiceberg/io/__init__.py b/pyiceberg/io/__init__.py index 9cae219f7f..fc46215558 100644 --- a/pyiceberg/io/__init__.py +++ b/pyiceberg/io/__init__.py @@ -74,6 +74,7 @@ def _is_local_path(path: str) -> bool: S3_CONNECT_TIMEOUT = "s3.connect-timeout" S3_REQUEST_TIMEOUT = "s3.request-timeout" S3_SIGNER = "s3.signer" +S3_REMOTE_SIGNING_ENABLED = "s3.remote-signing-enabled" S3_SIGNER_URI = "s3.signer.uri" S3_SIGNER_ENDPOINT = "s3.signer.endpoint" S3_SIGNER_ENDPOINT_DEFAULT = "v1/aws/s3/sign" diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py index 48e6eae0a4..3ac0aefe29 100644 --- a/tests/catalog/test_rest.py +++ b/tests/catalog/test_rest.py @@ -3481,7 +3481,7 @@ def test_load_table_catalog_config_outranks_table_properties( assert table.io.properties["s3.proxy-uri"] == "http://table-only-proxy" -def test_load_table_empty_catalog_signer_disables_remote_signing( +def test_load_table_remote_signing_disabled_in_catalog_config( rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any] ) -> None: metadata_location = "s3://warehouse/database/table/metadata/00001.metadata.json" @@ -3495,14 +3495,15 @@ def test_load_table_empty_catalog_signer_disables_remote_signing( status_code=200, request_headers=TEST_HEADERS, ) - catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **{"s3.signer": ""}) + catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **{"s3.remote-signing-enabled": "false"}) table = catalog.load_table(("fokko", "table")) - assert table.io.properties["s3.signer"] == "" + assert "s3.signer" not in table.io.properties -def test_load_table_server_signer_kept_without_catalog_override( - rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any] +@pytest.mark.parametrize("catalog_properties", [{}, {"s3.remote-signing-enabled": "true"}]) +def test_load_table_server_signer_kept_unless_disabled( + rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any], catalog_properties: dict[str, str] ) -> None: metadata_location = "s3://warehouse/database/table/metadata/00001.metadata.json" rest_mock.get( @@ -3515,7 +3516,7 @@ def test_load_table_server_signer_kept_without_catalog_override( status_code=200, request_headers=TEST_HEADERS, ) - catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN) + catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **catalog_properties) table = catalog.load_table(("fokko", "table")) assert table.io.properties["s3.signer"] == "S3V4RestSigner"