rbac-manager icon indicating copy to clipboard operation
rbac-manager copied to clipboard

Dynamic creation of service accounts based on rolebinding label selec…

Open tototoman opened this issue 3 years ago • 5 comments

This PR fixes #

Checklist

  • [x] I have signed the CLA
  • [x] I have updated/added any relevant documentation

Description

What's the goal of this PR?

Based on https://github.com/FairwindsOps/rbac-manager/issues/137 We have some similar need. We would like to create dynamically a service account in each namespace we request a role binding based on label selectors. I see the difficulty here, it's not that obvious. This is a suggestion, and I'm ready to take into account your comments.

What changes did you make?

If a service account in an rbac definition does not have a specified namespace, we get the ones from the rolebindings if there are any.

What alternative solution should we consider, if any?

The current way is to have an exhaustive list of services accounts/namespaces.

tototoman avatar Jun 21 '22 16:06 tototoman

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jun 21 '22 16:06 CLAassistant

Thanks for the PR! I'll have to take some time to consider this and test out this implementation.

sudermanjr avatar Jun 22 '22 14:06 sudermanjr

Hello,

Is there some news? Can you please rerun the ci/circleci: insights

It shows the message: Vault token expired or not available!

Thanks

tototoman avatar Jul 19 '22 09:07 tototoman

Sorry about the long tail on this. I've been re-familiarizing myself with this codebase lately, and I took a look at the implementation here.

I do really like the idea, and I think this is probably how we should have done this originally. However, I am really concerned about the lack of backward-compatibility here with the old way that rbac-manager created service accounts.

Do you have any thoughts on how we could manage that? It's possible we document the changes and do a major release, but I also want to consider how we could do this without breaking existing installations.

sudermanjr avatar Jul 28 '22 14:07 sudermanjr

Sorry about the long tail on this. I've been re-familiarizing myself with this codebase lately, and I took a look at the implementation here.

I do really like the idea, and I think this is probably how we should have done this originally. However, I am really concerned about the lack of backward-compatibility here with the old way that rbac-manager created service accounts.

Do you have any thoughts on how we could manage that? It's possible we document the changes and do a major release, but I also want to consider how we could do this without breaking existing installations.

Thanks for your feedback, I appreciate. Concerning the backward compatibility problem, the only difference is when someone creates a service account without specifying a namespace.

If I'm not wrong, in this case, the current context is used. This means we create the service account in the same namespace the rbac-manager is running on, while we give the permissions on the role binding to a service account with the same name, but in each of the rbac namespaces.

The second part does not change, we just make it explicit. The only difference then is that we create with this change a service account in each namespace pointed by the selector if it does not exist, while previously the user had to create it by himself (Another CR, helm chart...).

There are indeed some situations where this could be a problem even if it's possible to find workarounds.

We could add an optional boolean to the Subject struct, by default to false, that allow to activate this feature. For example "useRoleBindingsSelectors". It will be used only if the subject is a service account and ignored on cluster role bindings.

Let me know if this solution fits you so I can update my PR accordingly,

tototoman avatar Jul 29 '22 10:07 tototoman

Hi,

Any reason why this PR was just closed? Would really need this feature as it would reduce repetitive role binding definitions by a lot!

Thank you

bb-Ricardo avatar Dec 08 '22 12:12 bb-Ricardo

Thanks for tour support. I have the same question.

tototoman avatar Dec 08 '22 14:12 tototoman