Reimplement the add-pr-body interceptor as a ClusterInterceptor #967
Conversation
|
/kind feature |
Old style interceptors won't work with trigger groups. Until tektoncd#967 is merged, leave the add-pr-body interceptor out of the trigger group. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>
Old style interceptors won't work with trigger groups. Until tektoncd#967 is merged, leave the add-pr-body interceptor out of the trigger group. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>
Old style interceptors won't work with trigger groups. Until #967 is merged, leave the add-pr-body interceptor out of the trigger group. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>
|
Hi @dibyom, thank you for starting this! Are you still working on it? I could try and pick up your PR if you don't have time to work on it anymore. Thank you! |
hey @afrittoli almost done, just need to test + update the docs. I'll update this PR in the next couple of days. |
|
/test plumbing-yamllint |
|
@dibyom Any update on this one? Anything I can do to help? |
@afrittoli Sorry for the major delay. Should be ready for a look now 🙏 |
There was a problem hiding this comment.
Thanks for this. I a couple of comments on the docs but mostly looks great!
The existing interceptor has nightly builds, and I wonder if we should replace it with the new one instead of having two in the same folder, which will make the nightly builds harder to manage. Alternatively we could have two separate folders... wdyt?
| interceptors: | ||
| - cel: | ||
| filter: >- // TODO: This can be part of the interceptor itself | ||
| body.action == 'created' && | ||
| in('pull_request', body.issue) && | ||
| && body.issue.state == 'open' && | ||
| body.comment.body.matches('^/test($| [^ ]*$)') | ||
| overlays: | ||
| - key: add-pr-body.pull-request-url | ||
| expression: "body.issue.pull_request.url" |
There was a problem hiding this comment.
I think the new syntax looks something like:
interceptors:
- name: "Filter the GitHub org and actions and add git_clone_depth"
ref:
name: "cel"Should the add-pr-body interceptor be used in this example?
|
Thanks, I think I'm going to move this to a separate folder then to make it easier. |
b0ab941 to
5ad1b8d
Compare
This commit adds a cluster interceptor flavor of the add pr body interceptor. The old webhook interceptor has been left untouched. BREAKING: For the ClusterInterceptor flavor, the PR url input has to now come in via the `request.extensions ` field instead of the old `request.body` field. Similarly, the output pr-body is part of the extensions field instead of being merged back into the body. Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
|
@afrittoli I updated the PR. I also added a couple of TODOs around making this easier to use. Also, we could potentially merge this code with the packaged GitHub interceptor tektoncd/triggers#1339 |
There was a problem hiding this comment.
Thanks a lot @dibyom for this!
I really like the idea to embed this as opt-in in the cluster interceptor.
If no-one does that before I will start testing this on dogfooding once I'm back from PTO.
I will try to repeat the process for the other interceptor as well - and for that one too I wonder if that's something we could get into core interceptors as well, as it's a pretty common use case to filter based on the owner of the PR.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afrittoli 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:
Approvers can indicate their approval by writing |
|
/lgtm |
Changes
This commit adds a cluster interceptor flavor of the add pr body interceptor.
The old webhook interceptor has been left untouched.
BREAKING: For the ClusterInterceptor flavor, the PR url input has to now come
in via the
request.extensionsfield instead of the oldrequest.bodyfield.Similarly, the output pr-body is part of the extensions field instead of being
merged back into the body.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide
for more details.