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

Add clusterrolebinding for listing nodes #6

Closed
wants to merge 1 commit into from
Closed

Conversation

bliemli
Copy link
Member

@bliemli bliemli commented Sep 5, 2021

This is somewhat strange. There's already a RoleBinding (note: not a ClusterRoleBinding which it should be) that references the ClusterRole cluster-admin`, but this obviously doesn't work as it's e.g. not possible to list nodes. At the same time I'm wondering why users of the webshell should have cluster-admin permissions.

If there's an agreement that we can just add the needed permissions to a separate ClusterRole we should do that instead of giving cluster-admin as a ClusterRoleBinding and remove these cluster-admin references alltogether.

@bliemli bliemli requested a review from sybnex September 5, 2021 14:09
@splattner
Copy link
Member

splattner commented Oct 5, 2021

I'm gonna address this in #9 and therefore close this

@splattner splattner closed this Oct 5, 2021
@splattner
Copy link
Member

I think what we need is the admin ClusterRole and not cluster-admin ClusterRole: https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles

But it is also possible to use the cluster-admin ClusterRole in a (namespaced) Rolebinding as stated in the linked kubernetes doc arcticle.
When used in a RoleBinding, it gives full control over every resource in the role binding's namespace, including the namespace itself.

@bliemli
Copy link
Member Author

bliemli commented Oct 5, 2021

The default ServiceAccount doesn't even need admin permissions inside its own namespace as it doesn't have to create RoleBindings. But we require certain permissions in other namespaces such as ArgoCD applications inside the argocd namespace.

So my suggestion would be to remove everything not needed permission-wise and only add what we need in specific ClusterRoles/Roles and ClusterRoleBindings/RoleBindings.

@splattner
Copy link
Member

I created an Issue for Tracking and followup

@splattner splattner deleted the nodepermissions branch November 14, 2021 11:59
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