Skip to content

Commit

Permalink
Split controller RBAC into cluster-wide and tenant roles
Browse files Browse the repository at this point in the history
The controller currently operates with a single ClusterRole that
spans a very broad set of access permissions. In multi-tenant
scenarios this kind of RBAC configuration can be quite dangerous.

In order to better support potential multi-tenant configurations
this PR splits the roles that the controller receives into two.

This PR does not actually change the level of access afforded to
the controller. Instead, the roles are split but remain
cluster-scoped by default. There should be no noticeable change
in behaviour from the existing RBAC configuration in master.

If a team wanted to start running a multi-tenant service they
would be able to bind tekton-pipelines-controller-tenant-access
using a RoleBinding instead of a ClusterRoleBinding, thereby
limiting the access that the controller has to specific tenant
namespaces.

Full credit goes to to @eddie4941 for designing these changes!
  • Loading branch information
Scott authored and tekton-robot committed Apr 15, 2020
1 parent fe5981e commit 5d9c881
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 14 deletions.
39 changes: 27 additions & 12 deletions config/200-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,17 @@
kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: tekton-pipelines-admin
name: tekton-pipelines-controller-cluster-access
rules:
- apiGroups: [""]
resources: ["pods", "pods/log", "namespaces", "secrets", "events", "serviceaccounts", "configmaps", "persistentvolumeclaims", "limitranges"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
- apiGroups: ["apps"]
resources: ["deployments"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
- apiGroups: ["apps"]
resources: ["deployments/finalizers"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
- apiGroups: ["admissionregistration.k8s.io"]
resources: ["mutatingwebhookconfigurations", "validatingwebhookconfigurations"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
# Namespace access is required because the controller timeout handling logic
# iterates over all namespaces and times out any PipelineRuns that have expired.
# Pod access is required because the taskrun controller wants to be updated when
# a Pod underlying a TaskRun changes state.
resources: ["namespaces", "pods"]
verbs: ["list", "watch"]
# Controller needs cluster access to all of the CRDs that it is responsible for
# managing.
- apiGroups: ["tekton.dev"]
resources: ["tasks", "clustertasks", "taskruns", "pipelines", "pipelineruns", "pipelineresources", "conditions"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
Expand All @@ -45,6 +42,24 @@ rules:
---
kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
metadata:
# This is the access that the controller needs on a per-namespace basis.
name: tekton-pipelines-controller-tenant-access
rules:
- apiGroups: [""]
resources: ["pods", "pods/log", "namespaces", "secrets", "events", "serviceaccounts", "configmaps", "persistentvolumeclaims", "limitranges"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
# Unclear if this access is actually required. Simply a hold-over from the previous
# incarnation of the controller's ClusterRole.
- apiGroups: ["apps"]
resources: ["deployments"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
- apiGroups: ["apps"]
resources: ["deployments/finalizers"]
verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
---
kind: ClusterRole
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: tekton-pipelines-webhook-cluster-access
rules:
Expand Down
15 changes: 15 additions & 0 deletions config/200-role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@
# See the License for the specific language governing permissions and
# limitations under the License.

kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: tekton-pipelines-controller
namespace: tekton-pipelines
rules:
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["list", "watch"]
# The controller needs access to these configmaps for logging information and runtime configuration.
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get"]
resourceNames: ["config-logging", "config-observability", "config-artifact-bucket", "config-artifact-pvc", "feature-flags"]
---
kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
Expand Down
21 changes: 19 additions & 2 deletions config/201-clusterrolebinding.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,31 @@
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
name: tekton-pipelines-controller-admin
name: tekton-pipelines-controller-cluster-access
subjects:
- kind: ServiceAccount
name: tekton-pipelines-controller
namespace: tekton-pipelines
roleRef:
kind: ClusterRole
name: tekton-pipelines-admin
name: tekton-pipelines-controller-cluster-access
apiGroup: rbac.authorization.k8s.io
---
# If this ClusterRoleBinding is replaced with a RoleBinding
# then the ClusterRole would be namespaced. The access described by
# the tekton-pipelines-controller-tenant-access ClusterRole would
# be scoped to individual tenant namespaces.
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRoleBinding
metadata:
name: tekton-pipelines-controller-tenant-access
subjects:
- kind: ServiceAccount
name: tekton-pipelines-controller
namespace: tekton-pipelines
roleRef:
kind: ClusterRole
name: tekton-pipelines-controller-tenant-access
apiGroup: rbac.authorization.k8s.io
---
apiVersion: rbac.authorization.k8s.io/v1beta1
Expand Down
14 changes: 14 additions & 0 deletions config/201-rolebinding.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: RoleBinding
metadata:
name: tekton-pipelines-controller
namespace: tekton-pipelines
subjects:
- kind: ServiceAccount
name: tekton-pipelines-controller
namespace: tekton-pipelines
roleRef:
kind: Role
name: tekton-pipelines-controller
apiGroup: rbac.authorization.k8s.io
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: RoleBinding
metadata:
Expand Down
12 changes: 12 additions & 0 deletions docs/developers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,3 +332,15 @@ tkn pipelinerun logs sum-and-multiply-pipeline-run-rgd9j -f -n default
```

As you can see, you can define multiple tasks in the same pipeline and use the result of more than one task inside another task parameter. The substitution is only done inside `pipeline.spec.tasks[].params[]`. For a complete example demonstrating Task Results in a Pipeline, see the [pipelinerun example](../../examples/v1beta1/pipelineruns/task_results_example.yaml).

## Support for running in multi-tenant configuration

In order to support potential multi-tenant configurations the roles of the controller are split into two:

`tekton-pipelines-controller-cluster-access`: those permissions needed cluster-wide by the controller.
`tekton-pipelines-controller-tenant-access`: those permissions needed on a namespace-by-namespace basis.

By default the roles are cluster-scoped for backwards-compatibility and ease-of-use. If you want to
start running a multi-tenant service you are able to bind `tekton-pipelines-controller-tenant-access`
using a `RoleBinding` instead of a `ClusterRoleBinding`, thereby limiting the access that the controller has to
specific tenant namespaces.

0 comments on commit 5d9c881

Please sign in to comment.