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

Add dynamic service accounts in namespace matchLabel #137

Open
cnelson opened this issue Jun 3, 2020 · 15 comments
Open

Add dynamic service accounts in namespace matchLabel #137

cnelson opened this issue Jun 3, 2020 · 15 comments
Labels
enhancement Adding additional functionality or improvements pinned Prevents stalebot from removing priority: could Future work depending on bandwidth and availability

Comments

@cnelson
Copy link

cnelson commented Jun 3, 2020

When a namespace is created with a given label, I'd like to provision a service account in that namespace, and create rolebindings for it in other static namespaces

For example,

I have a static namespace named foo that always exists. Now when I create a new namespace with the label access: foo I'd like to

  • Automatically create a service account in the the newly created namespace
  • Create a rolebinding in the foo namespace that references this dynamically created account.

Is this possible?

@sudermanjr
Copy link
Member

Yep, that is possible. Please take a look at https://github.com/FairwindsOps/rbac-manager#dynamic-namespaces-and-labels

In this case your subject will be a service account, but that example should get you going.

@sudermanjr sudermanjr added the question Further information is requested label Jun 4, 2020
@cnelson
Copy link
Author

cnelson commented Jun 4, 2020

@sudermanjr I'm not quite getting how to make this work. I've got a config that looks like this:

rbacBindings:
  - name: foo
    subjects:
      - kind: ServiceAccount
        name: some-name
    roleBindings:
      - clusterRole: admin
         namespace: foo
         namespaceSelector:
           matchLabels:
             access: foo

which leads to two issues:

  • I can't specify the namespace for the service account in question, because I don't know it (I want it to be in whatever namespace matches the namespace selector. Leaving it unset, causes this error in the logs: Error creating Service Account: an empty namespace may not be set during creation

  • Assuming there is a way to create the service account in this manner, It doesn't appear that I can use namespace: foo to attempt to create a rolebinding in that namespace, while also using the namespace selector to do this based on labels. I end up with a rolebinding in the namespace matching the selector, not in in the foo namespace.

Does this make sense? I'm happy to provide more detail if needed

@sudermanjr sudermanjr self-assigned this Jun 4, 2020
@sudermanjr
Copy link
Member

Hmmm. This is interesting.

I am fairly certain that even if you specify the namespace on the serviceAccount (which you have to, you are correct), that it will create the first SA in that namespace, and then create additional ones in each namespace for matchLabels.

This may be a flaw in the logic for namespace selector. I will need to test that out.

@sudermanjr
Copy link
Member

Oh, and to create the label-matched bindings and the single-namespace binding, you'll need to specify multiple bindings, like so:

rbacBindings:
  - name: foo
    subjects:
      - kind: ServiceAccount
        name: some-name
        namespace: foo
    roleBindings:
      - clusterRole: admin
         namespaceSelector:
           matchLabels:
             access: foo
      - clusterRole: admin
         namespace: foo

I'm going to test that config on a local cluster here in a bit

@sudermanjr
Copy link
Member

sudermanjr commented Jun 4, 2020

apiVersion: rbacmanager.reactiveops.io/v1beta1
kind: RBACDefinition
metadata:
  name: testing
rbacBindings:
  - name: foo
    subjects:
      - kind: ServiceAccount
        name: test-service-account
        namespace: foo
    roleBindings:
      - clusterRole: admin
        namespaceSelector:
          matchLabels:
            access: foo
      - clusterRole: admin
        namespace: foo
└─ k get sa -n foo
NAME                   SECRETS   AGE
default                1         54s
test-service-account   1         6s

└─ kubectl get rolebinding -n foo
NAME                ROLE                AGE
testing-foo-admin   ClusterRole/admin   13s

└─ kubectl create ns foo2
namespace/foo2 created
└─ k label ns foo2 access=foo
namespace/foo2 labeled

└─ k get sa -n foo2
NAME      SECRETS   AGE
default   1         88s

└─ k get rolebinding -n foo2
NAME                ROLE                AGE
testing-foo-admin   ClusterRole/admin   84s

On inspecting that rolebinding in foo2:

  subjects:
  - kind: ServiceAccount
    name: test-service-account
    namespace: foo

Looks like I was entirely wrong initially. Sorry about the confusion.

@sudermanjr
Copy link
Member

This might be a good feature request, but I would have to look at the code to make sure.

@cnelson
Copy link
Author

cnelson commented Jun 5, 2020

If this use case is a feature you'd accept, I could submit a PR for it, provided we figure out how this should actually work first :)

@cnelson
Copy link
Author

cnelson commented Jul 10, 2020

@sudermanjr checking in on this? Thoughts on if this is a use case you'd want to support in this tool?

@sudermanjr
Copy link
Member

Sorry for dropping this.

I think it is something we would be willing to support. I need to ask my fellow maintainers. I posed the question internally.

@sudermanjr
Copy link
Member

Talked to @lucasreed and I think this is something we would accept a PR, but it will be a non-trivial amount of effort. Because of that, we probably won't be able to dedicate a large amount of time to it.

@sudermanjr sudermanjr added enhancement Adding additional functionality or improvements and removed question Further information is requested labels Jul 13, 2020
@sudermanjr sudermanjr changed the title Dynamic service accounts? Add dynamic service accounts in by namespace matchLable Jul 13, 2020
@sudermanjr sudermanjr changed the title Add dynamic service accounts in by namespace matchLable Add dynamic service accounts in namespace matchLabel Jul 13, 2020
@sudermanjr sudermanjr removed their assignment Aug 17, 2020
@stale
Copy link

stale bot commented Oct 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Marked as stale by stalebot label Oct 17, 2020
@stale stale bot closed this as completed Nov 7, 2020
@jjtroberts
Copy link

Was there any progress made on a PR for this?

I'm in a situation where I'd like to reduce friction for our developers. They are creating Argo workflows for ETL and we have each client process in its own namespace. I'd like to make this self-service for them, however, Argo requires elevated namespace privileges to manage the different pods it creates for each workflow step.

While I can submit workflows and specify a service account argo submit --serviceaccount <name>, AFAICT that service account must exist within the namespace where the workflows are created. If no sa is specified, it will use default.

I'd like for my devs to be able to create their namespace (along with other resources) from source control (Bitbucket pipelines in our case) and have rbac-manager automatically create the argo-bot service account and role binding within the namespace.

---
apiVersion: rbacmanager.reactiveops.io/v1beta1
kind: RBACDefinition
metadata:
  name: data-ops-default
  namespace: rbac-manager
rbacBindings:
  - name: argo-controller
    subjects:
      - kind: ServiceAccount
        name: argo-bot
    roleBindings:
      - clusterRole: admin
        namespaceSelector:
          matchLabels:
            workflows: argo
---
apiVersion: v1
kind: Namespace
metadata:
  labels:
    ci: edit
    workflows: argo
  name: data-ops-rbac-test

What is the LOE to accomplish something like this?

@sudermanjr sudermanjr removed the stale Marked as stale by stalebot label Dec 30, 2020
@sudermanjr sudermanjr reopened this Dec 30, 2020
@sudermanjr sudermanjr added the pinned Prevents stalebot from removing label Dec 30, 2020
@sudermanjr
Copy link
Member

@jjtroberts I don't believe any progress has been made on this. I definitely see the need for this, and it is something we would accept a PR for.

The one big consideration that I think will make this tricky to implement is backward compatibility. It's possible that we a new field to the CRD that is something along the lines of createSAInNamespace: <bool>. However, this means we would have a potentially breaking change to the CRD, which would require some careful consideration.

I believe the actual implementation would be relatively straightforward, but I haven't looked at adding this just yet.

@jjtroberts
Copy link

Understood, and thanks for the quick response. As tradeoff we'll probably allow developer PRs with required approval by our engineers into a central rbac repo. It doesn't get us away from having copies of similar definitions, but it would still be quicker than a ticket in a backlog.

@sudermanjr
Copy link
Member

Makes sense. Thanks for all the feedback and info. I'll make sure we at least discuss this feature going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adding additional functionality or improvements pinned Prevents stalebot from removing priority: could Future work depending on bandwidth and availability
Projects
Status: No status
Development

No branches or pull requests

3 participants