Enable TriggerBindings to validate requests (e.g. GitHub) #45
Comments
|
/assign |
|
Actually, not. Someone else feel free to tackle this. :) |
|
/assign |
|
I assign myself to work on this. Yet to start. Just following the discussions atm. |
|
Hey @vtereso ! I'm late to the From the example in #42 it looked like the main use case was for filtering based on the type of event, e.g.: But then I asked @dlorenc and he said the most important use case was about verifying that requests actually come from GitHub i.e. matching a token in the header against a previously known token - it's not clear to me from the designs I've seen how we would do that (esp if the matching is just a simple regex as we are discussing in #49) so I'd love to see more details about that if possible. |
|
I have mentioned in the conditionals discussion here, the headers field was an implicit way to access the headers without having to introduce another interpolation variable. I believe the intent was to "match" against anything that could be passed here, which could both designate and authenticate. It has been mentioned that an explicit separation between headers/conditionals might be good for the sake of clarity. I have thrown around the idea of combining them before, but that would imply introducing another variable to access the headers, which might be fine? From the recent discussion on that conditionals issue (in favor of using containers), headers seem to be overlapping more and more. |
|
I felt the general use case for headers match would be just simple match to send events to matching TriggerBinding whether it be Github, Gitlab or CE type coming in headers. If I understand @dlorenc use case, it's about securing webhook? I don't think we can validate token which is set up for webhook using just regex. Github computes the hash of payload using the above token and then pass the computed hash to the header. PS: I was on leave so the late response. I would be more active from Tuesday. |
|
@khrm I've tried to group together issues related to this: #78 (comment) It sounds like for this issue our next step will be to create a design for how we express authenticating the caller. |
bobcatfish
changed the title
|
For the authentication, we can extend the Filter idea in #71 to run the (Tekton)task. We need to create the TriggeringEvent pipeline resource that has the whole event header and payload. The task authenticates the event using the resource and success or failure based on the result. |
|
Interesting idea to use a Tekton Task to execute the validation 🙂 This sounds to me like an equivalent alternative to the conditionals being discussed in issue49. We could potentially use Tasks instead of Conditionals? Also, just a thought; instead of creating an "event" PipelineResource to pass the event header & payload to the Task, could we pass them to the Task as parameters? |
|
I thought we can have validation using conditionals. Just different pod for different providers. Like github pod, gitlab pod, etc. Why use tasks over conditionals, @akihikokuroda ? |
|
@khrm I thought it might not fit. I might not understand the conditional correctly. I'll look it, again. |
Yea. I was thinking of doing the same. @vtereso @akihikokuroda |
|
@khrm I looked at the implementation of the condition. The condition is executed as a task with one step in it internally. https://github.com/tektoncd/pipeline/blob/830ffc26e6bee0a5d11ce95ec328b2ef448f00de/pkg/reconciler/pipelinerun/pipelinerun.go#L624 |
I had the same reservation, but I have come around. To the point of GitHub authentication being non-trivial, the same can be said about other possible event sources and we don't want to shift this responsibility to Triggers directly. There is nothing stopping us from adding functionality to allow people to raw match/string match/etc. down the line to improve performance should we feel it necessary. If we consider Triggers as an extension point for runs, it should be reasonable to have run success toggle other runs. In this way, supporting Conditionals/images seems like a good approach since it follows the Triggers paradigm of supporting anything assuming it is configured properly. |
|
As far as the manifestation, I don't have particular feelings one way or the other. It seems like the |
|
Coming in late to all of this! Looks like a lot of discussion around whether or not we should use the condition implementation from pipelines. A few thoughts:
+1 I like this!
I think running an entire pipelinerun just to use conditions would be overkill. Though the current implementation is tied to pipelines, we could refactor the condition checking out into something that could be reusable. I think using TaskRuns is a good starting point. Besides reuse we get some additional features like timeouts, retries, variable substitution support etc. And as @vtereso says, it leaves room for optimization for simpler cases later on if we do determine that scheduling/running TaskRuns does not meet our performance/scaling goals.
Yeah, I think it would make sense to refactor this out into a package that can be reused by both pipelines, and triggers
I think these could work though I'm not sure you'd want to expose the full For conditionals in pipelines, we did briefly think about just using regular tasks but decided to go with a more restricted subset (i.e. In summary, I think it makes sense to run TaskRuns to execute the validation/filtering/conditional logic. It would be neat if we could reuse the pipeline condition implementation (though it would require some refactoring). We could let people define the logic either via regular Tasks, or something more specialized like Conditions or maybe even create even more specialized types like Filters. I have a slight preference towards reusing the |
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided.
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided.
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided.
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided.
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine
It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine This PR resolves the issue tektoncd#45.
It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine This PR resolves the issue tektoncd#45.
It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. Updated tektoncd/pipeline to v0.6.0 pipeline Factored out execute trigger & run it as goroutine This PR resolves the issue tektoncd#45.
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from params, serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. PS: This is still a work in progress. Need to test out further. Will do on Tuesday as Monday is holiday.
This PR resolves the issue tektoncd#45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from params, serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. PS: This is still a work in progress. Need to test out further. Will do on Tuesday as Monday is holiday.
This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from params, serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. PS: This is still a work in progress. Need to test out further. Will do on Tuesday as Monday is holiday.
|
Follow up with @iancoffey for documentation of Validation. |
This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources, task will be called alongwith serviceaccount which has github-secret used to create webhook. Assumption: 1. Task is defined in such a way that it can use headers and payload received as params. 2. Apart from params, serviceaccount, payload and headers, task doesn't need anything else. 3. Task gives us non zero exit if validation failed. A sample task and main.go is provided. PS: This is still a work in progress. Need to test out further. Will do on Tuesday as Monday is holiday.
Expected Behavior
Events received by the
EventListeneraddressable service should only be handled if the requester and request are authenticated.We can use GitHub as an initial use case.
From @khrm :
Actual Behavior
Currently, nothing does this.
We have discussed doing this via header matching:
However as mentioned above, more is required than just header matching.
Additional Info
@akihikokuroda included some example code at #78 (comment) to implement this.
The text was updated successfully, but these errors were encountered: