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

fix(argo-cd): Fix required cluster credentials name #3136

Merged
merged 5 commits into from
Jan 28, 2025
Merged

Conversation

atgane
Copy link
Contributor

@atgane atgane commented Jan 24, 2025

resummit [https://github.com/argoproj/argo-helm/pull/3111]

Fixes #3110

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.
  • My build is green (troubleshooting builds).

Signed-off-by: atgane <[email protected]>
@mbevc1
Copy link
Collaborator

mbevc1 commented Jan 24, 2025

Could you please update changelog as well?

@github-actions github-actions bot added size/S and removed size/XS labels Jan 24, 2025
Copy link
Member

@mkilchhofer mkilchhofer left a comment

Choose a reason for hiding this comment

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

Almost LGTM. I'd remove the lines inside the README.md/README.md.gotpl since our users will not notice this change.

Comment on lines 281 to 284
### 7.7.17

In clusterCredentials, key is used as the name of the cluster that will be registered in argocd. We have simplified the above condition by removing the require function.

Copy link
Member

Choose a reason for hiding this comment

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

Since there is no action required of our chart users, we can remove these lines. We use this chapter in the README only to instruct our users how to upgrade when special action is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your reply. I didn't really have anything to write because nothing has changed from the user's perspective, so I appreciate you clearing that up.

@github-actions github-actions bot added size/XS and removed size/S labels Jan 28, 2025
Copy link
Member

@mkilchhofer mkilchhofer left a comment

Choose a reason for hiding this comment

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

I implemented the requested changes for you (drop the unnecessary entries inside README).
Now LGTM.
Thanks for your contribution 🙏

@mbevc1 mbevc1 merged commit 2685b86 into argoproj:main Jan 28, 2025
6 checks passed
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.

Incorrect validation of CLUSTERNAME.name in clusterCredentials
4 participants