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

ClusterRole's and ClusterRoleBindings should include namespace in the name #374

Closed
jlewi opened this issue Mar 7, 2018 · 6 comments
Closed

Comments

@jlewi
Copy link
Contributor

jlewi commented Mar 7, 2018

See #373

ClusterRole's and ClusterRoleBinding's aren't namespaced scoped. So if we try to create multiple Kubeflow deployments in different namespaces they can interfere.

ClusterRoleBinding's are the bigger problem because we create namespace scoped service accounts, so the bindings for the different deployments are different.

If we include the namespace name in the ClusterRoleBinding this would make the names unique and allow us to better support multiple deployments.

@zjj2wry
Copy link
Member

zjj2wry commented Mar 7, 2018

serviceaccount is an array. when there are multiple serviceaccount in clusterrolebindding, we do not know which partition to use.

@jlewi
Copy link
Contributor Author

jlewi commented Mar 7, 2018

What I'm suggesting is that we create 1 clusterrolebinding per Kubeflow deployment and make the name unique so that if we have multiple Kubeflow deployments running in different namespaces there is still 1 clusterrolebinding per Kubeflow deployment.

@zjj2wry
Copy link
Member

zjj2wry commented Mar 8, 2018

kubectl create clusterrole ambassador
kubectl create  clusterrolebinding ambassador --clusterrole=ambassador --sa={ns-1}:ambassador --sa={ns-2}:ambassador
kubectl create rolebinding {ns-1}-ambassador --clusterrole=ambassador --sa={ns-1}:ambassador
kubectl create rolebinding {ns-2}-ambassador --clusterrole=ambassador --sa={ns-2}:ambassador

I think using rolebinding here may be a good approach, because clusterrolebinding is also a cluster-wide resource.
kubectl get rolebinding --all-namespaces | grep ambassador

@ankushagarwal
Copy link
Contributor

I tried using role bindings for ambassador, but it doesn't seem to work because by default ambassador tries to watch for services at the cluster level instead of the namespace level. When we use role binding, we can only give ambassador permissions within a single namespace and that fails.

So we might have to go with @jlewi 's proposal and create a separate clusterolebinding per deployment. eg: ns1-ambassador,ns2-ambassador where ns1 and ns2 are two kubeflow deployments on the same cluster

@ankushagarwal
Copy link
Contributor

I spoke too soon, we can restrict ambassador to a single namespace by setting the env variable AMBASSADOR_SINGLE_NAMESPACE : https://www.getambassador.io/reference/configuration

@ankushagarwal
Copy link
Contributor

Please take a look at #408

yanniszark pushed a commit to arrikto/kubeflow that referenced this issue Nov 1, 2019
* Skeleton for Kubernetes RBAC ClusterRole definitions

* Corrects tag spelling
Updates profile-controller image temporarily for testing

* Role definition corrections: labels and hierarchy

* Fixes unit tests

* Image change for frontend-k8s role mapping in kfam

* Fix Kubernetes Base test

* Changes 'kubernetes' pkg to 'kubeflow-roles'
Adds README to kubeflow roles
Removes image names and restores to original name

* Adds kubeflow-roles to KfDef for GCP IAP

* Address review comments
elenzio9 pushed a commit to arrikto/kubeflow that referenced this issue Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants