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

[Bug] Installing as a lower privileged user in namespace-scoped mode fails due to cluster-scoped resources in generated Role #1330

Closed
yaraskm opened this issue Dec 7, 2023 · 4 comments · Fixed by #1394
Labels
bug Something isn't working help wanted Extra attention is needed triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@yaraskm
Copy link

yaraskm commented Dec 7, 2023

Describe the bug
TokenReviews and SubjectAccessReviews permissions are not needed in the Role generated when running in namespace-scoped mode.

On our multitenant Kubernetes cluster, standard users only have the Admin role over the scope of their namespaces and cannot create resources at the cluster scope. When a user tries to install this chart in namespace-scoped mode, the generated Role and RoleBinding try to assign permissions on the TokenReviews and SubjectAccessReviews resources, which the user does not have so the request gets blocked. Since these are cluster-scoped resources, it makes more sense to just disable these rules in the generated Role conditionally.

Version
v5.5.2

To Reproduce
Steps to reproduce the behavior:

  1. Configure kubectl such that your are connecting to a cluster as someone who does not have admin permissions at the cluster scope.
  2. Attempt a helm install in namespace-scoped mode: helm upgrade -i grafana-operator oci://ghcr.io/grafana-operator/helm-charts/grafana-operator --version v5.5.2 -n tester --set namespaceScope=true

Expected behavior
The install should succeed with a generated Role that does not contain these cluster-scoped resource types.

Suspect component/Location where the bug might be occurring
https://github.com/grafana-operator/grafana-operator/blob/921a4da76410b7d1cc56d31da14a03859dc688fb/deploy/helm/grafana-operator/templates/rbac.yaml#L219-L230

Screenshots
Output of a test installation:

$ helm upgrade -i grafana-operator oci://ghcr.io/grafana-operator/helm-charts/grafana-operator --version v5.5.2 -n tester --set namespaceScope=true
Error: UPGRADE FAILED: failed to create resource: roles.rbac.authorization.k8s.io "grafana-operator-permissions" is forbidden: user "..." (groups=["system:authenticated"]) is attempting to grant RBAC permissions not currently held:
{APIGroups:["authentication.k8s.io"], Resources:["tokenreviews"], Verbs:["create"]}
{APIGroups:["authorization.k8s.io"], Resources:["subjectaccessreviews"], Verbs:["create"]}

Runtime (please complete the following information):

  • OS: Linux
  • Grafana Operator Version: v5.5.2
  • Environment: AKS v1.24, though this is repeatable on other cluster types
  • Deployment type: Helm deployment
  • Other: N/A

Additional context
Add any other context about the problem here.

@yaraskm yaraskm added bug Something isn't working needs triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 7, 2023
@weisdd
Copy link
Collaborator

weisdd commented Dec 7, 2023

@yaraskm thanks for opening the issue!

I suspect those permissions are actually unused in a generic k8s setup and are related to OpenShift oauth-proxy configuration. - They can be found in kustomize and helm manifests, but are absent from RBAC definition generated by kubebuilder. So, I guess, we can delete them.

@NissesSenap @pb82 @HubertStefanski WDYT?

@yaraskm
Copy link
Author

yaraskm commented Dec 7, 2023

@yaraskm thanks for opening the issue!

I suspect those permissions are actually unused in a generic k8s setup and are related to OpenShift oauth-proxy configuration. - They can be found in kustomize and helm manifests, but are absent from RBAC definition generated by kubebuilder. So, I guess, we can delete them.

@NissesSenap @pb82 @HubertStefanski WDYT?

That would make sense! The only time I've interacted with these resources has been for authentication delegation, so if the operator (or a child ClusteRole it creates for a managed Grafana instance) has no concept of delegating user authentication to the cluster, then they can be safely removed.

@NissesSenap
Copy link
Collaborator

Agreed, I think they should be removed. We would love a PR around this.
The PR might need a document part around oauth-proxy and point out that they need to add this access on their own.

We should make an extra note about this change in the release, since people might be reliant on it.

@NissesSenap NissesSenap added triage/accepted Indicates an issue or PR is ready to be actively worked on. help wanted Extra attention is needed and removed needs triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 8, 2023
@pb82
Copy link
Collaborator

pb82 commented Dec 12, 2023

The OAuth Proxy example already has a separate role with those permissions (and I think we only need them for the Oauth proxy). So we should be good to remove them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants