Skip to content

Commit 791e55f

Browse files
Use s3.remote-signing-enabled instead of an empty s3.signer
1 parent f137852 commit 791e55f

4 files changed

Lines changed: 13 additions & 9 deletions

File tree

‎mkdocs/docs/configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ catalog:
407407
| 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. |
408408
| 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. |
409409
| `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`) |
410+
| 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`) |
410411
| 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. |
411412
| 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). |
412413
| rest-scan-planning.poll-timeout-ms | 300000 | Maximum time to wait for an async scan plan to complete before failing (default: 5 minutes). |

‎pyiceberg/catalog/rest/__init__.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
AWS_REGION,
7474
AWS_SECRET_ACCESS_KEY,
7575
AWS_SESSION_TOKEN,
76+
S3_REMOTE_SIGNING_ENABLED,
7677
S3_SIGNER,
7778
FileIO,
7879
load_file_io,
@@ -646,10 +647,10 @@ def _load_file_io(
646647
table_properties: Properties = EMPTY_DICT,
647648
) -> FileIO:
648649
merged_properties = {**table_properties, **self.properties, **properties}
649-
# An explicitly empty s3.signer in the catalog config opts out of remote signing,
650+
# s3.remote-signing-enabled=false in the catalog config opts out of remote signing,
650651
# even when the server config or vended credentials set a signer.
651-
if self.properties.get(S3_SIGNER) == "":
652-
merged_properties[S3_SIGNER] = ""
652+
if not property_as_bool(self.properties, S3_REMOTE_SIGNING_ENABLED, True):
653+
merged_properties.pop(S3_SIGNER, None)
653654
if self._auth_manager:
654655
merged_properties[AUTH_MANAGER] = self._auth_manager
655656
return load_file_io(merged_properties, location)

‎pyiceberg/io/__init__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ def _is_local_path(path: str) -> bool:
7474
S3_CONNECT_TIMEOUT = "s3.connect-timeout"
7575
S3_REQUEST_TIMEOUT = "s3.request-timeout"
7676
S3_SIGNER = "s3.signer"
77+
S3_REMOTE_SIGNING_ENABLED = "s3.remote-signing-enabled"
7778
S3_SIGNER_URI = "s3.signer.uri"
7879
S3_SIGNER_ENDPOINT = "s3.signer.endpoint"
7980
S3_SIGNER_ENDPOINT_DEFAULT = "v1/aws/s3/sign"

‎tests/catalog/test_rest.py‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3481,7 +3481,7 @@ def test_load_table_catalog_config_outranks_table_properties(
34813481
assert table.io.properties["s3.proxy-uri"] == "http://table-only-proxy"
34823482

34833483

3484-
def test_load_table_empty_catalog_signer_disables_remote_signing(
3484+
def test_load_table_remote_signing_disabled_in_catalog_config(
34853485
rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any]
34863486
) -> None:
34873487
metadata_location = "s3://warehouse/database/table/metadata/00001.metadata.json"
@@ -3495,14 +3495,15 @@ def test_load_table_empty_catalog_signer_disables_remote_signing(
34953495
status_code=200,
34963496
request_headers=TEST_HEADERS,
34973497
)
3498-
catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **{"s3.signer": ""})
3498+
catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **{"s3.remote-signing-enabled": "false"})
34993499
table = catalog.load_table(("fokko", "table"))
35003500

3501-
assert table.io.properties["s3.signer"] == ""
3501+
assert "s3.signer" not in table.io.properties
35023502

35033503

3504-
def test_load_table_server_signer_kept_without_catalog_override(
3505-
rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any]
3504+
@pytest.mark.parametrize("catalog_properties", [{}, {"s3.remote-signing-enabled": "true"}])
3505+
def test_load_table_server_signer_kept_unless_disabled(
3506+
rest_mock: Mocker, example_table_metadata_with_snapshot_v1: dict[str, Any], catalog_properties: dict[str, str]
35063507
) -> None:
35073508
metadata_location = "s3://warehouse/database/table/metadata/00001.metadata.json"
35083509
rest_mock.get(
@@ -3515,7 +3516,7 @@ def test_load_table_server_signer_kept_without_catalog_override(
35153516
status_code=200,
35163517
request_headers=TEST_HEADERS,
35173518
)
3518-
catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN)
3519+
catalog = RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN, **catalog_properties)
35193520
table = catalog.load_table(("fokko", "table"))
35203521

35213522
assert table.io.properties["s3.signer"] == "S3V4RestSigner"

0 commit comments

Comments
 (0)