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

support for multiple rbac configmaps #8324

Open
bilalcaliskan opened this issue Jan 31, 2022 · 19 comments · Fixed by #12511 · May be fixed by #9976 or #17828
Open

support for multiple rbac configmaps #8324

bilalcaliskan opened this issue Jan 31, 2022 · 19 comments · Fixed by #12511 · May be fixed by #9976 or #17828
Labels
enhancement New feature or request

Comments

@bilalcaliskan
Copy link

bilalcaliskan commented Jan 31, 2022

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!

@bilalcaliskan bilalcaliskan added the enhancement New feature or request label Jan 31, 2022
@lknite
Copy link

lknite commented Oct 14, 2022

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
Copy link
Collaborator

This has been mostly done. Would someone be willing to pick up the PR and apply apply the changes Jann requested? #9976

@bilalcaliskan
Copy link
Author

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

@neiser
Copy link

neiser commented Nov 11, 2022

@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
Copy link
Collaborator

leoluz commented Feb 17, 2023

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: #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:

  • doesn't require users to provide additional configmaps.
  • backwards compatible
  • easy to integrate with kustomize overlays
  • simpler implementation

The cons are:

  • can not handle RBAC csv's bigger than 1MiB which is the current limit of configmap
  • requires config management setup (kustomize, helm, etc). Doesn't work with "JBOCM" concept(Just a bunch of ConfigMaps)

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

@bilalcaliskan
Copy link
Author

yes @leoluz, thank you all for your efforts.

@slammajamma28
Copy link

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
Copy link
Collaborator

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

@simonebenati
Copy link

@crenshaw-dev Hello, I checked out #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
Copy link
Collaborator

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

@crenshaw-dev crenshaw-dev reopened this Jul 4, 2023
@xeonic-ant
Copy link

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
Copy link
Author

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

@crenshaw-dev
Copy link
Collaborator

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

@crenshaw-dev
Copy link
Collaborator

Prior art: #9976

@bilalcaliskan
Copy link
Author

@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
Copy link

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

@bilalcaliskan
Copy link
Author

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

@bilalcaliskan
Copy link
Author

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
Copy link

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.

@meln5674 meln5674 linked a pull request Apr 13, 2024 that will close this issue
14 tasks
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
10 participants