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

[Feature Request] Allow adding annotations to ServiceAccounts #425

Closed
cawolf opened this issue Sep 19, 2023 · 5 comments
Closed

[Feature Request] Allow adding annotations to ServiceAccounts #425

cawolf opened this issue Sep 19, 2023 · 5 comments
Labels
enhancement Adding additional functionality or improvements help wanted Extra attention is needed priority: could Future work depending on bandwidth and availability stale Marked as stale by stalebot

Comments

@cawolf
Copy link

cawolf commented Sep 19, 2023

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.

@cawolf cawolf added enhancement Adding additional functionality or improvements triage This bug needs triage labels Sep 19, 2023
@sudermanjr
Copy link
Member

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

@sudermanjr sudermanjr added help wanted Extra attention is needed priority: could Future work depending on bandwidth and availability and removed triage This bug needs triage labels Sep 19, 2023
@cawolf
Copy link
Author

cawolf commented Sep 22, 2023

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

cawolf commented Sep 26, 2023

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 Allow extra manually added pull secrets to managed SA #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
Copy link
Member

  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.

@github-actions github-actions bot added the stale Marked as stale by stalebot label Nov 26, 2023
@github-actions github-actions bot closed this as completed Dec 4, 2023
@cawolf
Copy link
Author

cawolf commented Jan 8, 2024

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

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 help wanted Extra attention is needed priority: could Future work depending on bandwidth and availability stale Marked as stale by stalebot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants