Skip to content

fix: use registry for helm registry .. commands #23142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blakepettersson
Copy link
Member

@blakepettersson blakepettersson commented May 25, 2025

For OCI Helm repositories that require credentials, helm registry login and helm registry logout needs to use the registry host, rather than the full repository url. The issue is that if an app has a repoURL that contains more than a host, e.g registry-1.docker.io/bitnamicharts, the full repo url gets passed to helm registry .. rather than just the hostname.

Using a repository credential with just the host doesn't help either, since that doesn't get matched to the repoURL, hence the creds don't get passed along to the repo server.

This notably affects seems to affect only Dockerhub (I haven't yet tried Harbor, but Quay seems to work without issue even with the full repoURL), since pretty much any chart that's hosted privately will have this issue. Fixes #22831.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

Copy link

bunnyshell bot commented May 25, 2025

🔴 Preview Environment stopped on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🔵 /bns:start to start the environment
  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

For OCI Helm repositories that require credentials, `helm registry login`
and `helm registry logout` needs to use the registry host, rather than
the full repository url. The issue is that if an app has a `repoURL` that
contains more than a host, e.g `registry-1.docker.io/bitnamicharts`, the
full repo url gets passed to `helm registry ..` rather than just the
hostname.

Using a repository credential with just the host doesn't help either,
since that doesn't get matched to the `repoURL`, hence the creds don't get
passed along to the repo server.

This notably affects Dockerhub, since pretty much any chart that's hosted
privately will have this issue. Fixes argoproj#22831.

Signed-off-by: Blake Pettersson <[email protected]>
@blakepettersson blakepettersson force-pushed the fix/ensure-oci-registry-login-always-uses-registry-host branch from aaf3fdc to 0a0b85a Compare May 25, 2025 20:31
Copy link

codecov bot commented May 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@146a779). Learn more about missing BASE report.
Report is 41 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #23142   +/-   ##
=========================================
  Coverage          ?   60.08%           
=========================================
  Files             ?      343           
  Lines             ?    57904           
  Branches          ?        0           
=========================================
  Hits              ?    34790           
  Misses            ?    20335           
  Partials          ?     2779           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Incorrect helm registry login to registry-1.docker.io for repo-server
1 participant