validate *Run SA's have access to ClusterTask

[gabemontero]

Expected Behavior

If a PipelineRun or TaskRun's SA (where if one is not specified, the default SA in the *Run's namespace is used) references a ClusterTask, then one would expect that if you log into the cluster with that SA's token used on a successfult PipelineRun or TaskRun that accesses a ClusterTask, and run

kubectl  auth can-i get clustertask <cluster task name>

You would get true, meaning in fact the ClusterTask could be successfully accessed by the SA in question, meaning it was granted access to this cluster scoped object by an administrator.

Actual Behavior

With out of the box Tekton, by default you get false on that kubectl call, but things work because the controller has access to all ClusterTasks. The user in effect inherits escalated privileges from the controller.

Steps to Reproduce the Problem

1. Run any *Run object that references a ClusterTask

Additional Info

Per the Jun 29 API WG, consensus was reached that this is in fact a bug, and I was tasked with opening the bug issue to track this.

That said, as Tekton has been running this way for a while, enforcement that a *Run SA has access will at least initially be a opt-in choice, with a default of not enforcing.

PR #2797 is up for this. I am also working with @vdemeester on adding a new prow test job so that we can easily validate behavior both when admin's opt into the new enforcement, as well as stay opt out of the enforcement.

[gabemontero]

/assign @gabemontero

[bobcatfish]

Per the Jun 29 API WG, consensus was reached that this is in fact a bug, and I was tasked with opening the bug issue to track this.

kubectl auth can-i get clustertask <cluster task name>

@gabemontero is this the case for non-cluster Tasks and Pipelines? i.e. if you specify a service account in a PipelineRun or a TaskRun, is that service account required to be able to fetch the Task or Pipeline? I don't think that it is.

If that's the case, I don't agree that this is a bug, I think this is a fundamental change in how permissions have worked for Runs and I think we should TEP it before we rush into it.

[gabemontero]

Per the Jun 29 API WG, consensus was reached that this is in fact a bug, and I was tasked with opening the bug issue to track this.

kubectl auth can-i get clustertask <cluster task name>

@gabemontero is this the case for non-cluster Tasks and Pipelines? i.e. if you specify a service account in a PipelineRun or a TaskRun, is that service account required to be able to fetch the Task or Pipeline? I don't think that it is.

Some clarifications @bobcatfish ... even if you do not specify an SA in the TaskRun or PipelineRun SA, an SA is still associated with the Pod. Namely the "default" SA in every k8s namespace, where in this case, we are talking about the default SA in the namespace the PipelineRun and TaskRun object was created in.

With that, whichever SA is ultimately associated with the resulting Pod, if you login as that SA, I think you are then asking how something like

kubectl auth can-i get task <taskname> -n <namesapce taskrun is in>

responds. To that, yeah, I think minimally it may not be a given based on k8s cluster offering or config that a given SA may not even minimally have get access to arbitrary resources in the same namespace.

Or in other words, expectations around access to namespaced resources sometimes has greyer realities than cluster scoped resources.

If that's the case, I don't agree that this is a bug, I think this is a fundamental change in how permissions have worked for Runs and I think we should TEP it before we rush into it.

Certainly, I can move on that. In hindsight, I don't think you were able to attend the June 29 API WG meeting (the one where I was pushed to the top of the agenda because the prior week's meeting ran to conclusion before my topic came up). It was in that meeting where, as I mentioned in this issue's description, I asked the "bug vs. feature" question and got a consensus on "bug".

Hopefully we can consider this path as the "circling back to get Christie's opinion" and go from there.

I'll report back here once I have the TEP up. I'll also note than including both namespace scoped Tasks/Pipelines in addition to any cluster scoped alternatives in the opt-in validation is not a huge leap if that is where the TEP lands us.

thanks

[bobcatfish]

Thanks for your detailed explanation and patience with me injecting my opinion late!

Some clarifications @bobcatfish ... even if you do not specify an SA in the TaskRun or PipelineRun SA, an SA is still associated with the Pod. Namely the "default" SA in every k8s namespace, where in this case, we are talking about the default SA in the namespace the PipelineRun and TaskRun object was created in.

that makes sense, however the tekton controller is responsible for resolving Tasks/Pipelines/ClusterTasks, not the pod, so my understanding is that the service account supplied (or the default service account in the case none is provided) is not involved in retrieving tasks/pipelines/clustertasks at all right now, is that your understanding too?

if that's the case, then this doesnt feel like a bug to me, it feels like requiring the SA have permissions to fetch the Task/Pipeline/ClusterTask is a change in behavior.

For example, i have a PR in the catalog to create a task to use a tool called boskos and the Tasks need to create and delete pods to operate, so I provided an example of this service account you could use (https://github.com/tektoncd/catalog/pull/408/files#diff-78c0750b849e58f6f49a9ac104e55bd3) - which you'll notice doesn't have any permissions to retrieve Tasks.

[gabemontero]

Thanks for your detailed explanation and patience with me injecting my opinion late!

Some clarifications @bobcatfish ... even if you do not specify an SA in the TaskRun or PipelineRun SA, an SA is still associated with the Pod. Namely the "default" SA in every k8s namespace, where in this case, we are talking about the default SA in the namespace the PipelineRun and TaskRun object was created in.

that makes sense, however the tekton controller is responsible for resolving Tasks/Pipelines/ClusterTasks, not the pod, so my understanding is that the service account supplied (or the default service account in the case none is provided) is not involved in retrieving tasks/pipelines/clustertasks at all right now, is that your understanding too?

Yes the pod is not resolving the Tekton CRDs. But I think to some degree it is orthogonal. From my perspective, there is an implied default SA if none is specified for the Tekton Run objects.

Which then get into a distinction common with k8s authorization, which I noted in my design doc (and will transfer to the upcoming TEP), namely "requesting user" vs. "SA" for a given k8s object. And in talking with members of k8s upstream sig-auth, moving away from the either/or approach (i.e. if either has permissions, it is OK) that pod creation took is advisable. I'm also getting recommendations from them whenever possible using the SA as the permission actor vs. the requesting user (in this case the controller) is advisable. That way, you allow for finer grained access control, vs. everybody getting access for free because the controller has access.

if that's the case, then this doesnt feel like a bug to me, it feels like requiring the SA have permissions to fetch the Task/Pipeline/ClusterTask is a change in behavior.

Yeah if it was not clear earlier, coupling the namespace scoped elements with cluster scoped elements, and removing an implied access because namesapce co-location, I am fine with saying this is not a bug, going down the TEP path, etc.

I switched the kind label before in my WIP PR. I'll refresh my memory on how to do that and do it here as well.

For example, i have a PR in the catalog to create a task to use a tool called boskos and the Tasks need to create and delete pods to operate, so I provided an example of this service account you could use (https://github.com/tektoncd/catalog/pull/408/files#diff-78c0750b849e58f6f49a9ac104e55bd3) - which you'll notice doesn't have any permissions to retrieve Tasks.

[gabemontero]

/remove-kind bug

/kind feature

[gabemontero]

OK I have a TEP PR up at tektoncd/community#152

Note @bobcatfish @vdemeester .... after EOB today I'm out of the office for bereavement/vacation until the end of next week, so please be advised it may a bit before I respond to your respective initial comments.

Also note, based on @bobcatfish 's questions around cluster vs. namespace scoped objects the TEP considers both forms, vs. this PR at the moment only considering cluster scoped.

[bobcatfish]

Note @bobcatfish @vdemeester .... after EOB today I'm out of the office for bereavement/vacation until the end of next week, so please be advised it may a bit before I respond to your respective initial comments.

Hey @gabemontero , I'm really sorry to hear that. Hope you and yours are holding up okay. Absolutely no hurry at all.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[gabemontero]

/remove-lifecycle rotten

/reopen

[tekton-robot]

@gabemontero: Reopened this issue.

In response to this:

/remove-lifecycle rotten

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[gabemontero]

I've validated that OPA based policies can satisfy the requirment.