triggers icon indicating copy to clipboard operation
triggers copied to clipboard

Add PagerDuty interceptor

Open georgettica opened this issue 2 years ago • 19 comments

Feature request

I would like to have a PagerDuty Interceptor (possibly I will implement it myself if this has enough traction

Use case

As a developer, I want to act on pagerduty webhook actions and run tekton pipelines vai that info. Currently, adding the pagerduty event listener validation is not something easily done via CEL interceptors. I currently mitigated some of the risks I might have with this, but having an interceptor built in for this would make my life much easier

georgettica avatar Apr 03 '22 09:04 georgettica

hi @georgettica looking at the PagerDuty usage docs it does seem like payload validation would be hard with just CEL.

A custom interceptor for PagerDuty sounds good. I'll mention that the interceptor doesn't even have to be part of the Triggers code base if that makes it easier. You can write your own PagerDuty interceptor, install it in any cluster and use it. Eventually we'd like to have a catalog of interceptors that folks could install and use.

dibyom avatar Apr 05 '22 20:04 dibyom

so I will create an interceptor and link it here when it's created. this way once the catalog is created it will be added

georgettica avatar Apr 05 '22 21:04 georgettica

sounds great!

dibyom avatar Apr 08 '22 19:04 dibyom

@dibyom could you help me make this work https://github.com/georgettica/pagerduty-tekton-interceptor

the code looks ok but I am not sure how to invoke this

georgettica avatar Apr 25 '22 16:04 georgettica

hey @georgettica you are on the right track. I'll work on adding better docs for writing cluster interceptors but in the meantime you can take a look at this PR where I add a cluster interceptor for adding the Pull request body for a PR comment event: https://github.com/tektoncd/plumbing/pull/967/files

In particular, you'd need the main HTTP server as in tekton/ci/cluster-interceptors/add-pr-body/cmd/interceptor/main.go Also take a look at the test file to see how to unit test it https://github.com/tektoncd/plumbing/blob/4b254e24a51319ca3d3962d4472eb19a32b2bd06/tekton/ci/cluster-interceptors/add-pr-body/pkg/pr_test.go

To test it manually, you'd start the server and send it a HTTP request whose body would be a JSON of the InterceptorRequest struct.

dibyom avatar Apr 26 '22 23:04 dibyom

And to add this I will just add the custom interceptor? I'll take a look at you suggestions tomorrow

georgettica avatar Apr 27 '22 00:04 georgettica

And to add this I will just add the custom interceptor?

So, the PR I mentioned adds another cluster interceptor called add-pr-body so I thought it would be a helpful reference as you are writing your own pagerduty one. At its core, its just a HTTP server that receives requests in the InterceptorRequest format and responds with a InterceptorResponse.

dibyom avatar Apr 27 '22 16:04 dibyom

@dibyom Reading this does make me wonder about adding a simple hmac('sha256', body, '', 'secret-name', 'namespace') type function to the CEL library which would make it fairly simple to handle signed requests from a upstreams.

I have a use for this for another project...

bigkevmcd avatar May 18 '22 03:05 bigkevmcd

If that wouy exist, I could write the whole interceptor by this

georgettica avatar May 18 '22 05:05 georgettica

@bigkevmcd yeah I think that would be a good addition!

dibyom avatar May 19 '22 09:05 dibyom

I don't know if an implementation in CEL is really that useful. I spotted two major vendors with webhook signatures so far:

  • PagerDuty: https://developer.pagerduty.com/docs/ZG9jOjExMDI5NTkz-verifying-signatures
  • Datatrans (a big swiss payment provider): https://api-reference.datatrans.ch/#section/Webhook

The hooks are both very different and I would be afraid that a hmac('sha256', body, '', 'secret-name', 'namespace') function would only work for one specific vendor.

Also, for pagerduty you have to remove the v1= prefix + loop through multiple signatures and return true if one of them matches.

For Datatrans for instance the whole mechanism is already different. Instead of getting multiple signatures you get one signature and a timestamp and you have to use both for signature validation.

I am very confident that if you find a third vendor with signatures in webhooks the third vendor may have a different mechanism.

shibumi avatar May 19 '22 10:05 shibumi

Please TAL again at the repo, the code works with local testing

georgettica avatar May 26 '22 22:05 georgettica

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. 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 with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

tekton-robot avatar Aug 24 '22 23:08 tekton-robot

/remove-lifecycle stale Please TAL as I don't want this issue to die

georgettica avatar Aug 24 '22 23:08 georgettica

hi @georgettica sorry for the delay - are you looking for a review of https://github.com/georgettica/pagerduty-tekton-interceptor or are you looking to get that merged into the triggers source tree?

dibyom avatar Aug 25 '22 20:08 dibyom

First a review to see if this is good. Second help to merge into this repo

georgettica avatar Aug 26 '22 10:08 georgettica

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. 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 with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

tekton-robot avatar Nov 24 '22 11:11 tekton-robot

/lifecycle frozen

As I am looking for someone to look at my code and I think adding this to upstream might be useful for some people

georgettica avatar Dec 03 '22 17:12 georgettica

/area roadmap

vdemeester avatar Feb 15 '23 15:02 vdemeester