Deprecate repositories and repository.credentials keys of argocd-cm ConfigMap key

[alexmt]

Summary

The repositories and repository.credentials keys of argocd-cm ConfigMap contain yaml serialized list of repositories credentials. Both keys should be deprecated and replaced with just only list of secrets.

Motivation

These two keys make it difficult to manage repositories declaratively and imperatively at the same time (see #3218). In case when repositories are managed declaratively the one key forces users to put everything into a single YAML file. It is impossible to leverage e.g. Kustomize and merge patch files.

Proposal

Deprecate both repositories and repository.credentials keys. Infer list of repositories and repo credentials from list of secrets with label argocd.argoproj.io/secret-type=repository (same as we already manage cluster credentials)

[jangraefen]

I proposed something similar in #5398. I think one of them could be closed.

I am also very interessted to contribute this functionality. Would that be an option?

[alexmt]

Hello @jangraefen, Looks like #5398 proposes using CRDs. Is it a critical requirement or you agree that Secrets are good enough for repository credentials?

After had several discussions we've concluded that at least repository credentials don't need CRDs and could be stored just as Secrets. The reason is that sensitive information must be stored as a secret, either way, so CRDs looks like overkill in this case. Instead of strongly typed CRDs it is better to just use convention: Secret must have a label argocd.argoproj.io/secret-type=repository and predefined key names ( such as username, password, sshPrivateKey ) .

I agree that CRDs provide additional benefits, such as API validation. However the same time CRDs complicate deployment and require maintenance. We are hoping to simplify ConfigMap management using argocd-util CLI - CLI tool that simplify declarative management. More command and better documentation is coming in 1.9.

[alexmt]

Thank you for offering help! If you have time your contribution is very welcome. This feature is not on the critical path for 1.9 release and we have enough time to work on design and implementation. Please let me know what you think about using Secrets vs ConfigMaps.

I've created similar proposal for resource.customization setting: #5439

[jannfis]

To chip my 2 cents into this, both types of configuration (CRDs vs Secrets) would be a major breaking change for about all of our users and both would require equally much work on the user side to migrate existing repository configuration. Taking this into account, I guess v2.0 would be the proper release to target for either of both changes.

I have not been part of the mentioned discussions, so I don't know all arguments that were brought up already. However, implementing this as CRD instead of Secret will not only give us the already mentioned validation abilities, but also would allow us to manage repositories more in a truly Kubernetes-native way, enabling proper status retrieval using Kubernetes API. Also, viewing & making changes to Secrets is quite a pita, due to the base64 encoding requirements. argocd-util enhancement of course will remediate some of this pain, however the basic pita will remain.

But if there is consensus already on implementing as Secret, that'll be fine for me too.

[jangraefen]

Thank you for your reply @alexmt.

I tried to formulate #5398 in a way that it allows for secrets as a solution as well and discuss the pros and cons of using secrets vs. CRDs.

I fully agree that the actual credentials should always be stored within a secret and nowhere else. CRDs add a "strongly typed" API, validation and better compatibility. As someone who is responsible for operating several ArgoCD instances in a declarative way, these properties can be a huge qualitity of life improvment.

apiVersion: argoproj.io/v1alpha1
kind: Repository
metadata:
  name: argocd-example-apps
spec:
  url: https://github.com/argoproj/argocd-example-apps.git
  type: git
  usernameSecret:
    name: my-secret
    key: username
  passwordSecret:
    name: my-secret
    key: password
  privateKeySecret:
    name: my-secret
    key: privateKey

Personally, I am a big fan of explict APIs over conventions, but thats my personal taste. Secrets solve the problem at hand with way less complexity. Secrets are well supported by all Kubernetes tools, where as CRD support is not always perfect or even present.

In the end, @jannfis said it much better then I did. I fully agree with what he said.

apiVersion: v1
kind: Secret
metadata:
  name: argocd-example-apps
  labels:
    argocd.argoproj.io/secret-type: repository
type: Opaque
stringData:
  name: argocd-example-apps
  url: https://github.com/argoproj/argocd-example-apps.git
  username: ****
  passowrd: ****
  privateKey: ****

I closed #5398 in favor of this issue, since the discussion of CRDs vs. Secrets has seemingly already happend and secrets come out on top 👍.

---

Using ConfigMaps is something that did not cross my mind so far. I would be interessted in knowing what possible motivations for ConfigMaps would be?
One of the advantages of secrets is that all information is provided by a single resource. ConfigMaps would require storing the credentials externally within a Secret while also provide little to no advantage over Secrets, in my opinion.
Looking at #5439, where no sensitive information is stored, ConfigMaps could be a good option though.

Once a design is settled, I will gladly start working on this.

---

Could this be implemented in a downward-compatible way though? A first implementation could be done in two ways, without breaking compatibilty with existing installations:

1. The secrets are used to provision the argocd-cm config map. The argocd-cm config map is still the source of truth.
2. The secrets are used as additional inputs when building the list of all repositories.

Meanwhile the argocd-cm repository configuration is deprecated and marked for removal in v2. Personally, if this path would be chosen, I would do it with option 2. Option 2 does not suffer from any concurrency issues that might occur if multiple parties work with the same argocd-cm and seem like the cleaner transition.

[alexmt]

I agree both #5436 and #5439 it should be implemented in a downward-compatible way. I like option 2 as well:

Argo CD should keep parse the repository and repository.credentials config map keys. Additionally list of repositories should be loaded directly from secrets. So the user should be able to add more metadata to repository secret and remove repository entry from argocd-cm ConfigMap:

• label secret with argocd.argoproj.io/secret-type=repository or argocd.argoproj.io/secret-type=repository-credentials
• specify repository URL in annotation: argocd.argoproj.io/repo-url: <REPO-URL>

API methods that manage repositories should automatically migrate repo to new style during saving: remove config map entry and add secret metadata. What do you think?

[jangraefen]

I have a couple of questions:

• Should the repository URL really be an annotation? For clusters the URL is stored as part of the secret data. I think it would be preferable to not have two different ways to represent very similar information. I concede that having to encode the URL with base64 is not that nice, but stringData helps a bit here. Alternatively, cluster secrets should be aligned to also use annotations, in my opinion.
• Why distinguish between repositories and repository credentials? A repository secret is just a repository credentials secret without the actual credentials, right? Nevermind, I just noticed the difference. Repository credentials work for a lot of repositories, right? Makes sense.

I have some concerns about automatic migrations as well. That might lead to conflicts with GitOps-managed installations, during the migration period. Since the argocd-cm as well as the repository secrets could still both be used during a migration period, there is no need to automatically migrate to secrets.

What do you guys think about that? 🙂

[alexmt]

Agree about annotation vs data. Cluster secrets contain everything in data so it is good to be consistent and store repo secret metadata in data as well.

Regarding automatic migrations. I've proposed it to simplify code. If it is possible it would be perfect if we can keep old-style secrets untouched. If it is too difficult to implement I felt it is ok to "auto-migrate" secrets if user tried to change it using Argo CD API.

[jangraefen]

Thank you for clarifying your point. Now I can get behind your reasoning and agree with your idea. Still, I think this option should only be used as a last resort and needs to be discussed carefully if it should come down to this, to ensure that we do not break any self-managed instances.

[phs]

For clusters the URL is stored as part of the secret data. I think it would be preferable to not have two different ways to represent very similar information.

URLs are themselves sometimes secret as well, such as when they encode the basic auth user password directly with the https://user:password@hostname.com/ syntax.

[jangraefen]

@phs You have a valid point here, but I am unsure how to feel about this. For repositories, you should never have to decode credentials in the URL in the first place or am I missing something crucuial here? It should always be possible to store the credentials as own keys in the secret, right?

This is especially important, considering that the repository URL is displayed to normal users in plain text. The ApplicationResource usually contains the repository URL as plain text as well. If I get this right, havin the URL contain the credentials should therefor be discoraged anyway.

[jetersen]

🤔 Would this not complicate #5355

Github app credential would be used for the whole org or for several repos inside the org, so that has to be factored into this redesign.
As such @alexmt your suggestion for leaving the repo url in the annotation/data would work for a single repo.

• specify repository URL in annotation: argocd.argoproj.io/repo-url:

Perhaps an separately annotation with a list of repo urls or organization would work?

as in annotation: argocd.argoproj.io/github-org: github.com/argoproj or

argocd.argoproj.io/github-repos: |
  github.com/argoproj/argo-cd
  github.com/argoproj-labs/applicationset

Same could be true for multiple orgs

argocd.argoproj.io/github-orgs: |
  github.com/argoproj
  github.com/argoproj-labs

Potentially you can have the github host as a separate annotation and default to github.com

argocd.argoproj.io/github-host: github.com
argocd.argoproj.io/github-orgs: |
  argoproj
  argoproj-labs

argocd.argoproj.io/github-host: github.company.io
argocd.argoproj.io/github-orgs: |
  example-a
  example-b

[jangraefen]

Good point that you brought up @jetersen.

I had a look a quick look at #5355. Am I correct that this just adds extra fields to the Repository and RepoCreds struct, regarding this issue? In this case, I would be tempted to just see it as a different kind of the same secret. We already have the same scenario with Username-Password authentication vs. SSH-based authentication. If there is more to it, we might need to consider a different mechanism.
Also re-using the same credentials for a whole organisation or GitHub in general should work just fine by using RepoCreds, right? You just specify the URL prefix. I do get that this does not work for all scenarios, but I think we already do get good coverage for most use-cases here.

I am not too sure if I like the lists inside the annotations (or secret data) too much. Adding or removing organizations (or repositories for that matter) from the list would require manual patching and string manipulation. Something this feature tries to solve by splitting the data up. I think we have to accept the possible duplication and have one repository URL per secret or we go down the CRD route and "have a proper" list, which would be processable by the usual tooling again, without the need for string manipulation 😉.

What is your take on this?

[jetersen]

Yes it adds a few extra fields. So yes different kind would apply.

ah the URL prefix would solve the issue, I was not aware of this feature.

I think secrets that just have urls with no data would be a kin to existing repo creds.
Thereby it should read the fields from the secret that matched the repo prefix.

[jangraefen]

I think secrets that just have urls with no data would be a kin to existing repo creds.
Thereby it should read the fields from the secret that matched the repo prefix.

That is actually a very good point! I kinda like the idea of having a secret without data if no actual "secret" is stored. How would you weight that against having a consistent API for repository and cluster configurations?

[jetersen]

Now that I think about it would be hard to know which repositories a given app is using if everything is stored in secrets as in the declarative nature would go away. This would potentially also complicate the webhook.

[jangraefen]

Could you clarify that concern a bit more @jetersen?

The application still stores which repository it uses. Just the configuration and credentials of a repository is stored as a secret. The same behaviour as for declarative cluster configuration actually.

[jetersen]

Right, than there is no concern. I forgot we were only talking about argocd-cm.

[jangraefen]

@alexmt @jannfis Since the discussion seems to have quiet down for now, I would like to schedule some work time into working on this. I go the OK from my employer that I can contribute on company time, so I need to do some planning and coordination before I actually start tackling this.

Does that sound okay to you guys?

[alexmt]

Hello @jangraefen ,

Notification about your message got lost. Please go ahead! Assigned the ticket to you so that it would be clear that someone is working on it. No pressure :)