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

feat(post-install-job): add rbac #1

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

Conversation

vdice
Copy link

@vdice vdice commented May 14, 2024

  • Adds RBAC config so that the post-install job can create the SpinAppExecutor resource

Note that currently the resource will be created in the same namespace as the helm chart (eg spinkube as currently used in the Makefile). This means SpinApps will need to be created in the same namespace (eg kubectl -n spinkube apply myapp.yaml)

  1. Do we want to set the namespace as default for the SpinAppExecutor resource? Or perhaps both the helm release namespace and default?
  2. Do we want to configure a post-delete hook to delete this resource when the helm release is deleted (this could/would be a tricky thing especially if we're installing the SpinAppExecutor resource in a namespace other than where the helm release is installed)

@jpflueger
Copy link
Owner

@vdice thanks for this!

Do we want to set the namespace as default for the SpinAppExecutor resource? Or perhaps both the helm release namespace and default?

I think that it makes sense to apply that executor to the default namespace.

Do we want to configure a post-delete hook to delete this resource when the helm release is deleted (this could/would be a tricky thing especially if we're installing the SpinAppExecutor resource in a namespace other than where the helm release is installed)

I think that we would probably want a pre-delete hook otherwise it will hang on the finalizer right? Maybe we just configure the namespace as default in values and use the same namespace in both post-install and pre-delete?

@vdice vdice force-pushed the feat/rbac-post-install branch from f3559a4 to 5bfef85 Compare May 29, 2024 19:10
@vdice
Copy link
Author

vdice commented May 29, 2024

@jpflueger okay I just hard-coded the namespace to default for the rbac role/rolebinding and shimexecutor apply for now.

As follow-up(s) we can explore using a values.yaml entry to allow the user to specify this namespace and potentially explore the pre-delete hook. However, the latter may be tricky. Currently, I don't think it is possible to delete the spinappexecutor resource if there are any spinapps on the cluster using it, which there very well might be if/when users attempt to delete this helm release (in which case it would just hang). So, further discussion on this detail is needed.

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.

2 participants