Replace gitlab-eventlistener with a working example

[dibyom]

Changes

I moved the example to a separate folder so that it was easy to add the sample
payload as well as instructions for trying it out locally.

Part of #409

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes tests (if functionality changed/added)
• Includes docs (if user facing)
• Commit messages follow commit message best practices

See the contribution guide for more details.

[dibyom]

/test pull-tekton-triggers-build-tests

[ncskier]

I left a few comments. I think that we need a few more resources in the YAML file (ServiceAccount, Role, and RoleBinding).

Let me know if I'm misunderstanding #409, but it was my understanding that a user would be able to kubectl apply -f the example YAML file into an empty namespace, and it would work (as long as Triggers is installed on their cluster).

[dibyom]

Let me know if I'm misunderstanding #409, but it was my understanding that a user would be able to kubectl apply -f the example YAML file into an empty namespace, and it would work (as long as Triggers is installed on their cluster).

Yeah, we'd need to copy the same Role/SA for each so I was hoping for the role resources we could reuse one or two. The user flow being 1. install triggers 2. apply the SA/roles 3. apply any of the examples.

I can definitely update the readme instructions to make this clearer

[ncskier]

Thanks for updating the instructions @dibyom 👍 I tested out the example, and it worked pretty well. I just have a few comments.

Also, I think that there's still a discussion to have about splitting up examples into multiple files or not...
I kind of like the idea of having all resources for an example defined in one file (including the Role, RoleBinding, ServiceAccount, etc.). Using one file makes it easy for someone new to Triggers to see everything that's involved in making a Triggers example work. I think that it can be easy to forget about the ServiceAccount, Role, and RoleBinding resources when they're in separate files. Neglecting the ServiceAccount, Role, and RoleBinding could lead newcomers to make avoidable mistakes when getting started with Triggers.

Additionally, declaring all the example resources in one file would bring some isolation to each example. I don't think that we should have any examples be dependent on each other.

What do you think?

[dibyom]

Also, I think that there's still a discussion to have about splitting up examples into multiple files or not...
I kind of like the idea of having all resources for an example defined in one file (including the Role, RoleBinding, ServiceAccount, etc.). Using one file makes it easy for someone new to Triggers to see everything that's involved in making a Triggers example work. I think that it can be easy to forget about the ServiceAccount, Role, and RoleBinding resources when they're in separate files. Neglecting the ServiceAccount, Role, and RoleBinding could lead newcomers to make avoidable mistakes when getting started with Triggers.

My motivation for keeping the roles in one central place:

1. It adds about ~30 extra lines to the top of each example which IMO makes it harder to figure out what exactly the example is doing
2. (more importantly) we will be repeating the exact same role/binding/serviceaccount for almost all examples (the exception being the cluster binding ones).

This is just incremental fix but what I'd eventually like is :

1. each example in its own folder/file...I think each file should contain the triggers pieces it needs
2. a common folder for some common things e.g. the roles or a common pipeline
3. An updated examples/Readme that tells users the pre-reqs - e.g. install triggers and the resources in the common folder. And then apply any of the examples --> I'm hoping we can move most of the info in gitlab/readme to examples/Readme once we have a few more examples converted.

What do you think?

[ncskier]

Thanks for addressing my comments! I noticed a couple small things when looking over the README again, but I think it should be good to go after these are addressed 👍

(Also, there's a merge conflict with the EventListener docs now)

---

My motivation for keeping the roles in one central place:

1. It adds about ~30 extra lines to the top of each example which IMO makes it harder to figure out what exactly the example is doing
2. (more importantly) we will be repeating the exact same role/binding/serviceaccount for almost all examples (the exception being the cluster binding ones).

Thanks for sharing your vision for the examples; it helps to understand what you have in mind 🙂 I definitely understand those reasons for keeping the Role and Service Account in one central place. It would add a decent number of repeated lines to each example. However, I still that the benefits would outweigh the costs. I don't think this is something we need to address in this PR though; as you said, this PR is an incremental fix.

One idea to consider is using Symbolic Links to put the shared resource files into an example directory. We currently do this for the Cron example, and it could be a helpful way to make examples exist within their own directories while using shared files for common resources.

[dibyom]

One idea to consider is using Symbolic Links to put the shared resource files into an example directory. We currently do this for the Cron example, and it could be a helpful way to make examples exist within their own directories while using shared files for common resources.

Hmm...would Kustomize work as well?

[ncskier]

Hm, maybe 👍 (I don't know enough about Kustomize)

[ncskier]

Thanks for making those updates!
/lgtm

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ncskier

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ncskier]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment