Skip to content

Fix ENDPOINT_URL not exposed to Lua workers in nginx#219

Merged
matthiasL-scality merged 1 commit into
mainfrom
fix/gcs-presign-colon-encoding
Jul 17, 2026
Merged

Fix ENDPOINT_URL not exposed to Lua workers in nginx#219
matthiasL-scality merged 1 commit into
mainfrom
fix/gcs-presign-colon-encoding

Conversation

@matthiasL-scality

Copy link
Copy Markdown
Contributor

Summary

  • os.getenv('ENDPOINT_URL') always returned nil in Lua because nginx strips env vars from workers unless explicitly declared with the env directive — ENDPOINT_URL was missing from nginx.conf.template
  • Without it, the GCS backend detection in PRESIGN_PART (endpoint_url:find('googleapis', 1, true)) never matched; the else branch always ran, producing a canonical resource with ?partNumber=N&uploadId=X that GCS rejects
  • This explains why 4.4.5 still failed in production after the PR Fix GCS presigned URL signature for multipart part uploads #218 fix was deployed

Changes:

  • Add env ENDPOINT_URL; to conf/nginx.conf.template so os.getenv('ENDPOINT_URL') works in Lua workers
  • Move the %3A key encoding inside the if googleapis branch in PRESIGN_PART: GCS gets %3A + no subresources; standard S3 backends (cloudserver, Scaleway) keep literal : + subresources per V2 spec
  • Remove test_presign_upload_part_encodes_colons_in_url: the %3A encoding is GCS-specific and cannot be asserted in CI (cloudserver); test_presign_multipart_full_round_trip covers end-to-end correctness

Test plan

  • CI passes (test.yml — cloudserver round-trip test)
  • New release cut and deployed; re-run test-upload in scality/action-artifacts

🤖 Generated with Claude Code

@matthiasL-scality
matthiasL-scality requested a review from a team as a code owner July 17, 2026 12:06
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

LGTM — the root cause (missing env ENDPOINT_URL; in nginx.conf.template) and the PRESIGN_PART fix are correct.

Non-blocking note: the PRESIGN (single-file) block (~line 350) still unconditionally applies %3A encoding to colons. Now that ENDPOINT_URL is available, it could benefit from the same conditional treatment added here to PRESIGN_PART. Not a problem today (single-file presign has no subresources to omit), but will cause SignatureDoesNotMatch on non-GCS backends for keys containing colons.

Review by Claude Code

nginx strips env vars from Lua workers unless declared with the 'env'
directive. ENDPOINT_URL was missing from nginx.conf.template, so
os.getenv('ENDPOINT_URL') always returned nil — the GCS backend
detection in PRESIGN_PART never fired and presigned part PUTs to GCS
always used the wrong canonical resource (with subresources).

- Add 'env ENDPOINT_URL;' to nginx.conf.template so Lua can read it.
- In PRESIGN_PART, move the '%3A' encoding inside the GCS branch so
  standard S3 backends (cloudserver, Scaleway) keep literal ':' and
  subresources in the canonical resource, while GCS gets '%3A' and
  no subresources — matching what each backend verifies.
- Remove test_presign_upload_part_encodes_colons_in_url: the '%3A'
  encoding is GCS-specific and cannot be asserted in CI (cloudserver).
  The end-to-end round-trip test covers correctness for cloudserver.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@matthiasL-scality
matthiasL-scality force-pushed the fix/gcs-presign-colon-encoding branch from 6efb914 to b91ff11 Compare July 17, 2026 12:19
)


def test_presign_multipart_full_round_trip(session, artifacts_url):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three blank lines between test_presign_upload_part_encodes_special_chars_in_upload_id and test_presign_multipart_full_round_trip after the test removal — PEP 8 E303 expects two.

Suggested change
def test_presign_multipart_full_round_trip(session, artifacts_url):
def test_presign_multipart_full_round_trip(session, artifacts_url):

— Claude Code

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

LGTM — clean bugfix. The missing env ENDPOINT_URL; directive and the conditional %3A encoding are correct.

- Extra blank line in tests/end2end/test_presign_upload.py after test removal (3 blank lines instead of 2)
- Remove one blank line at line 120 to satisfy PEP 8 E303

Review by Claude Code

@matthiasL-scality
matthiasL-scality merged commit afa20fa into main Jul 17, 2026
5 checks passed
@matthiasL-scality
matthiasL-scality deleted the fix/gcs-presign-colon-encoding branch July 17, 2026 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant