search

kubernetes-retired / cluster-registry

[EOL] Cluster Registry API
https://kubernetes.github.io/cluster-registry/
Apache License 2.0
238 stars 94 forks source link

Remove cluster role and binding for clusters #207

Closed font closed 6 years ago

font commented 6 years ago

Remove unnecessary cluster role and cluster role binding objects as I don't think we'll need it. This is mainly used to provide permissions for controllers within the cluster to perform specific cluster operations e.g. an admission controller to perform specific cluster resource operations. This cleans up and simplifies the code a bit. We can always add it back later if needed.

The k8s docs on this are a little misleading and suggest this is needed.

/sig multicluster

mlowery commented 6 years ago

@font: Would it be better to do the following:

  1. Rename clusterregistry.k8s.io:apiserver clusterrole to clusterregistry.k8s.io:admin.
  2. Rename clusterregistry.k8s.io:apiserver clusterrolebinding to clusterregistry.k8s.io:admin.
  3. Modify clusterregistry.k8s.io:admin clusterrolebinding to reference clusterregistry.k8s.io:adminclusterrole.
  4. Modify clusterregistry.k8s.io:admin clusterrolebinding to bind to only user AdminCN (remove service account)?
  5. State in the doc that these steps are performed and are simply an example. Edits to either or both of clusterrole and clusterrolebinding will be necessary to allow other users to perform CRUD operations on cluster objects.
font commented 6 years ago

@mlowery We need to avoid creating unnecessary RBAC objects, especially if they are just an example, unless they are explicitly required by the cluster registry. The admin that deploys the cluster registry will have access to the clusters resource. They are then able to create more fine-grained ACLs if they wish. I don't think we want the crinit tool to assume a particular cluster registry security deployment model.

font commented 6 years ago

@perotinus PTAL.

perotinus commented 6 years ago

/lgtm

Apologies, this fell off my radar in the midst of discussions about CRDs.

k8s-ci-robot commented 6 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: font, perotinus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes/cluster-registry/blob/master/OWNERS)~~ [font,perotinus] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment