diff --git a/.github/workflows/helm-install-test.yml b/.github/workflows/helm-install-test.yml index d0571d6..4c398a4 100644 --- a/.github/workflows/helm-install-test.yml +++ b/.github/workflows/helm-install-test.yml @@ -49,7 +49,10 @@ jobs: spec: containers: - name: minio - image: minio/minio:latest + image: bitnamilegacy/minio:2025.7.23-debian-12-r5 + command: ["minio"] + securityContext: + runAsUser: 0 args: ["server", "/data"] env: - name: MINIO_ROOT_USER diff --git a/e2e/docker-compose.yml b/e2e/docker-compose.yml index 229c9f1..e17b687 100644 --- a/e2e/docker-compose.yml +++ b/e2e/docker-compose.yml @@ -370,7 +370,10 @@ services: spec: containers: - name: minio - image: minio/minio:latest + image: bitnamilegacy/minio:2025.7.23-debian-12-r5 + command: ["minio"] + securityContext: + runAsUser: 0 args: ["server", "/data", "--console-address", ":9001"] env: - name: MINIO_ROOT_USER diff --git a/s3proxy/metrics.py b/s3proxy/metrics.py index fd03057..54c0ef0 100644 --- a/s3proxy/metrics.py +++ b/s3proxy/metrics.py @@ -2,6 +2,8 @@ from __future__ import annotations +from urllib.parse import parse_qs + from prometheus_client import Counter, Gauge, Histogram # Request metrics @@ -69,6 +71,7 @@ def get_operation_name(method: str, path: str, query: str) -> str: Returns: S3 operation name for metrics labeling. """ + keys = parse_qs(query, keep_blank_values=True) is_bucket_only = "/" not in path.strip("/") and bool(path.strip("/")) is_root = path.strip("/") == "" @@ -77,15 +80,15 @@ def get_operation_name(method: str, path: str, query: str) -> str: return "ListBuckets" # Batch delete - if "delete" in query and method == "POST": + if "delete" in keys and method == "POST": return "DeleteObjects" # Multipart operations - if "uploadId" in query: + if "uploadId" in keys: if method == "GET" and "partNumber" not in query: return "ListParts" if method == "PUT": - if "x-amz-copy-source" in query: + if "x-amz-copy-source" in keys: return "UploadPartCopy" return "UploadPart" if method == "POST": @@ -94,7 +97,7 @@ def get_operation_name(method: str, path: str, query: str) -> str: return "AbortMultipartUpload" # List/Create multipart uploads - if "uploads" in query: + if "uploads" in keys: if method == "GET": return "ListMultipartUploads" if method == "POST": @@ -102,7 +105,7 @@ def get_operation_name(method: str, path: str, query: str) -> str: # Bucket operations if is_bucket_only: - if "location" in query and method == "GET": + if "location" in keys and method == "GET": return "GetBucketLocation" if method == "PUT": return "CreateBucket" @@ -114,7 +117,7 @@ def get_operation_name(method: str, path: str, query: str) -> str: return "ListObjects" # Object tagging - if "tagging" in query: + if "tagging" in keys: if method == "GET": return "GetObjectTagging" if method == "PUT": diff --git a/s3proxy/routing/dispatcher.py b/s3proxy/routing/dispatcher.py index f05613a..fe1ddbe 100644 --- a/s3proxy/routing/dispatcher.py +++ b/s3proxy/routing/dispatcher.py @@ -74,7 +74,7 @@ async def dispatch(self, request: Request, creds: S3Credentials) -> PlainTextRes Response from the appropriate handler method. """ method = request.method - query = str(request.url.query) + query = parse_qs(str(request.url.query), keep_blank_values=True) path = request.url.path headers = {k.lower(): v for k, v in request.headers.items()} @@ -106,8 +106,7 @@ async def dispatch(self, request: Request, creds: S3Credentials) -> PlainTextRes # Bucket listing (fallthrough from bucket operations) if self._is_bucket_only_path(path) and method == METHOD_GET: - query_params = parse_qs(query, keep_blank_values=True) - list_type = query_params.get("list-type", ["1"])[0] + list_type = query.get("list-type", ["1"])[0] if list_type == "2": return await self.handler.handle_list_objects(request, creds) return await self.handler.handle_list_objects_v1(request, creds) @@ -124,7 +123,7 @@ async def _dispatch_multipart( request: Request, creds: S3Credentials, method: str, - query: str, + query: dict[str, list[str]], headers: dict[str, str], ) -> PlainTextResponse: """Handle multipart upload operations.""" @@ -145,7 +144,7 @@ async def _dispatch_bucket( request: Request, creds: S3Credentials, method: str, - query: str, + query: dict[str, list[str]], ) -> PlainTextResponse | None: """Handle bucket-level operations. @@ -169,9 +168,7 @@ async def _dispatch_bucket( # list-type=2), not a bucket sub-resource. Fall through to the list # handler, which serves it via the backend's V2 API — raw-forwarding it # sends V1 to the backend, which Hetzner & co. reject with HTTP 400. - if method == METHOD_GET and set(parse_qs(query, keep_blank_values=True)) <= ( - LIST_QUERY_KEYS - ): + if method == METHOD_GET and set(query) <= LIST_QUERY_KEYS: return None return await self.handler.forward_request(request, creds) @@ -190,7 +187,7 @@ async def _dispatch_object( request: Request, creds: S3Credentials, method: str, - query: str, + query: dict[str, list[str]], ) -> PlainTextResponse: """Handle standard object operations.""" if QUERY_TAGGING in query: diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index 7412f8c..5ab7fc8 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -6,7 +6,9 @@ services: - "6379:6379" minio: - image: minio/minio + image: bitnamilegacy/minio:2025.7.23-debian-12-r5 + user: root + entrypoint: ["minio"] container_name: s3proxy-test-minio ports: - "9000:9000" diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index a51fadc..1a0ec33 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -80,7 +80,11 @@ def minio_backend(*, isolated: bool = False) -> Generator[str]: "MINIO_ROOT_USER=minioadmin", "-e", "MINIO_ROOT_PASSWORD=minioadmin", - "minio/minio", + "--user", + "0", + "--entrypoint", + "minio", + "bitnamilegacy/minio:2025.7.23-debian-12-r5", "server", "/data", ], diff --git a/tests/unit/test_dispatch_v1_list.py b/tests/unit/test_dispatch_v1_list.py index ecfc44f..75190d2 100644 --- a/tests/unit/test_dispatch_v1_list.py +++ b/tests/unit/test_dispatch_v1_list.py @@ -73,9 +73,18 @@ def test_subresource_get_still_forwards(): assert _route("GET", "/bkt", "location") == "location" +def test_prefix_containing_subresource_name_is_still_a_list(): + # scylla-manager lists prefixes like .../table/geocoded_locations/ + for word in ("location", "versioning", "uploads", "delete", "tagging"): + q = f"list-type=2&prefix=keyspace%2Fmain%2Ftable%2Fgeocoded_{word}s%2F" + assert _route("GET", "/bkt", q) == "v2", word + assert _route("GET", "/bkt", f"prefix=x_{word}%2F&delimiter=%2F") == "v1", word + + if __name__ == "__main__": test_v1_list_with_params_routes_to_v1_handler() test_bare_bucket_get_is_v1_list() test_v2_list_still_routes_to_v2() test_subresource_get_still_forwards() + test_prefix_containing_subresource_name_is_still_a_list() print("ok")