Pipeline: Support for Tasks that require manual "approval"

[skaegi]

In a Pipeline I'd like to be able to declare a Task that requires "approval" that may or may not contains a taskRef. When started, it immediately moves into a "suspend" state until an external API call changes its state to "running". The "approval" Task should have a "timeout" that causes the Task to fail after a set period of time.

Something like this...

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: craves-approval
spec:
  tasks:
  - name: approve-me
    requiresApproval: true
    timeOut: 3600
    taskRef:
      name: optional-task-ref

[vdemeester]

For now, the "idea" was to not make this part of the core but rely on multiple pipelines and trigger. Main reason to keep the core "not too complicated".

/kind feature
/cc @hrishin

[chmouel]

I think that's related too, isntit ? https://github.com/tektoncd/pipeline/issues/233

[vdemeester]

It is indeed

[chmouel]

My 2c,

we maybe want taking the complexity out of pipeline but we may end up having the complexity ending up in the user pipeline, which imho is not fair.

a simple requiresApproval: true is a much easier thing to write in my tasks/pipeline than having to come up with some triggers/tasks and other huge blob of yamls.

even if we go to the way of having it tools generated (via cli/dashboard or other) it can be painful to maintain and comprehend...

[vdemeester]

we maybe want taking the complexity out of pipeline but we may end up having the complexity ending up in the user pipeline, which imho is not fair.

It really depends on what is getting "presented" to the user. Should tekton allow to define that easily : yes, should tektoncd/pipeline (aka core) have it : maybe not. (if this means having a higher level of definition for some stuff, maybe that's the way it needs to go).

a simple requiresApproval: true is a much easier thing to write in my tasks/pipeline than having to come up with some triggers/tasks and other huge blob of yamls.

even if we go to the way of having it tools generated (via cli/dashboard or other) it can be painful to maintain and comprehend...

Right, this is where clearing defining what is the scope for Tekton, and what is the scope of tektoncd/pipeline, and other components is important.

[sbwsg]

I'd also be curious what auth would look like for this. Who's allowed to approve? Is it tied to service accounts? Secrets? Is it some kind of token-based mechanism to ensure the person approving also started the pipeline (suggested in #233 as well)? How directly tied to a trigger implementation do we allow ourselves to be?

Super interesting; lots of questions. I reckon there's a fair amount of design work we'd need to tackle around this to make it generally useful.

[skaegi]

Those are all good questions. I was going for something ultra-low level where someone able to edit the TaskRun (like an agent in my world) would update a status field. The TaskRun controller would then notice and proceed as normal. e.g. no tokens or triggers or secrets etc.

At this point I'm more interested in the low-level mechanism vs. how it might be presented to the user as I feel that is outside the needed scope at this point.

[vdemeester]

One "problem" I see with that approach, is that it keeps the object (TaskRun) in the reconcilier loop. Early reports shows that we are not very performant already. Having this handle outside the core reconciler loop means less "weight" on the core controller.

[sbwsg]

Ah, I would've considered the stuff I mentioned as "low-level mechanism" and the "which status field do I set?" more of a "high-level" question :D

I'd definitely like to see a design doc for this if we pursue it. Really want it to be a general solution and not a thing we tack on for a single use case. Even if we don't build the general solution right away we should definitely be mindful of how this feature could grow in scope over just being a single field in a Run.

[jlpettersson]

I like the idea with keeping the core simple. But I feel that there are some missing piece here.

Consider _Pipeline 1_ --> _Pipeline2_

• At the end of _Pipeline 1_, some option need to be presented, e.g. a generated resource, presented as a button in GUI. This button must have some kind of ID. If _Pipeline 1_ should end here, this must be a "Pipeline Output Resource" or a "Pipeline Result"?.
• There must be some way to pass state from 1 to 2, e.g. uploading it somewhere with a key, that can be looked up by _Pipeline 2_? Optionally this could be something that is base64-encoded into the key/ID in the point above.
• _Pipeline 2_ should probably be started by a new trigger event, and be spawned by a TriggerTemplate, this could be authorized using Rego https://github.com/tektoncd/triggers/issues/484 and the request need to contain a key/ID from above, parsed with a TriggerBinding.

[bobcatfish]

I'm pretty sure this is a duplicate of #233 but folks seem to be happier to engage here - should we resolve #233?

If it's possible for someone to start gathering the requirements and proposals that are being described above into a doc (and eventually a TEP!) that'd be great!

[jlpettersson]

I'm pretty sure this is a duplicate of #233 but folks seem to be happier to engage here - should we resolve #233?

If it's possible for someone to start gathering the requirements and proposals that are being described above into a doc (and eventually a TEP!) that'd be great!

I have now created a design doc for this issue and #233
The document is available on https://docs.google.com/document/d/1NCIUnSjsuEofwZJE0L0Il0yXgdB7FtELXXSArOSzj44/edit

[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]

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]

@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.

[bobcatfish]

My preference is that we pursue solving this via custom tasks https://github.com/tektoncd/pipeline/issues/215 - with custom tasks, folks can create manual approval Tasks specific to their needs.