Skip to content

Commit f877269

Browse files
ls unit test
1 parent 82629a6 commit f877269

File tree

4 files changed

+31
-10
lines changed

4 files changed

+31
-10
lines changed

.secrets.baseline

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,9 @@
205205
"filename": "tests/test_s3_endpoint.py",
206206
"hashed_secret": "08d2e98e6754af941484848930ccbaddfefe13d6",
207207
"is_verified": false,
208-
"line_number": 75
208+
"line_number": 77
209209
}
210210
]
211211
},
212-
"generated_at": "2026-02-25T19:02:33Z"
212+
"generated_at": "2026-02-27T22:12:14Z"
213213
}

gen3workflow/routes/s3.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,14 @@ async def s3_endpoint(path: str, request: Request):
166166
not support S3 endpoints with a path, such as the Minio-go S3 client.
167167
"""
168168

169-
# because this endpoint is exposed at root, if the GET path is empty, assume the user is not
170-
# trying to reach the S3 endpoint and redirect to the status endpoint instead
171-
if request.method == "GET" and not path:
172-
return await get_status(request)
169+
# Because this endpoint is exposed at root, if the GET path is empty, the user may not be
170+
# trying to reach the S3 endpoint: suggest using the status endpoint.
171+
# "All buckets" listing requests also land here and are not supported, since users can only
172+
# access their own bucket.
173+
if request.method == "GET" and path in ("", "s3"):
174+
err_msg = f"If you are using the S3 endpoint: 's3 ls' not supported, use 's3 ls s3://<your bucket>' instead. If you are trying to reach the Gen3-Workflow API, try '/_status'."
175+
logger.error(err_msg)
176+
raise HTTPException(HTTP_400_BAD_REQUEST, err_msg)
173177

174178
# extract the caller's access token from the request headers, and ensure the caller (user, or
175179
# client acting on behalf of the user) has access to run workflows

tests/test_s3_endpoint.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
import re
2+
import tempfile
3+
from unittest.mock import AsyncMock, patch
4+
15
import boto3
26
from botocore.config import Config
37
from botocore.exceptions import ClientError
48
from fastapi import HTTPException
5-
from unittest.mock import AsyncMock, patch
69
import pytest
7-
import tempfile
810

911
from conftest import MOCKED_S3_RESPONSE_DICT, TEST_USER_ID, TEST_USER_TOKEN
1012
from gen3workflow.config import config
@@ -207,6 +209,21 @@ def test_s3_endpoint_wrong_bucket(s3_client, access_token_patcher, bucket_name):
207209
s3_client.list_objects(Bucket=bucket_name)
208210

209211

212+
@pytest.mark.parametrize("client", [{"get_url": True}], indirect=True)
213+
def test_s3_endpoint_list_buckets(s3_client):
214+
"""
215+
Listing all the buckets the user can access is not supported, since users can only access
216+
their own bucket.
217+
"""
218+
with pytest.raises(
219+
ClientError,
220+
match=re.escape(
221+
"An error occurred (400) when calling the ListBuckets operation: Bad Request"
222+
),
223+
):
224+
s3_client.list_buckets()
225+
226+
210227
@pytest.mark.asyncio
211228
@pytest.mark.parametrize("path", ["s3", ""], ids=["s3 path", "root path"])
212229
async def test_s3_endpoint_with_bearer_token(client, path):
@@ -331,7 +348,7 @@ async def test_set_access_token_and_get_user_id_invalid_auth():
331348
def test_s3_upload_file(s3_client, access_token_patcher, multipart):
332349
"""
333350
Test that the boto3 `upload_file` function works with the `/s3` endpoint, both for a small
334-
file uploaded in 1 chunk and for a large file uploaded in multiple chunks.
351+
file uploaded in 1 part and for a large file uploaded in multiple parts.
335352
"""
336353
with patch(
337354
"gen3workflow.aws_utils.get_existing_kms_key_for_bucket",

tests/test_system.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33

44
@pytest.mark.asyncio
5-
@pytest.mark.parametrize("endpoint", ["/", "/_status", "/_status/"])
5+
@pytest.mark.parametrize("endpoint", ["/_status", "/_status/"])
66
@pytest.mark.parametrize(
77
"client",
88
[

0 commit comments

Comments
 (0)