Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support a pipeline of execution #820

Closed
afrittoli opened this issue Nov 2, 2020 · 8 comments
Closed

Support a pipeline of execution #820

afrittoli opened this issue Nov 2, 2020 · 8 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@afrittoli
Copy link
Member

Expected Behavior

As a triggers user I would like to be able to run a series of common operations on the event payload before splitting into the various triggers.

Examples are:

  • (1) checking the git provider token
  • add extra content / perform pre-processing of the payload to add fields required for CEL filters downstream. Examples:
    • (2) extract commands from a github comment
    • (3) append the PR details to the payload when missing
    • (4) verify if the PR / comment submitter is authorized to run CI checks

Actual Behavior

In the CI setup I have a single event listener with multiple triggers:

  • one for pull_request events on any repo (for shared CI jobs)
  • one for comment events on any repo (for shared CI jobs)
  • one for pull_request events on repo X (one for each tektoncd repo)
  • one for comment events on repo X (one for each tektoncd repo)

Each of these need (1) and (4). All the comment triggers need (2) and (3) too.
Today the interceptor have to be executed again and again for each trigger for each event.
(3) and (4) are implemented via custom interceptors. (2) is done in the pipelines downstream.

Alternatives

a. Implement some way in triggers to run common filters / overlays before getting into the triggers.
b. Use a single trigger, and run all the checks on it. From there, trigger a task / pipeline that receives the whole payload, enhances it as required, and forwards the whole event to the event listener again. This first task could then do extra things like setting up source caches for downstream tasks.
c. Leave things as they are and implement a caching layer in the interceptors so that we do not have to go all way back to github all the time

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 2, 2020
@afrittoli afrittoli changed the title Support a hierarchy of execution Support a pipeline of execution Nov 2, 2020
@wlynch
Copy link
Member

wlynch commented Nov 3, 2020

Thanks for the feature request!

Initial reaction - this seems like a good goal (in particular, for webhook validation). We're looking to keep the EventListener largely agnostic, deferring to Interceptors to handle user customization and other things like credentials and caching. Having an EventListener level hook for user defined interceptors might be one way to approach this. We've been discussing adding catalog support for interceptors to make using Interceptors asier (e.g. you could use a predefined interceptor for comment -> PR resolution instead of writing your own), which EventListenters could presumably also use.

There's a few things we need to work through. In particular, we'll need to figure how how this impacts debugging for triggers (particularly w/ multi-tenant EventListeners where users have access to Triggers, but not necessarily to the EventListener).

As a short term solution, alternative (c) seems most reasonable to me. (b) has the downside of decoupling events, which can make it harder to debug/trace what's happening. (a) is this FR, which we'll continue to look into, but isn't available at the moment.

@tekton-robot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
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 with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 1, 2021
@dibyom dibyom removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 5, 2021
@dibyom
Copy link
Member

dibyom commented Feb 5, 2021

Ref #945, #940

@afrittoli
Copy link
Member Author

This seems partly related too: #205 - the feature talks about chaining event listeners - but the idea to have an initial common filtering logic is the same.

@dibyom dibyom added this to the Triggers Post Beta milestone May 12, 2021
@tekton-robot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
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 with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 15, 2021
@tekton-robot
Copy link

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

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 14, 2021
@tekton-robot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

4 participants