[Feature Request] Allow adding annotations to ServiceAccounts

[cawolf]

Is your feature request related to a problem? Please describe. We are using rbac-manager to set up our ServiceAccounts for our GitLab runners, and we need to grant these ServiceAccounts permissions using AWS IAM roles. We do this by adding an annotation to the ServiceAccount, indicating the IAM role to assume. Right now, there is no native way to add annotations to the ServiceAccounts created by rbac-manager.

Describe the solution you'd like We would love to see the possibility to add metadata like labels and annotations to the created ServiceAccounts to help us maintaining our access controls properly.

Describe alternatives you've considered Currently, we are working around this by annotating the created ServiceAccounts in the rollout pipeline with kubectl annotate, exploiting the fact that currently the equality check of ServiceAccounts is not altered by additional annotations.

Additional context We can try to provide a PR for this feature if you think this makes sense.

[sudermanjr]

I agree, adding more metadata to service accounts could be valuable. There's some work currently ongoing around how we manage service accounts, and some discussion about the right ways to do it. It doesn't necessarily overlap or preclude this from happening, but something to be aware of if this is implemented.

See discussion here for more details - https://github.com/FairwindsOps/rbac-manager/issues/417

[cawolf]

Thanks for the feedback, and after a quick scan I think it makes sense to base this feature on the changes made in #417 . We will monitor that issue and base our PR on these changes.

[cawolf]

We saw the solution in #418 for #417 and would like to begin our implementation draft. However, we have some questions to start, as this is our first go at a kubernetes controller:

• in order to allow metadata being provided by the user, we would start by adding metav1.ObjectMeta ´json:"metadata,omitempty"´ to the v1beta1.Subject. Is this a backward compatible API change from your point of view?
• we saw the granular implementation of reconciler.saMatches() from #418 and are wondering, if we also should explicitly compare only metadata provided by the user instead?
• in any case, we will need to compare more metadata than before, should we extend the reconciler.metaMatches() or should we create a separate method for the metadata provided by the user?

[sudermanjr]

1. Yes, I would consider adding a field that is optional to be a backward compatible (minor semver) change.
2. I do think that only comparing the metadata provided by the user is appropriate. There's a lot of other things that might affect the metadata of a service account (including some automated), so this seems safest.
3. I think extending metaMatches would likely be fine.

[cawolf]

Hi @sudermanjr , sorry for the delay. I added a PR to solve this issue, and I'm looking forward to your feedback.