Convert cluster scoped RBAC to namespace scoped

[Gregory-Pereira]

In the llm-d project we have a strong interest in driving multi-tenancy, and to enable multiple installations per cluster. We currently fight quite a bit with helm in this regard:

STDERR:
  Error: Failed to render chart: exit status 1: Error: Unable to continue with install: ClusterRole "gaie-inference-scheduling-epp" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-namespace" must equal "llm-d-inference-scheduler": current value is "llm-d-inference-scheduling"
  Error: plugin "diff" exited with error

COMBINED OUTPUT:
  Error: Failed to render chart: exit status 1: Error: Unable to continue with install: ClusterRole "gaie-inference-scheduling-epp" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-namespace" must equal "llm-d-inference-scheduler": current value is "llm-d-inference-scheduling"

I propose we convert these ClusterRole and ClusterRoleBinding to namespace scoped RBAC.

cc @ahg-g @kfswain @nirrozenbaum

[Gregory-Pereira]

Tracking a conversation with Nir here:

the reason for this being a ClusterRole is the need of EPP for creating SubjectAccessViews and TokenReviews for secured metrics collection.
having said that, there is NO GOOD REASON for pods access in cluster scope.
there is a PR that was merged two days ago to split these rbac yamls, but there is a bug there that should be fixed- #1071

This means we need cannot simply swap CRBAC to RBAC, instead we need to use a uniquely qualified name for the release + namespace for CRABC, and can leave naming conventions as is for Namespace scoped stuff

[ahg-g]

This means we need cannot simply swap CRBAC to RBAC, instead we need to use a uniquely qualified name for the release + namespace for CRABC, and can leave naming conventions as is for Namespace scoped stuff

I support this as a short term fix, but I think we should push to have namespaced RBACs

[nirrozenbaum]

I support this as a short term fix, but I think we should push to have namespaced RBACs

+1. cluser scoped RBAC just for exposing metrics sounds like we’re doing something wrong.

[liu-cong]

@Gregory-Pereira Can you rename this to "convert cluster scoped RBAC to namespace scoped"?

[Gregory-Pereira]

Sounds good

[nirrozenbaum]

@liu-cong is there an acceptable alternative to using SubjectAccessViews and TokenReviews that would help with secured metrics collection?
as far as I know, these two are cluster scoped and there is no way to convert the roles to namespace scoped when using those.

[Gregory-Pereira]

as far as I know, these two are cluster scoped and there is no way to convert the roles to namespace scoped when using those.

+1, And this was why I renamed the issue previously to change scope.

[nirrozenbaum]

@liu-cong if I got it right from reviewing one of the other PRs, was the intention to switch to using PodMonitor from Prometheus operator?
if yes, we need to do the adaptations but it will work, as it is a namespace scoped resource.
I support this kind of a change (I prefer avoiding cluster scoped permissions when possible).