Repository navigation
REST: Allow disabling remote signing with an empty s3.signer - #4134
Open
Ankitraj-builds wants to merge 1 commit into
Open
Ankitraj-builds wants to merge 1 commit into
Ankitraj-builds wants to merge 1 commit into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3888
Rationale for this change
REST catalogs like Lakekeeper always return remote signing settings (
s3.signer,s3.signer.uri) in the table response config. Since #3956 the FileIO property order is table metadata < catalog config < server config < vended credentials, so the server's signer always wins. A client that has direct S3 credentials cannot turn remote signing off, and every S3 request pays an extra round trip to the signer.This change lets the user opt out explicitly: if the catalog config sets
s3.signerto an empty string, that value is applied after the server config inRestCatalog._load_file_io(). Only an explicit empty value does this. Ifs3.signeris not set in the catalog config, or set to a real signer, the current order is unchanged.Because the check is in
_load_file_io(), it coversTable,StagedTableand scan planning FileIOs.Are these changes tested?
Yes. Added two tests in
tests/catalog/test_rest.py:test_load_table_empty_catalog_signer_disables_remote_signing: the server returnss3.signer: S3V4RestSigner, the catalog setss3.signerto"", and the table FileIO gets"". This test fails onmain.test_load_table_server_signer_kept_without_catalog_override: same server response with no catalog override, and the server signer is kept.All 176 tests in
tests/catalog/test_rest.pypass locally, and lint passes.Are there any user-facing changes?
Yes. Setting
s3.signerto an empty string in the REST catalog config now disables remote signing, even when the server config sets a signer. Nothing changes for users who do not set it.