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

Allow reuse of Argo CD repo credentials #138

Closed
mgoodness opened this issue Jan 8, 2021 · 6 comments · Fixed by #141
Closed

Allow reuse of Argo CD repo credentials #138

mgoodness opened this issue Jan 8, 2021 · 6 comments · Fixed by #141
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@mgoodness
Copy link
Contributor

Per the latest docs argocd-image-updater won't reuse Argo CD repo credentials for Git write-back. Is there a technical reason for the limitation, a security concern, or other? It would be great if users who understand the security implications could configure reuse and avoid adding another annotation to each Application definition.

@mgoodness mgoodness added the enhancement New feature or request label Jan 8, 2021
@jannfis
Copy link
Contributor

jannfis commented Jan 8, 2021

The main reason right now is, that Argo CD API does not return credentials with a repository request, for security reasons. So, there's no way for Image Updater to automatically fetch the credentials. Also, the default SA of image updater has no access to read K8s resources in the argocd namespace by default.

I guess one of the upcoming steps of integration is to move the image updater into the argocd namespace, and also allow it to read the Argo CD resources in K8s. That way, we can also read and re-use credentials configured in Argo CD, directly from the K8s resources, and optionally have them override this with an annotation (e.g. if the user configured in Argo CD is read only).

@jannfis jannfis added this to the v1.0.0 milestone Jan 8, 2021
@jannfis
Copy link
Contributor

jannfis commented Jan 8, 2021

I also think that in #35, @hawksight suggested to have a globally configurable secret holding Git credentials, so that not each Application would require a dedicated annotation, but repos would need a shared deploy key configured.

I'm pretty sure we won't change Argo CD API to also return secrets on repository data retrieval, and making argocd-image-updater deployment to be mandatory in argocd namespace is also more on the long term roadmap, would that be a trade-off you could live with?

@jannfis
Copy link
Contributor

jannfis commented Jan 8, 2021

Or maybe, it'd be possible to create a Role in the argocd namespace to allow reading of argocd-cm and the Git secrets, and bind it to argocd-image-updater SA. This would require some manual configuration as well.

Then, argocd-image-updater could use a rule like the following to figure out which secret to use:

  1. If neither global secret nor application specific credential is given, read Git credentials from secret configured in Argo CD
  2. If application specific credentials are given as annotation, use that
  3. Fall back to globally configured secrets

Or, maybe it's even better to merge the two annotations into one, and then use something like:

 argocd-image-updater.argoproj.io/write-back-method: git:<credref>

where <credref> would be:

  • global - use globally configured creds
  • repocreds - use creds from repo defined in Argo CD
  • secret:<namespace>/<secret>

@mgoodness
Copy link
Contributor Author

Using RBAC to grant image-updater read access to argocd-cm is exactly what I was thinking. I also like the idea of consolidating annotations.

@jannfis jannfis modified the milestones: v1.0.0, v0.9.0 Jan 19, 2021
@alexmt
Copy link

alexmt commented Jan 21, 2021

I also like both proposals. Working on PR

@alexmt
Copy link

alexmt commented Jan 21, 2021

If we are going in that direction we might want to consider loading applications from the same namespace ( in addition to load the list using Argo CD API). Created proposal: #140

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants