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 ServiceAccountName support to operator #283

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elliott-davis
Copy link

@elliott-davis elliott-davis commented May 17, 2018

When running services under the operator that need a service
account it is impossible to specify one given the current service
definition. This is useful for services running in pods that need to communicate with the kubernetes API with RBAC mode enabled

Signed-off-by: Elliott Davis elliott@excellent.io

@iaguis
Copy link
Contributor

iaguis commented May 23, 2018

Sorry for the late reply.

It seems your code generator is old and it regenerated some files, that's why the tests fail.

I think @asymmetric and @krnowak had some ongoing discussions on where in the struct to put new fields. They're off until Monday, maybe we can sync about this then.

@asymmetric
Copy link
Contributor

asymmetric commented May 28, 2018

I think you ran the code-generator at a version that's either too new or too old. Could you check out the kubernetes-1.10.0 tag on the code-genearator and run it again?

Also, could you explain a usecase for this? AFAIU, you don't need to care about ServiceAccounts for your Pods, unless the stuff running in them needs to talk to the APIServer, and the default SA doesn't have enough permissions.

Is this related to the work on the builder?

@elliott-davis
Copy link
Author

elliott-davis commented May 28, 2018 via email

@asymmetric
Copy link
Contributor

I guess the problem with running the operator on GKE with RBAC enabled is that we cannot create ClusterRoles. This should be fixed IMO by the person in charge of user authorizations.

I'm not sure we need support for custom ServiceAccounts once that's taken care of. WDYT?

@elliott-davis
Copy link
Author

elliott-davis commented May 28, 2018 via email

@asymmetric
Copy link
Contributor

I think two things are being conflated here. The operator needs its own SA (because it needs to do special tihngs with the API), and that's taken care of by examples/rbac/rbac.yml.

Pods on the other hand most likely won't need to talk to the API, so they can just keep on using the default SA.

we should be able to tell a service what that role is

Do we though? Are there usecases where a service running in a Pod has needed to talk to the API server?

Even the operator needs to know what it's service role is in its deployment yaml

Sure, but that doesn't require changes to the Habitat type.

@elliott-davis
Copy link
Author

elliott-davis commented May 28, 2018 via email

@asymmetric
Copy link
Contributor

@elliott-davis could you re-run the code-gen with the right tag?

The other missing thing would be an example using this key.

After that, I think we should be good to go :)

@elliott-davis elliott-davis force-pushed the elliott/service_account branch 2 times, most recently from e82f463 to 5564407 Compare May 29, 2018 14:49
@elliott-davis elliott-davis changed the title DO NOT MERGE: Add ServiceAccountName support to operator Add ServiceAccountName support to operator May 29, 2018
@asymmetric
Copy link
Contributor

@iaguis What was the usecase you mentioned where it would make sense to add the SA field?

@iaguis
Copy link
Contributor

iaguis commented May 29, 2018

If you have some application running in your cluster that's protected by RBAC and you want to access it from a service deployed with the Habitat Operator, you will need a Service Account that gives access to the RBAC-protected application.

For example, if you want a service that deploys Helm Charts, you probably need to pass the tiller Service Account to the service.

@iaguis
Copy link
Contributor

iaguis commented May 30, 2018

RBAC-protected application

Although this means the application needs to be accessible through some kind of k8s API so maybe it's not the use case you're looking for...

@elliott-davis
Copy link
Author

Yeah my use-case is creating a service account for my service to interact with RBAC protected services in a cluster.

Copy link
Contributor

@krnowak krnowak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, but I'd like to have some changes in the example.

examples/habitat-updater/habitat.yml Outdated Show resolved Hide resolved
examples/habitat-updater/habitat.yml Outdated Show resolved Hide resolved
@jamesc jamesc self-requested a review January 16, 2019 23:50
@jamesc jamesc dismissed krnowak’s stale review January 17, 2019 00:04

changing reviewers

@jamesc jamesc force-pushed the elliott/service_account branch 2 times, most recently from 5382424 to 32d4bfe Compare January 17, 2019 00:19
When running services under the operator that need a service
account it is impossible to specify one given the current service
definition. This is useful for services running in pods that need to communicate with the kubernetes API with RBAC mode enabled

Signed-off-by: Elliott Davis <elliott@excellent.io>

Signed-off-by: James Casey <james@chef.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants