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 routing to different eventlisteners based on request content #205

Open
bobcatfish opened this issue Nov 5, 2019 · 10 comments
Open
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@bobcatfish
Copy link
Collaborator

Expected Behavior

In our recent working group meeting, @mnuttall described his use case where he wants to have triggering for many repos, where each needs to have different eventlistener configuration (). He would like a way to minimize the number of ingresses and running interceptors that are required to support this, e.g. something like:

  • One endpoint for all GitHub events to hit
  • One interceptor invoked to make sure that the request is valid
  • A way to route traffic to different eventlisteners based on request content (e.g. if the github repo is X and it is a pullrequest, use eventlistener Foo, but if it's a cron - not a PR - use eventlistener Bar)

Actual Behavior

In order to support this today, you can:

  • Have many eventlisteners, each with an interceptor that has to both 1) check if the request is valid and then 2) check if the request meets the criteria for that eventlistener. All events must go to all eventlisteners
  • ?

Additional Info

Question for @mnuttall: do you want to have many eventlisteners configured running in this scenario, or would you rather have one eventlisteners with many bindings, and a way of mapping from the request to the binding? (And of course is there anything I missed / got wrong about your use case? :D)

@mnuttall
Copy link

mnuttall commented Nov 5, 2019

Hi @bobcatfish thank you for writing this up.

One EventListener makes event routing simple - it gets all events. However it's then routing to multiple triggers across multiple repositories, and so maintaining it - via GitOps or otherwise - can become contentious. We may start with one uber-EventListener but I think we need to evolve beyond it.

Ideally I'd like the owners of a Pipeline for a specific repository to also own its Trigger artifacts - TriggerTemplate, TriggerBinding, EventListener. That would leave them more in control of a simple GitOps process to manage their EventListener - but now we have multiple EventListeners.

If we assume that all inbound webhooks arrive at a single collector, then that collector can do one of two things:

  • spray all inbound (webhook) events to all listeners, each of which must then filter out the bulk of them, or
  • select a particular event listener, perhaps by labels on its associated Service, to route the payload to the right target - which could then skip some validation.

So we have to have smarts either in an EventListener - 'is this for me?' - or in front of a set of EventListeners. I would like to be able to implement the 'spray' pattern, which makes EventListeners simple 'subscribers to an event feed.' So I'm left with wanting a simple condition or similar syntax on an EventListener - that makes it clear, from the EventListener's definition, what it's filtering for. A slightly less pleasant alternative is to allow multiple validators, and then have one take the job of 'check that target (repository) matches a given field in the event payload.' It's less pleasant because it's less declarative - less obvious from simply looking at the object - I need to understand the function and API of the validator in question.

@vtereso
Copy link

vtereso commented Jan 13, 2020

One endpoint for all GitHub events to hit | A way to route traffic to different eventlisteners

I suppose it would be possible to send a request to an "interceptor" that is actually an EventListener? A CEL expression (or some custom interceptor) could read the request and mutate/decorate the data to construct the necessary EventListener service name. However, I believe we will need to introduce support for interpolation within interceptors (or by some such method) so we could pass to the generated service name. If that is sound, I think it would be possible to have a single exposed endpoint that routes internally.

I think there is some discussion around what an appropriate response is for interceptors, which might tie into auditing of such a design.

Alternatively, handling any number of event types can already be handled with multiple triggers.

One interceptor invoked to make sure that the request is valid

Whether through use of the core interceptors (occurring within the EventListener sink pod) or as a separate interceptor, this should already be possible.

@dibyom
Copy link
Member

dibyom commented Jan 27, 2020

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 27, 2020
@tekton-robot
Copy link

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.

@tekton-robot
Copy link

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
Copy link

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 added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 13, 2020
@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.
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.

@tekton-robot tekton-robot added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 13, 2020
@bobcatfish
Copy link
Collaborator Author

/remove-lifecycle rotten

@dibyom do you think this makes sense to include in the roadmap?

@bobcatfish bobcatfish reopened this Aug 13, 2020
@tekton-robot tekton-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 13, 2020
@bobcatfish
Copy link
Collaborator Author

Oh it already IS in the roadmap https://github.com/tektoncd/triggers/blob/master/roadmap.md

/lifecycle frozen

@tekton-robot tekton-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Aug 13, 2020
@vdemeester
Copy link
Member

/area roadmap

@tekton-robot tekton-robot added the area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) label Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/roadmap Issues that are part of the project (or organization) roadmap (usually an epic) kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
Status: Todo
Development

No branches or pull requests

6 participants