feat(bitbucketserver): multiple improvements to bitbucketserver event source #2921

ryancurrah · 2023-11-26T19:14:04Z

There are no breaking changes in the pull request, the event source will behave the same by default. One bigger internal change is that the Bitbucket Server client is now created once, set on the Route struct.

This Pull Request contains 3 improvements to the Bitbucket Server Event source.

Customizable Apply Webhook Check Interval

Currently the apply webhook check runs every 60 seconds which could be to often. This is now configurable.

Manage Webhooks for Entire Projects

When a list of Bitbucket Server Projects are provided the event source will ensure a webhook exists in all repositories in the provided project.

Ability to skip Refs Changed event type when a Pull Request is Open for the same commit.

This feature aims to reduce redundant workflow triggers caused by Bitbucket's event handling. Specifically, when a Pull Request is opened and a new commit is pushed, Bitbucket sends two events: repo:refs_changed and either pr:from_ref_created (for a new PR) or pr:from_ref_updated (for an existing PR). This results in multiple workflows being launched for both Branch and Pull Request events.

To address this, the Pull Request adds a boolean option named skipBranchRefsChangedOnOpenPR. This option, when enabled, performs a check to determine if a Pull Request is open for a specific branch commit associated with a repo:refs_changed event. If a PR is open for that commit, the new feature will skip sending the repo:refs_changed event. This effectively prevents the triggering of multiple workflows for the same commit in the scenario where a Pull Request is opened.

ryancurrah · 2023-11-27T17:11:46Z

Testing these changes in our environment. When they have baked for a little bit I will change this PR to Ready for Review.

whynowy

@daniel-codefresh - could you take a look?

pkg/apis/eventsource/v1alpha1/types.go

ryancurrah · 2023-12-01T15:42:24Z

Fixed incorrect protobuf struct tag options 37958b8.

Though these types should be auto generated, not doing so makes modifications error prone.

ryancurrah · 2023-12-15T22:17:21Z

@daniel-codefresh this is ready for review.

ryancurrah · 2024-01-04T20:06:50Z

Hey @daniel-codefresh I hope you had a great holidays and happy new year. I am wondering if you would be able to review this?

ryancurrah · 2024-01-16T15:32:48Z

Is anyone available to review this @whynowy?

whynowy · 2024-01-27T05:56:06Z

pkg/apis/eventsource/v1alpha1/webhook_context.go

@@ -32,6 +32,9 @@ type WebhookContext struct {
 	// Default value: 1048576 (1MB).
 	// +optional
 	MaxPayloadSize *int64 `json:"maxPayloadSize,omitempty" protobuf:"bytes,9,opt,name=maxPayloadSize"`
+	// CheckInterval is a duration in which to wait before checking that the webhooks exist, e.g. 1s, 30m, 2h... (defaults to 1m)
+	// +optional
+	CheckInterval string `json:"checkInterval" protobuf:"bytes,10,opt,name=checkInterval"`


Use *metav1.Duration.

I tried it but I get this error when running make codegen.

The swagger spec at "api/openapi-spec/swagger.json" is invalid against swagger specification 2.0. See errors below: - some references could not be resolved in spec. First found: object has no key "io.k8s.apimachinery.pkg.apis.meta.v1.Duration"

I was copying an existing duration example from here, as I did not see any uses of a Duration type, probably for this reason maybe?

argo-events/pkg/apis/eventsource/v1alpha1/types.go

Line 1134 in 6414f3b

CheckInterval string `json:"checkInterval,omitempty" protobuf:"bytes,3,opt,name=checkInterval"`

We can fix this later.

Just realized it's added to the generic WebhookContext, which is used by many event sources. This is not right, please move to bigbuckets specific types.

Ack, I will move this.

whynowy · 2024-01-27T05:59:54Z

eventsources/sources/bitbucketserver/start.go

-		if err != nil {
-			return fmt.Errorf("failed to get bitbucketserver token. err: %w", err)
-		}
+	if !bitbucketserverEventSource.DeleteHookOnFinish && len(router.hookIDs) == 0 {


Logic changed?

Returning early to make the code more readable by removing a level of nesting. I should split this up into two if statements though.

This is done. ✅

whynowy · 2024-01-27T06:05:39Z

eventsources/sources/bitbucketserver/start.go

+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	bitbucketClient := newBitbucketServerClient(ctx, bitbucketConfig, bitbucketToken)


The ctx will be cancelled when this function exits, will the bitbucketConfig still be able to used for delete the webhook?

Good catch. I created a second client that will be used by PostInactivate. I also ensured that the customClient uses the ctx.

In this case, I don't think this is a good approach, maybe just extract the way to get a client to a function?

Your instructions aren't clear here. I will take a second look though.

Looking at this again, I feel this is the right approach using a separate instance of the bitbucket client for deleting which does not use the canceled context and adding the bitbucket clients to the Router struct which can be accessed by the different functions is a common approach to using clients.

ryancurrah

Changes pushed and responded to comments. Thanks for reviewing.

ryancurrah · 2024-01-27T15:34:02Z

eventsources/sources/bitbucketserver/start.go

-		if err != nil {
-			return fmt.Errorf("failed to get bitbucketserver token. err: %w", err)
-		}
+	if !bitbucketserverEventSource.DeleteHookOnFinish && len(router.hookIDs) == 0 {


Returning early to make the code more readable by removing a level of nesting. I should split this up into two if statements though.

ryancurrah · 2024-01-27T16:10:32Z

eventsources/sources/bitbucketserver/start.go

+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	bitbucketClient := newBitbucketServerClient(ctx, bitbucketConfig, bitbucketToken)


Good catch. I created a second client that will be used by PostInactivate. I also ensured that the customClient uses the ctx.

ryancurrah · 2024-01-27T16:21:45Z

pkg/apis/eventsource/v1alpha1/webhook_context.go

@@ -32,6 +32,9 @@ type WebhookContext struct {
 	// Default value: 1048576 (1MB).
 	// +optional
 	MaxPayloadSize *int64 `json:"maxPayloadSize,omitempty" protobuf:"bytes,9,opt,name=maxPayloadSize"`
+	// CheckInterval is a duration in which to wait before checking that the webhooks exist, e.g. 1s, 30m, 2h... (defaults to 1m)
+	// +optional
+	CheckInterval string `json:"checkInterval" protobuf:"bytes,10,opt,name=checkInterval"`


I tried it but I get this error when running make codegen.

The swagger spec at "api/openapi-spec/swagger.json" is invalid against swagger specification 2.0. See errors below: - some references could not be resolved in spec. First found: object has no key "io.k8s.apimachinery.pkg.apis.meta.v1.Duration"

I was copying an existing duration example from here, as I did not see any uses of a Duration type, probably for this reason maybe?

argo-events/pkg/apis/eventsource/v1alpha1/types.go

Line 1134 in 6414f3b

CheckInterval string `json:"checkInterval,omitempty" protobuf:"bytes,3,opt,name=checkInterval"`

ryancurrah · 2024-01-31T19:19:54Z

@whynowy this is ready for review again.

whynowy · 2024-02-03T07:28:38Z

pkg/apis/eventsource/v1alpha1/webhook_context.go

@@ -32,6 +32,9 @@ type WebhookContext struct {
 	// Default value: 1048576 (1MB).
 	// +optional
 	MaxPayloadSize *int64 `json:"maxPayloadSize,omitempty" protobuf:"bytes,9,opt,name=maxPayloadSize"`
+	// CheckInterval is a duration in which to wait before checking that the webhooks exist, e.g. 1s, 30m, 2h... (defaults to 1m)
+	// +optional
+	CheckInterval string `json:"checkInterval" protobuf:"bytes,10,opt,name=checkInterval"`


We can fix this later.

Just realized it's added to the generic WebhookContext, which is used by many event sources. This is not right, please move to bigbuckets specific types.

whynowy · 2024-02-03T07:36:49Z

eventsources/sources/bitbucketserver/start.go

@@ -93,6 +98,40 @@ func (router *Router) HandleRoute(writer http.ResponseWriter, request *http.Requ
 		return
 	}

+	// When SkipBranchRefsChangedOnOpenPR is enabled and webhook event type is repo:refs_changed,


Why is there so specific logic hardcoded?

If there's a requirement like you mentioned in the PR description, instead of doing this, you can use filter feature in the Sensor take the events you are interested.

I met with Pipekit folks about this and we determined that a filter would not work, because of the API call and self-signed certificate required. This addition goes a long way for Argo to replace the multibranch pipeline in Jenkins, also note that this behaviour is opt-in not opt-out so they have to specifically enable it.

whynowy · 2024-02-03T07:44:05Z

eventsources/sources/bitbucketserver/start.go

+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	bitbucketClient := newBitbucketServerClient(ctx, bitbucketConfig, bitbucketToken)


In this case, I don't think this is a good approach, maybe just extract the way to get a client to a function?

whynowy

Please fix the conflict.

ryancurrah · 2024-02-19T15:52:01Z

Please fix the conflict.

Done and also squashed commits for a clean merge.

ryancurrah · 2024-02-22T02:55:31Z

@whynowy hoping this gets merged before another conflict in go.mod.

whynowy · 2024-02-25T05:56:36Z

Please fix the conflict.

Done and also squashed commits for a clean merge.

Sorry, you have to do it again... Ping me on slack after you do it.

…t source This Pull Request contains 3 improvements to the Bitbucket Server Event source. 1. Customizable Apply Webhook Check Interval Currently the apply webhook check runs every 60 seconds which could be to often. This is now configurable. 2. Manage Webhooks for Entire Projects When a list of Bitbucket Server Projects are provided the event source will ensure a webhook exists in all repositories in the provided project. 3. Ability to Skip Refs Changed when a Pull Request is Open A new option has been added that adds a check when a wehook is received, when a repo:refs_changed event is received a check will be ran to skip publishing the event if a Pull Request is opened. This is to get around the issue that Bitbucket will send this event even when a Pull Request is opened and there is already a pr:from_ref_updated event that would launch multiple workflow. This allows us to have a branch, pull request and tag workflows. Signed-off-by: Ryan Currah <[email protected]>

tooptoop4 · 2024-03-16T01:46:44Z

when dis bad boy gonna be released?

… source (#2921) Signed-off-by: Derek Wang <[email protected]>

ryancurrah changed the title ~~feat(bitbucketserver): multiple improvements to bitbucket server even…~~ feat(bitbucketserver): multiple improvements to bitbucketserver event source Nov 26, 2023

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch 4 times, most recently from 299a80a to 461fbe8 Compare November 27, 2023 05:21

whynowy reviewed Dec 1, 2023

View reviewed changes

pkg/apis/eventsource/v1alpha1/types.go Outdated Show resolved Hide resolved

pkg/apis/eventsource/v1alpha1/types.go Outdated Show resolved Hide resolved

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch from 49094db to 37958b8 Compare December 1, 2023 15:40

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch from 132bc9c to 964d305 Compare December 15, 2023 21:30

ryancurrah marked this pull request as ready for review December 15, 2023 21:31

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch from 964d305 to 818996b Compare December 15, 2023 21:32

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch from 14187f8 to a81564e Compare January 16, 2024 14:26

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch 4 times, most recently from 5843a9c to b0ed545 Compare January 20, 2024 15:06

whynowy reviewed Jan 27, 2024

View reviewed changes

ryancurrah commented Jan 27, 2024

View reviewed changes

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch 2 times, most recently from 2f216ee to fa58b2d Compare January 27, 2024 16:49

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch from fa58b2d to b8338ad Compare February 1, 2024 18:58

whynowy reviewed Feb 3, 2024

View reviewed changes

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch 2 times, most recently from 4a7608b to 7886c94 Compare February 16, 2024 18:44

whynowy approved these changes Feb 19, 2024

View reviewed changes

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch 2 times, most recently from c53f9d4 to 63d9a44 Compare February 19, 2024 15:43

ryancurrah force-pushed the multiple-improvements-to-the-bitbucket-server-eventsource branch from 63d9a44 to 58fa25b Compare February 25, 2024 17:31

whynowy merged commit 8cbbbef into argoproj:master Feb 25, 2024
8 checks passed

ryancurrah deleted the multiple-improvements-to-the-bitbucket-server-eventsource branch February 25, 2024 17:53

whynowy pushed a commit that referenced this pull request Jun 14, 2024

feat(bitbucketserver): multiple improvements to bitbucketserver event…

39cd54f

… source (#2921) Signed-off-by: Derek Wang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bitbucketserver): multiple improvements to bitbucketserver event source #2921

feat(bitbucketserver): multiple improvements to bitbucketserver event source #2921

ryancurrah commented Nov 26, 2023 •

edited

Loading

ryancurrah commented Nov 27, 2023

whynowy left a comment

ryancurrah commented Dec 1, 2023 •

edited

Loading

ryancurrah commented Dec 15, 2023

ryancurrah commented Jan 4, 2024

ryancurrah commented Jan 16, 2024

whynowy Jan 27, 2024

ryancurrah Jan 27, 2024

whynowy Feb 3, 2024

ryancurrah Feb 3, 2024

whynowy Jan 27, 2024

ryancurrah Jan 27, 2024

ryancurrah Feb 16, 2024

whynowy Jan 27, 2024

ryancurrah Jan 27, 2024

whynowy Feb 3, 2024

ryancurrah Feb 3, 2024

ryancurrah Feb 16, 2024

ryancurrah left a comment

ryancurrah Jan 27, 2024

ryancurrah Jan 27, 2024

ryancurrah Jan 27, 2024

ryancurrah commented Jan 31, 2024

whynowy Feb 3, 2024

whynowy Feb 3, 2024

ryancurrah Feb 3, 2024 •

edited

Loading

whynowy Feb 3, 2024

whynowy left a comment

ryancurrah commented Feb 19, 2024

ryancurrah commented Feb 22, 2024

whynowy commented Feb 25, 2024

tooptoop4 commented Mar 16, 2024

feat(bitbucketserver): multiple improvements to bitbucketserver event source #2921

feat(bitbucketserver): multiple improvements to bitbucketserver event source #2921

Conversation

ryancurrah commented Nov 26, 2023 • edited Loading

ryancurrah commented Nov 27, 2023

whynowy left a comment

Choose a reason for hiding this comment

ryancurrah commented Dec 1, 2023 • edited Loading

ryancurrah commented Dec 15, 2023

ryancurrah commented Jan 4, 2024

ryancurrah commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryancurrah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryancurrah commented Jan 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryancurrah Feb 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whynowy left a comment

Choose a reason for hiding this comment

ryancurrah commented Feb 19, 2024

ryancurrah commented Feb 22, 2024

whynowy commented Feb 25, 2024

tooptoop4 commented Mar 16, 2024

ryancurrah commented Nov 26, 2023 •

edited

Loading

ryancurrah commented Dec 1, 2023 •

edited

Loading

ryancurrah Feb 3, 2024 •

edited

Loading