argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.8k stars 5.43k forks source link

support for multiple rbac configmaps #8324

Open bilalcaliskan opened 2 years ago

bilalcaliskan commented 2 years ago

Summary

Currently if we dont provide a config file with flag --policy-file argument, the ConfigMap argocd-rbac-cm from K8s is used as expressed in here. It will be great if we can provide multiple configmaps to increase maintainability of our RBAC configurations. If this is discussed earlier, sorry for the duplicate issue.

Motivation

Let's consider the case we are managing an environment with 100+ different teams. For each team, we should provide at least 3 lines of code in our configmap to be able to handle rbac over ldap. Managing a configmap which is that much huge is very difficult. Instead it would be better if we can create multiple configmaps per team and put a specific label on each configmap and then merge these proper labelled configmaps.

Proposal

As mentioned in the Summary and Motivation sections, my proposal is somehow picking the properly labelled configmaps and then merging them somehow in the related file. Roughly we can provide something like below arguments to provide mentioned functionality;

--multiple-policy-configmaps             boolean, defaults to false
--policy-configmap-label                    string, defaults to 'argocd-policy-file=true'

If this is OK, i will be glad working on the implementation of this issue!

lknite commented 2 years ago

Also looking for this. I use argocd and an applicationset that lets me install argocd on multiple clusters. I have a default rbac in the applicationset which grants admin access to users in a group, but I'd like to also be able to grant specific access to an installed argocd instance, allowing team members full access to a specific application only.

Maybe, if a configmap exists with a certain label it will be recognized as another rbac configuration? such as: app.kubernetes.io/part-of: argocd

crenshaw-dev commented 2 years ago

This has been mostly done. Would someone be willing to pick up the PR and apply apply the changes Jann requested? https://github.com/argoproj/argo-cd/pull/9976

bilalcaliskan commented 2 years ago

@crenshaw-dev missed that one, im on it!

neiser commented 1 year ago

@crenshaw-dev sorry for asking directly here, but we would also appreciate the same idea for the ArgoCD config map itself. It could merge the config by key similar how Helm would do value merging. This would improve composability I think. Would you appreciate a similar PR for that feature?

leoluz commented 1 year ago

sorry for asking directly here, but we would also appreciate the same idea for the ArgoCD config map itself. It could merge the config by key similar how Helm would do value merging. This would improve composability I think. Would you appreciate a similar PR for that feature?

I agree with @neiser and think that support for multiple keys in argocd-rbac-cm is preferable since we wouldn't have to manage another configmap.

I implemented a draft PR with that concept here: https://github.com/argoproj/argo-cd/pull/12511. This way the argocd-rbac-cm could then be composed by multiple keys starting with policy.csv.* like in the example below:

apiVersion: v1
kind: ConfigMap
metadata:
  name: argocd-rbac-cm
data:
  policy.default: role:readonly
  policy.csv: |
    g, group-admins, role:admin
    g, kubernetes:argocd-readonly, role:readonly
  policy.csv.overlay1: |
    p, role:tester, applications, *, */*, allow
    p, role:tester, projects, *, *, allow
    g, kubernetes:argocd-readonly, role:tester
  policy.csv.overlay2: |
    p, role:devops, applications, *, */*, allow
    p, role:devops, projects, *, *, allow
    g, group-devops, role:devops

The advantages of that approach are:

The cons are:

@bilalcaliskan would that approach address your use-case as well?

bilalcaliskan commented 1 year ago

yes @leoluz, thank you all for your efforts.

slammajamma28 commented 1 year ago

Hi all, our team is very interested in implementing this for our multi-tenant use case. What is the timeline we expect to see this made available?

crenshaw-dev commented 1 year ago

@slammajamma28 it'll be in 2.8.0-rc1, which will be released this coming Monday (June 26).

simonebenati commented 1 year ago

@crenshaw-dev Hello, I checked out https://github.com/argoproj/argo-cd/pull/12511 but it seems that there won't be any support for multiple configmaps. Am I reading it wrong? My use case would be I create an user via a secret and then I create a configmap for that user. without having to modify the existing argocd-rbac-cm and having to manage a long list of permissions

Is there anything on this?

crenshaw-dev commented 1 year ago

@simonebenati correct. Let's reopen this to track the request for multiple configmaps rather than just multiple keys.

xeonic-ant commented 1 year ago

It would also be beneficial if you could add not only configmap but also secret. Because it is quite convenient to store the permission configuration in vault, dynamically updating it, and most solutions for delivering values from vault to k8s create secrets rather than configmaps.

bilalcaliskan commented 1 year ago

if anyone is not working on this issue, i can start working on throwing a PR

crenshaw-dev commented 1 year ago

@bilalcaliskan I don't think anyone's actively working on it.

crenshaw-dev commented 1 year ago

Prior art: https://github.com/argoproj/argo-cd/pull/9976

bilalcaliskan commented 1 year ago

@crenshaw-dev Thanks for the reminder. i had tried to contribute that PR but somehow could not interact with the PR owner. starting from scratch right now.

maximmold commented 1 year ago

Would there be any concept of trusting other namespaces or would this all have to be colocated in the same argocd namespace?

bilalcaliskan commented 1 year ago

@maximmold IMHO, it should be in the same namespace since more namespace will make things harder and increase complexity.

bilalcaliskan commented 1 year ago

i could not make it sadly and stopped trying. it highly exceeds my knowledge about the project.

but i still think this will increase the value of product and if someone can implement this, it will help lots of operators around the world.

meln5674 commented 6 months ago

It seems the last three attempts have withered on the vine, and I have an urgent need for this, so I am throwing my hat into the ring as well.

@crenshaw-dev I believe I have addressed all concerns raised by maintainers in the PRs already linked to this issue, please let me know if there is any additional context I need to know, or any obstacles to resolving this in a timely fashion based on the ideas already explored here.

Asuforce commented 2 days ago

@crenshaw-dev Our team currently managing 10,000 applications in a single cluster, and the number of lines in the configMap exceeds 11,000. At this rate, we will reach the capacity limit of the configMap, making it difficult to operate the cluster. The PR that @meln5674 proposed matches our needs very well, and we would like to adopt it as an alpha feature. If there are any issues or necessary tests for this PR, I can assist.

Please let me know if there is anything you are considering.