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: Better handling of K8s secrets for repository credentials & templates (#3016 #3028) #3029

Merged
merged 7 commits into from
Jan 27, 2020

Conversation

jannfis
Copy link
Member

@jannfis jannfis commented Jan 24, 2020

This PR fixes correct deletion of credential templates secrets when the corresponding template is deleted from ArgoCD (fixes #3028) and also handle secret generation in a much more stable way (fixes #3016).

It changes the name of newly generated secrets in that it will not encode part of the repo or template URL into the name anymore, and just uses the FNV hash as identifier (i.e. repo-XYZ, where XYZ is the hash represented as number). Also, to better differentiate between actual repository credential secrets and credential template secrets, the latter will be prefixed with creds- instead of repo-. This change is not breaking and fully backward compatible, as existing secrets will not be touched.

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).
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Optional. My organization is added to the README.
  • I've signed the CLA and my build is green (troubleshooting builds).

@@ -55,7 +55,6 @@ var (
KubeClientset kubernetes.Interface
AppClientset appclientset.Interface
ArgoCDClientset argocdclient.Client
settingsManager *settings.SettingsManager
Copy link
Member Author

Choose a reason for hiding this comment

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

The linter accused this of being unused, and I think it was right.

@@ -123,7 +122,6 @@ func init() {
})
CheckError(err)

settingsManager = settings.NewSettingsManager(context.Background(), KubeClientset, "argocd-e2e")
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above.

@jannfis jannfis marked this pull request as ready for review January 24, 2020 21:43
Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @jannfis !

@alexmt alexmt merged commit 9790a5d into argoproj:master Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants