internal/checks/kube: add more API resource requirements #10
Conversation
|
This pull request has been linked to Clubhouse Story #15962: Checks for required cluster API resources. |
| } | ||
|
|
||
| // TODO: delete this when the enterprise-helm role no longer requests resources on things | ||
| // that don't exist. |
There was a problem hiding this comment.
This is 100% duplicated code, but it's easy to delete.
| NewResourceRequirement("metrics.k8s.io", "v1beta1", "storageclasses"): verbsGetListWatch, | ||
| NewResourceRequirement("networking.k8s.io", "v1", "deployments"): verbsAll, | ||
| NewResourceRequirement("networking.k8s.io", "v1", "events"): verbsAll, | ||
| NewResourceRequirement("networking.k8s.io", "v1", "persistentvolumeclaims"): verbsAll, |
There was a problem hiding this comment.
some of these don't appear to be networking requirements, maybe a typo of the group name here?
Not sure if that's related to your comment:
I ended up needing to break out some of the ResourceRequirements into a separate RoleOnlyResourceRequirements, due to how the role is specified in enterprise-helm. A number of the resources required to create the role don't seem to be things that can exist in the real world (please do correct me if I'm mistaken!)
There might be an issue in our Helm chart... The RBAC permissions are done very generically at an API level, so do not account for whether or not the resources actually exist.
There was a problem hiding this comment.
Yeah, the enterprise helm chart specifies the following:
rules:
- apiGroups: ["", "apps", "networking.k8s.io"] # "" indicates the core API group
resources: ["persistentvolumeclaims", "pods", "deployments", "services", "secrets", "pods/exec","pods/log", "events", "networkpolicies"]
verbs: ["create", "get", "list", "watch", "update", "patch", "delete", "deletecollection"]
- apiGroups: ["metrics.k8s.io", "storage.k8s.io"]
resources: ["pods", "storageclasses"]
verbs: ["get", "list", "watch"]That seems to be exploded by Kubernetes to the things we're seeing.
I agree that we should fix it. However, it's a pretty high-impact change that we'd want to approach carefully, and I don't want to make coder-doctor working properly dependant on that being fixed.
| NewResourceRequirement("networking.k8s.io", "networking.k8s.io/v1", "ingresses"): verbsCreateDeleteList, | ||
| NewResourceRequirement("rbac.authorization.k8s.io", "rbac.authorization.k8s.io/v1", "roles"): verbsCreateDeleteList, | ||
| NewResourceRequirement("rbac.authorization.k8s.io", "rbac.authorization.k8s.io/v1", "rolebindings"): verbsCreateDeleteList, | ||
| NewResourceRequirement("", "v1", "events"): verbsAll, |
There was a problem hiding this comment.
Putting this here so we can have a threaded discussion:
At this point, running the check is starting to take a while, so I'd say the next step is to look into parallelising the RBAC checks (or using the equivalent API call to kubectl auth can-i --list).
@johnstcn I think it's preferable to figure out if we can reduce our requests before we parallelize, because doing stuff in parallel increases load on the API server and it might have throttling in place anyways. Is it possible to check multiple RBAC settings in one API call?
There was a problem hiding this comment.
Agreed, I believe doing the equivalent of kubectl auth can-i --list should work here.
Would you be happy with me pursuing this in a separate PR?
There was a problem hiding this comment.
Yeah, working code is better than perfect code :)
This updates the list of ResourceRequirements based on the permissions required to install Coder into a clean KinD cluster.
An initial role/rolebinding/user were created in the cluster, and permissions gradually added to the role until the helm install succeeded.
I ended up needing to break out some of the ResourceRequirements into a separate RoleOnlyResourceRequirements, due to how the role is specified in enterprise-helm. A number of the resources required to create the role don't seem to be things that can exist in the real world (please do correct me if I'm mistaken!)
At this point, running the check is starting to take a while, so I'd say the next step is to look into parallelising the RBAC checks (or using the equivalent API call to
kubectl auth can-i --list).