Modify BitBucketInterceptor to BitbucketInterceptor #596
Conversation
tekton-robot
added
the
size/M
savitaashture
changed the title
docs/eventlisteners.md
Outdated
| @@ -421,7 +421,7 @@ spec: | |||
| ### Bitbucket Interceptors | |||
|
|
|||
| Bitbucket Interceptors contain logic to validate and filter webhooks that come from | |||
| Bitbucket server or cloud. Supported features include validating webhooks actually came from Bitbucket as well as | |||
| Bitbucket server and not the Bitbucket cloud. Supported features include validating webhooks actually come from Bitbucket server as well as | |||
There was a problem hiding this comment.
How about something like...
The Bitbucket interceptor provides support for hooks originating in Bitbucket server, providing server hook signature validation and event-filtering.
Bitbucket cloud is not currently supported by this interceptor, but it has no secret validation, so you could match on the incoming requests using the CEL interceptor.
There was a problem hiding this comment.
question: what would it take to support bitbucket cloud?
There was a problem hiding this comment.
Bitbucket cloud interceptor doesn't support secret validation, but it could match on the event types.
Couple of options, maybe an extra parameter for the interceptor to indicate cloud or server?
Or try one and then the other, but the JSON bodies are actually quite different between them.
(This is "solved" by my hook standardisation proposal 🙂)
There was a problem hiding this comment.
Oops, lemme clarify, Bitbucket cloud doesn't send secrets to validate.
There was a problem hiding this comment.
https://confluence.atlassian.com/bitbucket/manage-webhooks-735643732.html
Secure webhooks
Webhooks work Bitbucket sending a POST to the customer service at a particular URL.
It's important that you protect your service from potential attacks. As a workaround, you
could whitelist the Bitbucket IPs to restrict who can access your webhook resource.
So, maybe instead of secrets, for the "cloud" version, you add the list of IPs?
There was a problem hiding this comment.
oh ok, did not realize cloud did not support shared secret validation. IP whitelisting might get a bit complex for scenarios where there are other entities like reverse proxies or knative eventing processing the request before the EventListener. Maybe just the types are fine for now until the standardized hook type?
There was a problem hiding this comment.
How about something like...
The Bitbucket interceptor provides support for hooks originating in Bitbucket server, providing server hook signature validation and event-filtering.
Bitbucket cloud is not currently supported by this interceptor, but it has no secret validation, so you could match on the incoming requests using the CEL interceptor.
@bigkevmcd is that okay if we include work hook right now in the doc because it may lead to confusion on what can be hook so just wanted to clarify before updating PR
There was a problem hiding this comment.
oh ok, did not realize cloud did not support shared secret validation. IP whitelisting might get a bit complex for scenarios where there are other entities like reverse proxies or knative eventing processing the request before the EventListener. Maybe just the types are fine for now until the standardized hook type?
@dibyom Maybe just the types are fine for now until the standardized hook type? Could you please help me to understand what will be type here
There was a problem hiding this comment.
I was just referring to the eventTypes field. Going through this again -- I think adding @bigkevmcd 's suggested change seems like a good way to proceed here!
There was a problem hiding this comment.
@dibyom Updated README as per the @bigkevmcd suggestion
savitaashture
force-pushed
the
bitbucketupdate
branch
from
68c073d
to
045b21b
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom 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 |
tekton-robot
added
the
approved
|
/lgtm |
Changes
atlassiandocSubmitter 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.
Fixes: #362