kubeflow / manifests

A repository for Kustomize manifests
Apache License 2.0
807 stars 869 forks source link

jupyter-web-app defines RoleBinding for non-existing ServiceAccount jupyter-notebook #2601

Open kromanow94 opened 8 months ago

kromanow94 commented 8 months ago

Description

The jupter-web-app defined in apps/jupyter/jupyter-web-app/upstream/base defines role-binding.yaml for a ServiceAccount that doesn't exist: jupyter-notebook.

The app is still working correctly because of a ClusterRoleBinding in cluster-role-binding.yaml. This cluster-role-binding.yaml references cluster-role.yaml with rules that consist and extends the rules defined in cluster-role.yaml.

Considering the above, I think it makes sense to delete both the role.yaml and role-binding.yaml

juliusvonkohout commented 8 months ago

are you sure that this serviceaccount does not exist in a single user / development deployment? We probably need to fix it in Kubeflow/kubeflow then.

kromanow94 commented 3 months ago

I checked across the kubeflow/kubeflow and kubeflow/manifests repository and it's true for both:

$ grep -RnI jupyter-notebook | grep -v "\.md" 
manifests/apps/jupyter/jupyter-web-app/upstream/base/role.yaml:4:  name: jupyter-notebook-role
manifests/apps/jupyter/jupyter-web-app/upstream/base/role-binding.yaml:4:  name: jupyter-notebook-role-binding
manifests/apps/jupyter/jupyter-web-app/upstream/base/role-binding.yaml:8:  name: jupyter-notebook-role
manifests/apps/jupyter/jupyter-web-app/upstream/base/role-binding.yaml:11:  name: jupyter-notebook
kubeflow/components/crud-web-apps/jupyter/manifests/base/role.yaml:4:  name: jupyter-notebook-role
kubeflow/components/crud-web-apps/jupyter/manifests/base/role-binding.yaml:4:  name: jupyter-notebook-role-binding
kubeflow/components/crud-web-apps/jupyter/manifests/base/role-binding.yaml:8:  name: jupyter-notebook-role
kubeflow/components/crud-web-apps/jupyter/manifests/base/role-binding.yaml:11:  name: jupyter-notebook

For now I'll be focusing or more critical Issues/PRs but I'll get back to it sometime in future.

juliusvonkohout commented 3 months ago

CC @thesuperzapper @kimwnasptd then.

nishi-t commented 3 months ago

@kromanow94 @juliusvonkohout Hi, I am interested in contributing to the kubeflow project and have started working on this issue. I have submitted a PR to 'kubeflow/kubeflow'. Your review would be greatly appreciated.

Please let me know if you also need PR for kubeflow/manifests.

juliusvonkohout commented 3 months ago

@kromanow94 @juliusvonkohout Hi, I am interested in contributing to the kubeflow project and have started working on this issue. I have submitted a PR to 'kubeflow/kubeflow'. Your review would be greatly appreciated.

Please let me know if you also need PR for kubeflow/manifests.

@nishi-t The PR https://github.com/kubeflow/kubeflow/pull/7616 will be merged by @thesuperzapper or @kimwnasptd and then I can synchronize it back to kubeflow/manifetsts and close the issue here as well.

nishi-t commented 3 months ago

@juliusvonkohout Thank you for your comment. I understand.