Consider using fine grained RBAC for components which interact with Knative CRDs

[dprotaso]

Once we transition to k8s 1.11 and merge the subresources PR #786 we should be able to utilize RBAC to limit which system components can update different resources and subresources (status & scale).

See slack for some discussion https://knative.slack.com/archives/C93E33SN8/p1528725270000107

Background Info

Under Authentication, authorization, and auditing

CRDs always use the same authentication, authorization, and audit logging as the built-in resources of your API Server.

If you use RBAC for authorization, most RBAC roles will not grant access to the new resources (except the cluster-admin role or any role created with wildcard rules). You’ll need to explicitly grant access to the new resources. CRDs and Aggregated APIs often come bundled with new role definitions for the types they add.

Our controllers use the cluster-admin role specified here:

serving/config/201-clusterrolebinding.yaml

Lines 15 to 26 in e1b3aae

	apiVersion: rbac.authorization.k8s.io/v1beta1
	kind: ClusterRoleBinding
	metadata:
	name: knative-serving-controller-admin
	subjects:
	- kind: ServiceAccount
	name: controller
	namespace: knative-serving-system
	roleRef:
	kind: ClusterRole
	name: cluster-admin
	apiGroup: rbac.authorization.k8s.io

We can make these permissions more fine grained. In Referring to Resources

Most resources are represented by a string representation of their name, such as “pods”, just as it appears in the URL for the relevant API endpoint. However, some Kubernetes APIs involve a “subresource”, such as the logs for a pod
...
To represent this in an RBAC role, use a slash to delimit the resource and subresource.

kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
 namespace: default
 name: pod-and-pod-logs-reader
rules:
- apiGroups: [""]
 resources: ["pods", "pods/log"]
 verbs: ["get", "list"]

Related:
Clusterrole binding (#1095)
Best practices for updating [CRD] status (#1107)
Best practices for updating a CRD's metadata (#1127)
Best practices for updating a CRD's spec (#1128)

[grantr]

Related: @rootfs is working on RBAC for the current CRDs without subresources in #1109.

[mattmoor]

Related: #664

[mattmoor]

@dprotaso Given the recent changes to our RBAC, what do you envision is left here?

When your and @bsnchan's subresources change lands, should we start taking advantage of even finer RBAC?

[dprotaso]

@dprotaso Given the recent changes to our RBAC, what do you envision is left here?

For now remove the ability for users to create revisions explicitly. Only our controller should have that capability

When your and @bsnchan's subresources change lands, should we start taking advantage of even finer RBAC?

Only allow our controller to update the status subresource

/assign @dprotaso

[mattmoor]

@dprotaso Are you still interested in this?

[dprotaso]

I'm going to close this issue as this isn't necessarily applicable at this time.

Long explanation

The controller pod runs with a single service account so it's not really possible to restrict this account to only update object statuses. This is because we have this 'monolith' process that reconciles & creates many objects (ie. services, configs, revisions, routes).

At this point I don't think it's necessary to break up this controller. It has the benefit of the shared informer cache between the reconcilers. Secondly, now that we use UpdateStatus calls it should prevent reconcilers from 'accidentally' updating an object's spec.

Secondly, as of this time our controller no longer references the cluster admin role but instead uses a finer RBAC rules.

As for

For now remove the ability for users to create revisions explicitly. Only our controller should have that capability

This could technically be done already using RBAC rules already.