fix(sdk-python): pass S3 region to obstore S3Store (derive from endpoint)#45
fix(sdk-python): pass S3 region to obstore S3Store (derive from endpoint)#45dylan-jung wants to merge 1 commit into
Conversation
…int) ObjectStorage built the obstore S3Store with an endpoint but no region, so obstore signs SigV4 for us-east-1. Against a build-context/volume bucket in any other region that yields HTTP 400 (region mismatch) on every object op — breaking self-hosted deployments outside us-east-1 (e.g. Dockerfile snapshot builds, volume uploads). Derive the region from the endpoint host (s3.<region>.amazonaws.com), falling back to us-east-1 so existing/default behavior is unchanged. Applied to both _async and _sync. Signed-off-by: dylan-jung <dylanjungko@gmail.com>
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="sdk-python/src/daytona/_async/object_storage.py">
<violation number="1" location="sdk-python/src/daytona/_async/object_storage.py:26">
P2: S3 operations through valid dual-stack or FIPS endpoints would still be signed with the wrong region because this regex treats `dualstack` or `fips` as the region label. Since that value is passed directly to `S3Store(region=...)`, endpoints like `s3.dualstack.us-east-2.amazonaws.com` and `s3-fips.us-east-2.amazonaws.com` remain broken even though they include the real region. Consider handling the optional `dualstack`/`fips` labels and adding those cases to the parametrized test.</violation>
</file>
<file name="sdk-python/src/daytona/_sync/object_storage.py">
<violation number="1" location="sdk-python/src/daytona/_sync/object_storage.py:24">
P2: S3 operations through valid dual-stack or FIPS endpoints would still be signed with the wrong region because this regex treats `dualstack` or `fips` as the region label. Since that value is passed directly to `S3Store(region=...)`, endpoints like `s3.dualstack.us-east-2.amazonaws.com` and `s3-fips.us-east-2.amazonaws.com` remain broken even though they include the real region. Consider handling the optional `dualstack`/`fips` labels and adding those cases to the parametrized test.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
|
||
| obstore defaults SigV4 signing to us-east-1, so buckets in other regions 400 without this. | ||
| """ | ||
| match = re.search(r"s3[.-]([^.]+)\.amazonaws\.com", endpoint_url or "") |
There was a problem hiding this comment.
P2: S3 operations through valid dual-stack or FIPS endpoints would still be signed with the wrong region because this regex treats dualstack or fips as the region label. Since that value is passed directly to S3Store(region=...), endpoints like s3.dualstack.us-east-2.amazonaws.com and s3-fips.us-east-2.amazonaws.com remain broken even though they include the real region. Consider handling the optional dualstack/fips labels and adding those cases to the parametrized test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At sdk-python/src/daytona/_async/object_storage.py, line 26:
<comment>S3 operations through valid dual-stack or FIPS endpoints would still be signed with the wrong region because this regex treats `dualstack` or `fips` as the region label. Since that value is passed directly to `S3Store(region=...)`, endpoints like `s3.dualstack.us-east-2.amazonaws.com` and `s3-fips.us-east-2.amazonaws.com` remain broken even though they include the real region. Consider handling the optional `dualstack`/`fips` labels and adding those cases to the parametrized test.</comment>
<file context>
@@ -17,6 +18,15 @@
+
+ obstore defaults SigV4 signing to us-east-1, so buckets in other regions 400 without this.
+ """
+ match = re.search(r"s3[.-]([^.]+)\.amazonaws\.com", endpoint_url or "")
+ return match.group(1) if match else "us-east-1"
+
</file context>
| match = re.search(r"s3[.-]([^.]+)\.amazonaws\.com", endpoint_url or "") | |
| match = re.search( | |
| r"(?:^|://)(?:[^/@]+\.)?s3(?:-fips)?(?:\.dualstack)?[.-]([a-z0-9-]+)\.amazonaws\.com(?:\.cn)?(?::\d+)?(?:/|$)", | |
| endpoint_url or "", | |
| ) |
|
|
||
| obstore defaults SigV4 signing to us-east-1, so buckets in other regions 400 without this. | ||
| """ | ||
| match = re.search(r"s3[.-]([^.]+)\.amazonaws\.com", endpoint_url or "") |
There was a problem hiding this comment.
P2: S3 operations through valid dual-stack or FIPS endpoints would still be signed with the wrong region because this regex treats dualstack or fips as the region label. Since that value is passed directly to S3Store(region=...), endpoints like s3.dualstack.us-east-2.amazonaws.com and s3-fips.us-east-2.amazonaws.com remain broken even though they include the real region. Consider handling the optional dualstack/fips labels and adding those cases to the parametrized test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At sdk-python/src/daytona/_sync/object_storage.py, line 24:
<comment>S3 operations through valid dual-stack or FIPS endpoints would still be signed with the wrong region because this regex treats `dualstack` or `fips` as the region label. Since that value is passed directly to `S3Store(region=...)`, endpoints like `s3.dualstack.us-east-2.amazonaws.com` and `s3-fips.us-east-2.amazonaws.com` remain broken even though they include the real region. Consider handling the optional `dualstack`/`fips` labels and adding those cases to the parametrized test.</comment>
<file context>
@@ -15,6 +16,15 @@
+
+ obstore defaults SigV4 signing to us-east-1, so buckets in other regions 400 without this.
+ """
+ match = re.search(r"s3[.-]([^.]+)\.amazonaws\.com", endpoint_url or "")
+ return match.group(1) if match else "us-east-1"
+
</file context>
| match = re.search(r"s3[.-]([^.]+)\.amazonaws\.com", endpoint_url or "") | |
| match = re.search( | |
| r"(?:^|://)(?:[^/@]+\.)?s3(?:-fips)?(?:\.dualstack)?[.-]([a-z0-9-]+)\.amazonaws\.com(?:\.cn)?(?::\d+)?(?:/|$)", | |
| endpoint_url or "", | |
| ) |
Summary
ObjectStorage/AsyncObjectStoragebuilt the obstoreS3Storewith an endpoint but noregion, so obstore signs SigV4 for
us-east-1. Against a build-context/volume bucket inany other region this returns HTTP 400 on every object op — breaking self-hosted
deployments outside us-east-1 (Dockerfile snapshot builds, volume uploads).
Fixes #44
Change
s3.<region>.amazonaws.com) and pass it toS3Store.us-east-1when the host is non-standard (minio, custom) → no behaviorchange for existing/default users.
_asyncand_sync.Tests
test_region_from_endpoint(parametrized: regional, legacy dash-style, GovCloud,no-region host, non-AWS, empty) to both sync and async suites.
Checklist
black/isortclean (line-length 120)Summary by cubic
Derives the S3 region from the endpoint and passes it to
obstoreS3Store, fixing SigV4 region mismatches that returned HTTP 400. Restores object operations for buckets outside us-east-1 in self-hosted setups.Written for commit 3ce4841. Summary will update on new commits.