Skip to content

Commit 402fa43

Browse files
m0g3rclaude
andcommitted
Register the S3 request signer on the session
S3FileSystem creates the client that issues requests lazily inside the running event loop, so a handler attached to the eagerly constructed fs.s3 is never inherited by it. With signature_version set to UNSIGNED, requests then reach S3 with no Authorization header and are rejected. Registering on the session means every client it creates carries the signer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 3bd4621 commit 402fa43

2 files changed

Lines changed: 60 additions & 7 deletions

File tree

pyiceberg/io/fsspec.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,19 @@ def _s3(properties: Properties) -> AbstractFileSystem:
219219
if profile_name := get_first_property_value(properties, S3_PROFILE_NAME, AWS_PROFILE_NAME):
220220
s3_fs_kwargs["profile"] = profile_name
221221

222-
fs = S3FileSystem(**s3_fs_kwargs)
223-
224-
for event_name, event_function in register_events.items():
225-
fs.s3.meta.events.unregister(event_name, unique_id=1925)
226-
fs.s3.meta.events.register_last(event_name, event_function, unique_id=1925)
227-
228-
return fs
222+
if register_events:
223+
from aiobotocore.session import AioSession
224+
225+
# Register on the session rather than on an already-constructed client. S3FileSystem
226+
# creates the client that issues requests lazily inside the running event loop, and that
227+
# client does not inherit handlers attached to an earlier one.
228+
session = AioSession()
229+
event_emitter = session.get_component("event_emitter")
230+
for event_name, event_function in register_events.items():
231+
event_emitter.register_last(event_name, event_function, unique_id=1925)
232+
s3_fs_kwargs["session"] = session
233+
234+
return S3FileSystem(**s3_fs_kwargs)
229235

230236

231237
def _gs(properties: Properties) -> AbstractFileSystem:

tests/io/test_fsspec.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18+
import asyncio
1819
import os
1920
import pickle
2021
import tempfile
@@ -23,6 +24,7 @@
2324
from unittest import mock
2425

2526
import pytest
27+
from botocore import UNSIGNED
2628
from botocore.awsrequest import AWSRequest
2729
from fsspec.implementations.local import LocalFileSystem
2830
from fsspec.spec import AbstractFileSystem
@@ -877,6 +879,51 @@ def _test_fsspec_pickle_round_trip(fsspec_fileio: FsspecFileIO, location: str) -
877879
TEST_URI = "https://iceberg-test-signer"
878880

879881

882+
def test_s3v4_rest_signer_registered_on_session(requests_mock: Mocker) -> None:
883+
"""Signer must be registered on the session, which every client the filesystem creates inherits."""
884+
new_uri = "https://other-bucket/metadata/snap-8048355899640248710-1-a5c8ea2d-aa1f-48e8-89f4-1fa69db8c742.avro"
885+
requests_mock.post(
886+
f"{TEST_URI}/v1/aws/s3/sign",
887+
json={
888+
"uri": new_uri,
889+
"headers": {"Authorization": ["AWS4-HMAC-SHA256 Credential=ASIAQPRZZYGHUT57DL3I/20221017/us-west-2/s3/aws4_request"]},
890+
"extensions": {},
891+
},
892+
status_code=200,
893+
)
894+
895+
with mock.patch("s3fs.S3FileSystem") as mock_s3fs:
896+
s3_fileio = FsspecFileIO(properties={"s3.signer": "S3V4RestSigner", "uri": TEST_URI, "token": "abc"})
897+
s3_fileio.new_input(location="s3://warehouse/foo")
898+
899+
session = mock_s3fs.call_args.kwargs["session"]
900+
901+
request = AWSRequest(
902+
method="HEAD",
903+
url="https://bucket/metadata/snap-8048355899640248710-1-a5c8ea2d-aa1f-48e8-89f4-1fa69db8c742.avro",
904+
headers={},
905+
data=b"",
906+
params={},
907+
auth_path="/metadata/snap-8048355899640248710-1-a5c8ea2d-aa1f-48e8-89f4-1fa69db8c742.avro",
908+
)
909+
request.context = {"client_region": "us-west-2"}
910+
911+
asyncio.run(
912+
session.get_component("event_emitter").emit(
913+
"before-sign.s3",
914+
request=request,
915+
signing_name="s3",
916+
region_name="us-west-2",
917+
signature_version=UNSIGNED,
918+
request_signer=None,
919+
operation_name="HeadObject",
920+
)
921+
)
922+
923+
assert request.url == new_uri
924+
assert dict(request.headers)["Authorization"].startswith("AWS4-HMAC-SHA256")
925+
926+
880927
def test_s3v4_rest_signer(requests_mock: Mocker) -> None:
881928
new_uri = "https://other-bucket/metadata/snap-8048355899640248710-1-a5c8ea2d-aa1f-48e8-89f4-1fa69db8c742.avro"
882929
requests_mock.post(

0 commit comments

Comments
 (0)