Page MenuHomePhabricator

Drop the use of nonexisting groups in kubernetes infrastructure_users
Closed, ResolvedPublic

Description

We currently have a lot of users in profile::kubernetes::master::infrastructure_users: (private puppet) referring to groups that don't actually exist ("deploy" and "calico").

In Kubernetes, groups (apart from default groups that start with system:) "arise" from ClusterRoleBinding or RoleBinding objects with a subjects referring to them, e.g.:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: api-metrics
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: api-metrics
subjects:
- apiGroup: rbac.authorization.k8s.io
  kind: Group
  name: api-metrics

While there will not be an Group API object, the reference to a Group named api-metrics allows for users in the token file [1] to use that group and be granted permissions according to the above ClusterRoleBinding (that is, those of the ClusterRole api-metrics).

That said, we don't have such Bindings for "deploy" or "calico" and so we should remove those groups from the users to not cause further confusion.

If someone wants a list of "groups available in the cluster", that can be generated by something like: https://wikitech.wikimedia.org/wiki/Kubernetes/Kubectl/Cheat_Sheet#List_all_RBAC_%22Groups%22_referenced_in_the_cluster

[1] https://people.wikimedia.org/~jayme/k8s-docs/v1.16/docs/reference/access-authn-authz/authentication/#static-token-file

Event Timeline

JMeybohm created this task.

Thanks for looking this up!

I like the idea of dropping the unused groups deploy and calico. I checked the RoleBindings and ClusterRoleBindings and can only find bindings for the groups api-metrics, view and multiple system: groups.

This also matches my experience from RBAC refactoring for helm3. The group deploy had no effect on the actual permissions.

I've invented another group imagecatalog which is not used in RBAC roles. But I wanted to prevent having the users added to the deploy group (which is what we do currently if the user has no group set).

+1 on removing deploy, calico groups since they are unused. I am not sure that the imagecatalog group is for though?

+1 on removing deploy, calico groups since they are unused. I am not sure that the imagecatalog group is for though?

Just a dummy to prevent the imagecatalog user from being granted deployer rights (as we currently add deploy as default group in the infrastructure_users template. Can absolutely be removed when that default is gone.

+1 on removing deploy, calico groups since they are unused. I am not sure that the imagecatalog group is for though?

Just a dummy to prevent the imagecatalog user from being granted deployer rights (as we currently add deploy as default group in the infrastructure_users template. Can absolutely be removed when that default is gone.

Thanks for clarifying that for me.

Change 925707 had a related patch set uploaded (by JMeybohm; author: JMeybohm):

[operations/puppet@production] kubernetes: Remove infrastructure_users static token file

https://gerrit.wikimedia.org/r/925707

Change 925707 merged by JMeybohm:

[operations/puppet@production] kubernetes: Remove infrastructure_users static token file

https://gerrit.wikimedia.org/r/925707

Change 925726 had a related patch set uploaded (by JMeybohm; author: JMeybohm):

[operations/puppet@production] kubernetes: Remove infrastructure_users static token file

https://gerrit.wikimedia.org/r/925726

Change 925726 merged by JMeybohm:

[operations/puppet@production] kubernetes: Remove infrastructure_users static token file

https://gerrit.wikimedia.org/r/925726

JMeybohm claimed this task.

infrastructure_users is no more