Feature: Allow Triggers to invoke other triggers after interceptor processing

[jmcshane]

Feature request

Triggers processed in the sink right now can currently only resolve to a single template and set of bindings. As outlined in #820, there is often a goal to do some initial interceptor processing before invoking additional triggers. In this implementation, a Triggers field would be added to the current TriggerSpec that would be a list of refs to Trigger objects accessible to the EventListener sink. After interceptor processing, the sub-triggers would be invoked alongside any existing binding/template pair set on the trigger.

Some changes that would have to happen to make this work:

• Template/Bindings would become optional in the Trigger CRD - update the openapi spec for Triggers to force one-of either template/bindings or triggers.
• The sink Trigger resolution function would have to distinguish between "exposed" triggers and "internal" triggers (Propose using a label for this)
• Need to prevent infinite loop of triggers somehow
• Determine what triggers are available for "internal" processing. In my mind, all triggers would be available to use as a trigger ref, we are just filtering some of them from the sink.
• Determine if a Trigger can have both a template/binding and trigger refs.
• Determine how the event listener will respond to these requests. Should it wait for all the "sub"-triggers to be invoked to determine if it should response 201 or 202? For our use case its not essential, but I may not be aware of other use cases that take advantage of that status code.

Use case

The use case is fairly well outlined in #820 as well. My team's use case is that we are passing additional metadata along with each repository and having a webhook add that metadata via a webhook extension. We want to run that webhook once for each invocation of the eventlistener, then separate processing into a set of subsequent triggers after that initial extension is processed.

[dibyom]

Thanks for writing this up. So this looks like a solution to the problem discussed in #820 It's a problem that I've been thinking through a bit myself :)

We want to run that webhook once for each invocation of the eventlistener, then separate processing into a set of subsequent triggers after that initial extension is processed.

Is the main reason you want this maintainability of the YAML configurations or some performance concerns (or both)?

From going through the slack discussion, it seems to me that maintainability of the trigger configs was one big concern (if possible, could you share some of the trigger configs?)

So, my initial thoughts are:

1. It seems like that having filters as an implementation of interceptors and having interceptors only under triggers leads to reusability issues e.g. you can't share a common CEL filter interceptor if you need to. (this is the issue that@jmcshane mentioned)

2. In addition, calling one interceptor multiple times for a single event is inefficient and might lead to performance issues (though I'm not sure if that is the case currently) (this is the issue @afrittoli mentioned)

3. One issue is that this solution changes what constitutes a Trigger and makes it a bit more complex. This is something we can do but before we do that I'd like to see if there are other alternatives that might work as well.

4. Thinking out loud here. A few alternatives that you have already considered -- some notion of a shareable interceptor configuration, interceptors at the EventListener level, a simple caching layer for interceptor calls, some notion of a "group" of triggers that is distinct from the single Trigger that we have today.

5. There is also the issue of whether or not Triggers is the right place for this type of processing or whether it should be done in a different layer (@bigkevmcd brought this up)

Overall, looks like a lot to discuss so before we jump into a solution. Let's discuss the use cases (maintainability, performance) and the tradeoffs (complexity of the Trigger type). I can add this to the agenda for next week's WG.

In the meantime, we can keep discussing in this issue (and I'll try to spend some more time thinking through this problem!). Appreciate any feedback!

/cc @afrittoli @jmcshane @bigkevmcd

[jmcshane]

Thanks for your feedback @dibyom. I'll definitely plan to join the WG on Wednesday. I'm trying to sanitize out my company specific information from the Trigger objects, but I'll share some examples before WG.

One alternative I want to address is that idea of a "group" of triggers. I'm very willing to go with that approach, but the downside here is that you need to somehow select the bindings that tie to that trigger. For example, we're adding extensions on the event based on an API call as we are tracking the apps "onboarded" to our tekton platform. Then, these extensions allow a repository to get a number of different pipelines invoked based on different git events (PR, merge, tag, release, comment). With a group of triggers, we need to be able to conditionally select which ones in the group are valid for this event. This leads back to "how do you make that determination" and triggers already provides that answer: interceptors.

A shareable interceptor chain across multiple triggers is an interesting alternative, but it still leads to an explosion of triggers that are processed on invocation (especially if that requires multiple external API calls). One of the challenges here is that trigger templates provision static k8s resources (specifically, workspace configuration is something we are trying to control with this logic). The proposed solution reduces this because you can reduce the filter steps to exactly the point that you know that this one specific trigger is valid.

As we continue in the discussion, check out the changes I made to the API packages in #946. I don't mean to say its the "right" solution, but it gives a starting point of how this could be implemented.

[tekton-robot]

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.

[savitaashture]

Fixes as part of this PR https://github.com/tektoncd/triggers/pull/1232