Skip to content

fix(argo-workflows): default S3 endpoint to s3.amazonaws.com when not set#3796

Merged
tico24 merged 3 commits intoargoproj:mainfrom
AndreKurait:fix/conditional-s3-endpoint
Mar 24, 2026
Merged

fix(argo-workflows): default S3 endpoint to s3.amazonaws.com when not set#3796
tico24 merged 3 commits intoargoproj:mainfrom
AndreKurait:fix/conditional-s3-endpoint

Conversation

@AndreKurait
Copy link
Copy Markdown
Contributor

@AndreKurait AndreKurait commented Mar 20, 2026

Fixes #3801

Summary

The S3 endpoint field in the artifact repository ConfigMap template renders an empty value when no endpoint is configured. The argoexec S3 driver passes this directly to minio-go (minio.New("")), which rejects it:

failed to create new S3 client: Endpoint:  does not follow ip address or domain name standards.

Neither an empty string nor omitting the key works — Go unmarshals a missing string field as "" (zero value), and argoexec has no empty-endpoint guard before calling minio.New(). The fix must provide a valid default.

Change

Default the endpoint to s3.amazonaws.com when not explicitly set:

Before:

endpoint: {{ tpl (.Values.artifactRepository.s3.endpoint | default "") . }}

After:

endpoint: {{ tpl (.Values.artifactRepository.s3.endpoint | default "s3.amazonaws.com") . }}

s3.amazonaws.com is the AWS global S3 endpoint. Minio-go explicitly recognizes it (IsAmazonEndpoint), enabling signature v4 and dual-stack automatically. It works with VPC gateway endpoints and interface endpoints (with private DNS enabled).

Deployments with custom endpoints (MinIO, LocalStack, GCS, etc.) are unaffected — their value takes precedence over the default.

Checklist:

  • I have bumped the chart version according to versioning
  • I have updated the documentation according to documentation
  • I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • Any new values are backwards compatible and/or have sensible default.
  • I have signed off all my commits as required by DCO.
  • I have created a separate pull request for each chart according to pull requests
  • My build is green (troubleshooting builds).

Copy link
Copy Markdown
Member

@tico24 tico24 left a comment

Choose a reason for hiding this comment

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

You're the second person today who allowed their LLM to delete the PR checklist and to not do the things in them. Humans have to spend time reviewing these PRs.

Please mark this PR as draft, re add the checklist and follow it. Thank you.

@AndreKurait AndreKurait force-pushed the fix/conditional-s3-endpoint branch from a6ec565 to 9f57d5a Compare March 20, 2026 19:46
@AndreKurait AndreKurait changed the title fix(argo-workflows): conditionally render S3 endpoint in artifact repository config fix(argo-workflows): default S3 endpoint to s3.amazonaws.com when not set Mar 20, 2026
@yu-croco yu-croco marked this pull request as draft March 21, 2026 07:03
@tico24
Copy link
Copy Markdown
Member

tico24 commented Mar 22, 2026

… set

The endpoint field was rendered with an empty value when no endpoint was
configured, causing argoexec to fail with 'does not follow ip address or
domain name standards'. The argoexec S3 driver passes the endpoint
directly to minio-go which requires a valid hostname.

Neither an empty string nor omitting the key works — Go unmarshals a
missing string field as "" (zero value), and argoexec has no
empty-endpoint guard before calling minio.New().

Default to s3.amazonaws.com (the AWS global S3 endpoint) when no
endpoint is explicitly configured. This is recognized by minio-go as an
Amazon endpoint and enables signature v4 and dual-stack automatically.

Fixes argoproj#3801

Signed-off-by: Andre Kurait <[email protected]>
@AndreKurait AndreKurait force-pushed the fix/conditional-s3-endpoint branch from e452661 to 5c2fdbe Compare March 22, 2026 13:39
@AndreKurait AndreKurait marked this pull request as ready for review March 22, 2026 13:42
@AndreKurait AndreKurait requested a review from tico24 March 22, 2026 13:43
@AndreKurait
Copy link
Copy Markdown
Contributor Author

@tico24 Thank you, i'm ready for a re-review now with the checklist followed

@tico24 tico24 enabled auto-merge (squash) March 24, 2026 08:17
@tico24 tico24 merged commit f6d5bb1 into argoproj:main Mar 24, 2026
7 checks passed
xavier-re pushed a commit to xavier-re/argo-helm-xr that referenced this pull request Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

argo-workflows: empty S3 endpoint causes argoexec failure — neither empty string nor omitting the key works

5 participants