From b8376e48afc624725b048fcec2bc144723152efa Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre <4224001+paulineribeyre@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:10:16 -0500 Subject: [PATCH 1/4] debug logs --- gen3workflow/routes/s3.py | 18 +++++++++++------- gen3workflow/routes/storage.py | 2 +- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/gen3workflow/routes/s3.py b/gen3workflow/routes/s3.py index 769bcca..4187361 100644 --- a/gen3workflow/routes/s3.py +++ b/gen3workflow/routes/s3.py @@ -445,16 +445,20 @@ async def s3_endpoint(path: str, request: Request): logger.error( f"Error from S3: {response.status_code} {response.text}" ) - # do not retry in the case of a 403 error: authentication is done internally by - # this function, so 403 errors are internal service errors - if response.status_code != HTTP_403_FORBIDDEN: + # in the case of a client-side (4xx) error (except `408 Request Timeout` and + # `429 Too Many Requests`), print debug logs and do not retry + if ( + response.status_code >= 400 + and response.status_code < 500 + and response.status_code not in [408, 429] + ): proceed = False - # SignatureDoesNotMatch errors are a sign of a bug in this code => debug logs - if "SignatureDoesNotMatch" in response.text: logger.debug(f"Incoming headers:\n{in_headers}") logger.debug(f"Outgoing headers:\n{out_headers}") logger.debug(f"Canonical request:\n{canonical_request}") - logger.debug(f"String to sign:\n{string_to_sign}") + logger.debug(f"Incoming query params:\n{request.query_params}") + logger.debug(f"Outgoing query params:\n{query_params}") + logger.debug(f"Outgoing body:\n{body}") else: logger.debug(f"Error from S3: {response.status_code}") except Exception as e: @@ -484,7 +488,7 @@ async def s3_endpoint(path: str, request: Request): # Return the response from AWS S3. # - mask the details of 403 errors from the end user: authentication is done internally by this - # function, so 403 errors are internal service errors + # function, so 403 errors are internal service errors. # - return all the headers from the AWS response, except: # - `x-amz-bucket-region` which for some reason causes this error for tasks ran through # Nextflow: `The AWS Access Key Id you provided does not exist in our records`. diff --git a/gen3workflow/routes/storage.py b/gen3workflow/routes/storage.py index 4ede884..20cfd02 100644 --- a/gen3workflow/routes/storage.py +++ b/gen3workflow/routes/storage.py @@ -59,7 +59,7 @@ async def storage_setup( # Create S3 Files Filesystem ID if not exists fs_id = s3_files.setup_s3_filesystem(bucket_name) # NOTE: To avoid blocking `/storage/setup` call, setting s3 filesystem just returns - # the filesystem id and continue with the rest of the steps asynchronously + # the filesystem id and continues with the rest of the steps asynchronously background_tasks.add_task(s3_files.provision_mount_targets, fs_id) storage_info["s3files_filesystem_id"] = fs_id From 9afe51c5935c8534ff38003841ba0b5dd9d58adc Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre <4224001+paulineribeyre@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:56:33 -0500 Subject: [PATCH 2/4] update deps --- gen3workflow/routes/s3.py | 11 ++++++++--- poetry.lock | 36 ++++++++++++++++++------------------ 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/gen3workflow/routes/s3.py b/gen3workflow/routes/s3.py index 4187361..860f658 100644 --- a/gen3workflow/routes/s3.py +++ b/gen3workflow/routes/s3.py @@ -18,6 +18,9 @@ HTTP_401_UNAUTHORIZED, HTTP_403_FORBIDDEN, HTTP_404_NOT_FOUND, + HTTP_408_REQUEST_TIMEOUT, + HTTP_429_TOO_MANY_REQUESTS, + HTTP_500_INTERNAL_SERVER_ERROR, ) from gen3workflow import logger @@ -448,14 +451,16 @@ async def s3_endpoint(path: str, request: Request): # in the case of a client-side (4xx) error (except `408 Request Timeout` and # `429 Too Many Requests`), print debug logs and do not retry if ( - response.status_code >= 400 - and response.status_code < 500 - and response.status_code not in [408, 429] + response.status_code >= HTTP_400_BAD_REQUEST + and response.status_code < HTTP_500_INTERNAL_SERVER_ERROR + and response.status_code + not in [HTTP_408_REQUEST_TIMEOUT, HTTP_429_TOO_MANY_REQUESTS] ): proceed = False logger.debug(f"Incoming headers:\n{in_headers}") logger.debug(f"Outgoing headers:\n{out_headers}") logger.debug(f"Canonical request:\n{canonical_request}") + logger.debug(f"String to sign:\n{string_to_sign}") logger.debug(f"Incoming query params:\n{request.query_params}") logger.debug(f"Outgoing query params:\n{query_params}") logger.debug(f"Outgoing body:\n{body}") diff --git a/poetry.lock b/poetry.lock index 30fe0d8..0da8637 100644 --- a/poetry.lock +++ b/poetry.lock @@ -125,18 +125,18 @@ files = [ [[package]] name = "boto3" -version = "1.43.65" +version = "1.43.67" description = "The AWS SDK for Python" optional = false python-versions = ">=3.10" groups = ["main", "dev"] files = [ - {file = "boto3-1.43.65-py3-none-any.whl", hash = "sha256:a8217fb68cae3f8a7575eef395383b68b846d92d29b0e8f6e948e6e9e08dcc3f"}, - {file = "boto3-1.43.65.tar.gz", hash = "sha256:f2331154aee1ae97ece48077d77f41d3bd5ea39eb4e3037030448b58695a3a79"}, + {file = "boto3-1.43.67-py3-none-any.whl", hash = "sha256:082cf9df068168cb44028a1703822374c1eb7e48fa49470d7e8a76f0c977d0bc"}, + {file = "boto3-1.43.67.tar.gz", hash = "sha256:75fe983b70d39cfdc274dc51f9bb02b8a0a104bdad4fa073c1af85a35e707c91"}, ] [package.dependencies] -botocore = ">=1.43.65,<1.44.0" +botocore = ">=1.43.67,<1.44.0" jmespath = ">=0.7.1,<2.0.0" s3transfer = ">=0.19.0,<0.20.0" @@ -145,14 +145,14 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.43.65" +version = "1.43.67" description = "Low-level, data-driven core of boto 3." optional = false python-versions = ">=3.10" groups = ["main", "dev"] files = [ - {file = "botocore-1.43.65-py3-none-any.whl", hash = "sha256:41a13d9a1954d786f90cdbba0be0953fe9b9389f240b5ff71340c4706ea40135"}, - {file = "botocore-1.43.65.tar.gz", hash = "sha256:eea5440cf4b850d0f4de4f7eda418c325c87de6d8980873f8b636bb482b31d0f"}, + {file = "botocore-1.43.67-py3-none-any.whl", hash = "sha256:48ab8e9fac26fbc2a700d57010251003e6a5f731cf74d8540fb796bc8f3fc0ef"}, + {file = "botocore-1.43.67.tar.gz", hash = "sha256:6fe5cfa0c8676ba809efe505b618ec00f30d1af2d014bf316a7aa4ee86accb20"}, ] [package.dependencies] @@ -177,14 +177,14 @@ files = [ [[package]] name = "cachelib" -version = "0.15.0" +version = "0.15.4" description = "A collection of cache libraries in the same API interface." optional = false python-versions = ">=3.11" groups = ["main"] files = [ - {file = "cachelib-0.15.0-py3-none-any.whl", hash = "sha256:14a226c9856a48f1666cdb107fc4573171c1c820cb3fd2528a57f81fde8170e5"}, - {file = "cachelib-0.15.0.tar.gz", hash = "sha256:40a387c42d12e90c8a1a2cb87dedcc34fd9143a5b2c7f2daed3d4c56ebf7bdfc"}, + {file = "cachelib-0.15.4-py3-none-any.whl", hash = "sha256:500f447d6ebf52654bcbb1b7ed1329daf890252227f09933870a01a84798687a"}, + {file = "cachelib-0.15.4.tar.gz", hash = "sha256:66fb75f48f9077713c8a1ce2319f523d08418a1576f2cbdaad8aad15c884afa0"}, ] [package.extras] @@ -1834,14 +1834,14 @@ files = [ [[package]] name = "starlette" -version = "1.4.1" +version = "1.6.0" description = "The little ASGI library that shines." optional = false python-versions = ">=3.10" groups = ["main"] files = [ - {file = "starlette-1.4.1-py3-none-any.whl", hash = "sha256:7d078e0fbefae0d2cecfb80a799d6fb84b1c0c6acd4f14ac79d17d0e7ec27f19"}, - {file = "starlette-1.4.1.tar.gz", hash = "sha256:b7332de6e9375593a29ba9eee1e6ecfeb3eb2043e2e19a13b4b71da73ff35540"}, + {file = "starlette-1.6.0-py3-none-any.whl", hash = "sha256:a86dd39d14bb45f85a3d18525215a9ef0cfd1f192ac793220e72598c90335f0c"}, + {file = "starlette-1.6.0.tar.gz", hash = "sha256:d4e3ac5e546444960c710297a3c9fc3f7ebae1b7e963f3d36173b49da535be9b"}, ] [package.dependencies] @@ -1922,18 +1922,18 @@ files = [ [[package]] name = "typing-inspection" -version = "0.4.2" +version = "0.4.3" description = "Runtime typing introspection tools" optional = false -python-versions = ">=3.9" +python-versions = ">=3.10" groups = ["main"] files = [ - {file = "typing_inspection-0.4.2-py3-none-any.whl", hash = "sha256:4ed1cacbdc298c220f1bd249ed5287caa16f34d44ef4e9c3d0cbad5b521545e7"}, - {file = "typing_inspection-0.4.2.tar.gz", hash = "sha256:ba561c48a67c5958007083d386c3295464928b01faa735ab8547c5692e87f464"}, + {file = "typing_inspection-0.4.3-py3-none-any.whl", hash = "sha256:5f42b23858a91e0b4ef521f5418f03a0da3c9216fd2995ef5e73463100e676cd"}, + {file = "typing_inspection-0.4.3.tar.gz", hash = "sha256:c5f9ec1530b5c1e2c9bc34a84d9a3466ed1b2f3f2fa9f901368d9c5596210e4d"}, ] [package.dependencies] -typing-extensions = ">=4.12.0" +typing-extensions = ">=4.15.0" [[package]] name = "urllib3" From 8bc036860f2864658ec4cc7c714350ac1ef243b3 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre <4224001+paulineribeyre@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:01:32 -0500 Subject: [PATCH 3/4] not returning kms_key_arn anymore --- gen3workflow/aws/bucket.py | 16 ++++++---------- gen3workflow/routes/storage.py | 5 +---- tests/test_storage.py | 1 - 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/gen3workflow/aws/bucket.py b/gen3workflow/aws/bucket.py index 4ddde55..994294a 100644 --- a/gen3workflow/aws/bucket.py +++ b/gen3workflow/aws/bucket.py @@ -258,7 +258,6 @@ def setup_kms_encryption_on_bucket(bucket_name: str) -> None: # The deny in this policy fires when the headers are present but wrong (e.g. trying not to use # KMS encryption, or trying to use a different KMS key). If the headers are absent, the request # is accepted and AWS falls back on the bucket's default encryption (set above). - # TODO: stop specifying the KMS key in the funnel config new_bucket_policy = { "Version": "2012-10-17", "Statement": [ @@ -301,8 +300,6 @@ def setup_kms_encryption_on_bucket(bucket_name: str) -> None: else: logger.debug("Bucket policy is already up to date") - return kms_key_arn - def enable_bucket_versioning(bucket_name: str) -> None: """ @@ -330,7 +327,7 @@ def enable_bucket_versioning(bucket_name: str) -> None: raise -async def _create_user_bucket(user_id: str) -> Tuple[str, str]: +async def _create_user_bucket(user_id: str) -> str: """ Create an S3 bucket for the specified user and return information about the bucket. @@ -402,9 +399,8 @@ async def _create_user_bucket(user_id: str) -> Tuple[str, str]: ChecksumAlgorithm="SHA256", ) - kms_key_arn = None if config["KMS_ENCRYPTION_ENABLED"]: - kms_key_arn = setup_kms_encryption_on_bucket(user_bucket_name) + setup_kms_encryption_on_bucket(user_bucket_name) else: logger.warning(f"Disabling KMS encryption on bucket '{user_bucket_name}'") clients.s3_client.delete_bucket_encryption(Bucket=user_bucket_name) @@ -414,7 +410,7 @@ async def _create_user_bucket(user_id: str) -> Tuple[str, str]: # Bucket versioning is necessary for S3Files enable_bucket_versioning(user_bucket_name) - return user_bucket_name, kms_key_arn + return user_bucket_name async def create_user_bucket(user_id: str) -> Tuple[str, str, str]: @@ -435,9 +431,9 @@ async def create_user_bucket(user_id: str) -> Tuple[str, str, str]: retry_backoff_factor = 2 for attempt in range(1, max_tries + 1): try: - bucket_info = await _create_user_bucket(user_id) - USER_BUCKET_CACHE.set(user_id, bucket_info) - return bucket_info + bucket_name = await _create_user_bucket(user_id) + USER_BUCKET_CACHE.set(user_id, bucket_name) + return bucket_name except ClientError as e: if ( e.response["Error"]["Code"] diff --git a/gen3workflow/routes/storage.py b/gen3workflow/routes/storage.py index 20cfd02..a5b7e64 100644 --- a/gen3workflow/routes/storage.py +++ b/gen3workflow/routes/storage.py @@ -40,7 +40,7 @@ async def storage_setup( # only users with access to create tasks should be able to setup their storage await auth.authorize("create", ["/services/workflow/gen3-workflow/tasks"]) - bucket_name, kms_key_arn = await create_user_bucket(user_id) + bucket_name = await create_user_bucket(user_id) bucket_prefix = "ga4gh-tes" bucket_region = config["USER_BUCKETS_REGION"] @@ -48,9 +48,6 @@ async def storage_setup( "bucket": bucket_name, "workdir": f"s3://{bucket_name}/{bucket_prefix}", "region": bucket_region, - "kms_key_arn": ( - kms_key_arn if config["KMS_ENCRYPTION_ENABLED"] and kms_key_arn else None - ), } if config["ENABLE_S3_FILES"]: diff --git a/tests/test_storage.py b/tests/test_storage.py index 03c486d..2a37277 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -62,7 +62,6 @@ async def test_storage_setup( "bucket": expected_bucket_name, "workdir": f"s3://{expected_bucket_name}/ga4gh-tes", "region": config["USER_BUCKETS_REGION"], - "kms_key_arn": kms_key_arn, } # check that the bucket was created after the call to `/storage/setup` From f1a106d43bc4ef3f77d391b18e0da5dd123d3c64 Mon Sep 17 00:00:00 2001 From: Pauline Ribeyre <4224001+paulineribeyre@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:13:10 -0500 Subject: [PATCH 4/4] update docstring --- gen3workflow/aws/bucket.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gen3workflow/aws/bucket.py b/gen3workflow/aws/bucket.py index 994294a..1e35fac 100644 --- a/gen3workflow/aws/bucket.py +++ b/gen3workflow/aws/bucket.py @@ -335,7 +335,7 @@ async def _create_user_bucket(user_id: str) -> str: user_id (str): The user's unique Gen3 ID Returns: - tuple: (bucket name, kms key ARN) + (str) bucket name """ user_bucket_name = get_bucket_name_from_user_id(user_id) try: