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

Embed TriggerBinding in EventListener triggers #371

Closed
ncskier opened this issue Jan 22, 2020 · 5 comments · Fixed by #548
Closed

Embed TriggerBinding in EventListener triggers #371

ncskier opened this issue Jan 22, 2020 · 5 comments · Fixed by #548
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@ncskier
Copy link
Member

ncskier commented Jan 22, 2020

Expected Behavior

I expect to be able to send static parameters to my TriggerTemplate without creating an extra TriggerBinding resource to define these parameters in.

Actual Behavior

In v0.1.0, we had a params field in the EventListener:

apiVersion: tekton.dev/v1alpha1
kind: EventListener
metadata:
  name: listener-interceptor
spec:
  serviceAccountName: tekton-triggers-example-sa
  triggers:
    - name: foo-trig
      binding:
        name: pipeline-binding
      template:
        name: pipeline-template
      params:
      - name: message
        value: Hello from the Triggers EventListener!

This field made it very easy for me to send static parameters to my TriggerTemplate. However, we removed it with the v0.2.0 release, because parameters were replaced with multiple TriggerBindings. So, now I must create a separate TriggerBinding resource with static parameters to use in my EventListener.
I expect to be able to define my static parameters in the EventListener. Having to create a separate TriggerBinding resource with my parameters is annoying.

I think that a nice solution to this problem is to implement the option to embed TriggerBindings in the EventListener. The implementation could be similar to how Tekton Pipelines handles embedding PipelineResources. They have a resourceRef or resourceSpec for each PipelineResource used in a TaskRun or PipelineRun (more info here). Similarly, we could have a bindingRef or bindingSpec for each TriggerBinding used in an EventListener.

@ncskier ncskier added this to the Triggers 0.3 milestone Jan 22, 2020
@dibyom
Copy link
Member

dibyom commented Jan 23, 2020

/kind feature

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

khrm commented Feb 4, 2020

/assign

@khrm
Copy link
Contributor

khrm commented Feb 5, 2020

We might have to go with supporting embedding in TriggerCRD if we implement that for solving EL pod proliferation issue.

@ncskier
Copy link
Member Author

ncskier commented Feb 5, 2020

Yep, I think that's true. However, I don't think that it affects this issue right now. The inherent structure of how TriggerBindings are used will remain the same.

Plus, I assume that if we decide to make a separate TriggerCRD, then there will be a discussion about whether or not to allow users to embed the TriggerCRD in an EventListener.

@ncskier
Copy link
Member Author

ncskier commented Feb 21, 2020

@khrm are you still working on this issue? Since there isn't a PR yet, we'll probably want to push this issue back into the v0.4.0 release.

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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants