Skip to content
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

Migrate chart location from ACR to GHCR #6801

Closed
2 tasks
youngbupark opened this issue Nov 17, 2023 · 11 comments · Fixed by #7455
Closed
2 tasks

Migrate chart location from ACR to GHCR #6801

youngbupark opened this issue Nov 17, 2023 · 11 comments · Fixed by #7455
Labels
bug Something is broken or not working as expected triaged This issue has been reviewed and triaged

Comments

@youngbupark
Copy link

youngbupark commented Nov 17, 2023

Summary

When helm retrieves the chart from GHCR, it always uses local docker credential to authenticate ghcr.io even for public repos. So if the credential is expired or invalid, it will return 403 Forbidden error. To mitigate this issue, we can override the authorization header to inject anonmous token (here is the PR #6799), but this way is just workaround solution. So it is better to show users the proper message to fix the credential problem.

 This is to migrate chart location from ACR to GHCR:

  • Revert this commit to reuse the old code - 6a4c730
  • When pull.Run returns 401 or 403, show the message to update ghcr.io credential or logout the ghcr.io

AB#10416

@youngbupark youngbupark added the bug Something is broken or not working as expected label Nov 17, 2023
@radius-triage-bot
Copy link

👋 @youngbupark Thanks for filing this bug report.

A project maintainer will review this report and get back to you soon. If you'd like immediate help troubleshooting, please visit our Discord server.

For more information on our triage process please visit our triage overview

@nicolejms nicolejms added the triaged This issue has been reviewed and triaged label Nov 20, 2023
@radius-triage-bot
Copy link

👍 We've reviewed this issue and have agreed to add it to our backlog. Please subscribe to this issue for notifications, we'll provide updates when we pick it up.

We also welcome community contributions! If you would like to pick this item up sooner and submit a pull request, please visit our contribution guidelines and assign this to yourself by commenting "/assign" on this issue.

For more information on our triage process please visit our triage overview

@nicolejms
Copy link
Contributor

We need to figure out how to do anonymous pull on GHCR. If user has expired credential, then need to update error message and ask user to refresh their creds.

@rynowak
Copy link
Contributor

rynowak commented Nov 20, 2023

Did we try overriding the authorizer? https://github.com/helm/helm/blob/main/pkg/registry/client.go#L83

It looks like we could configure this so that it ignores the users docker credentials.

@youngbupark
Copy link
Author

Did we try overriding the authorizer? https://github.com/helm/helm/blob/main/pkg/registry/client.go#L83

It looks like we could configure this so that it ignores the users docker credentials.

I tried but no way to set authorizer. The only way is using custom client with transport See #6799

@rynowak
Copy link
Contributor

rynowak commented Nov 20, 2023

I double-checked this and could confirm it. There's no way to set the registry authorizer from options.

@rynowak
Copy link
Contributor

rynowak commented Nov 20, 2023

Here's someone else asking for the same feature back in June: helm/helm#12144 It was closed as stale without any fixes submitted.

@rynowak
Copy link
Contributor

rynowak commented Nov 20, 2023

helm/helm#12584

@kachawla
Copy link
Contributor

kachawla commented Mar 25, 2024

Removing triage label to bring it back up for discussion to decide on its importance, since this is blocking part of #7309

@kachawla kachawla removed the triaged This issue has been reviewed and triaged label Mar 25, 2024
@willdavsmith
Copy link
Contributor

We can print a good error message and return the 403

@nicolejms nicolejms added the triaged This issue has been reviewed and triaged label Mar 28, 2024
@radius-triage-bot
Copy link

👍 We've reviewed this issue and have agreed to add it to our backlog. Please subscribe to this issue for notifications, we'll provide updates when we pick it up.

We also welcome community contributions! If you would like to pick this item up sooner and submit a pull request, please visit our contribution guidelines and assign this to yourself by commenting "/assign" on this issue.

For more information on our triage process please visit our triage overview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken or not working as expected triaged This issue has been reviewed and triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants