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 pre-created Service Accounts in the Helm chart #485

Closed
TBBle opened this issue Apr 21, 2020 · 6 comments · Fixed by #688
Closed

Support pre-created Service Accounts in the Helm chart #485

TBBle opened this issue Apr 21, 2020 · 6 comments · Fixed by #688
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@TBBle
Copy link

TBBle commented Apr 21, 2020

Is your feature request related to a problem?/Why is this needed

When using eksctl to build a cluster, it can auto-create Service Accounts with the desired IAMs Roles already configured.

However, the EBS-CSI Helm chart cannot use those existing roles as it tries to create its own, and Helm won't create resources over existing resources; nor should it.

/feature

Describe the solution you'd like in detail

The Helm chart's config allows disabling the creation of Service Accounts, and passing in the name of an existing Service Account for each use-case. This is very common in published Helm charts, e.g.
serviceAccount.create and serviceAccount.name in the nginx-ingress chart.

Describe alternatives you've considered

The workaround appears to be to create the cluster with eksctl, record the annotations and delete those service accounts, then install the EBS-CSI Helm chart passing the relevant annotations in as parameters.

I considered whether eksctl should be able to skip some ServiceAccounts during cluster creation, but that seems like an unusual case, since as mentioned above it seems that generally you can tell Helm charts to use an existing service account when they need one.

Additional context

This should be a relatively simple change in the chart, as we already have

serviceAccount:
  controller:
    annotations: {}
  snapshot:
    annotations: {}

in the values.yaml, so I expect it would look like

serviceAccount:
  controller:
    create: True
    name: ebs-csi-controller-sa
    annotations: {}
  snapshot:
    create: True
    name: ebs-snapshot-controller
    annotations: {}

and then changes in the templates to consume the new fields as appropriate.

@tomhuang12
Copy link

I would like this feature as well. Also, the deployment template for ebs-csi-controller needs to allow pod security context to specify fsGroup so that it can use the service account.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 19, 2020
@TBBle
Copy link
Author

TBBle commented Aug 22, 2020

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 22, 2020
@markussiebert
Copy link

I really would like to see this feature

@ayberk
Copy link
Contributor

ayberk commented Jan 8, 2021

/good-first-issue

@k8s-ci-robot
Copy link
Contributor

@ayberk:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants