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 podServiceAccountName to SpinApp CRD #228

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

Conversation

ThorstenHans
Copy link
Collaborator

With this PR, users are able to specify the ServiceAccountName for underlying pods

closes #226

Copy link
Contributor

@michelleN michelleN left a comment

Choose a reason for hiding this comment

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

Code looks good to me @ThorstenHans and seems like this a reasonable addition.

What does the failure scenario look like when a service account with the provided name doesn't exist?

@ThorstenHans
Copy link
Collaborator Author

What does the failure scenario look like when a service account with the provided name doesn't exist?

@michelleN that's a good question. When a user tries to create a Pod and is referencing a non-existing ServiceAccountName, the Pod is not accepted at all and a corresponding error message is presented to the user.

I would prefer our controller also checking for the specified ServiceAccountName in the current namespace, before it starts with creating underlying Kubernetes primitives

@ThorstenHans
Copy link
Collaborator Author

Any tips how I could implement validation of the serviceAccountName in the controller in order to prevent the SpinApp from being created, if the user provides a non-existing serviceAccountName?

Copy link
Contributor

Choose a reason for hiding this comment

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

Any tips how I could implement validation of the serviceAccountName in the controller in order to prevent the SpinApp from being created, if the user provides a non-existing serviceAccountName?

I would add a validation step in constructDeployment that ensures the service account exists before adding the service account name to the deployment spec. If the serviceaccount name was provided but doesn't exists, we should return an error / update the spinapp status.

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.

Spin App: Support ServiceAccountName for underlying Pods
2 participants