Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eventsources/bitbucketserver): add OneEventPerChange config option for webhook event handling #3135

Conversation

ryancurrah
Copy link
Contributor

@ryancurrah ryancurrah commented May 17, 2024

Add OneEventPerChange config option to control whether to process each change in a repo:refs_changed webhook event as a separate event. This allows independent processing of each tag or reference update in a single webhook event, useful for triggering distinct workflows in Argo Workflows.

Note: This is an opt-in behavior. It is not a breaking change.

Example Webhook Event with Multiple Changes
{
  "eventKey": "repo:refs_changed",
  "date": "2024-05-15T23:58:18+0000",
  "actor": {
    "name": "ryancurrah",
    "emailAddress": "[email protected]",
    "id": 4127,
    "displayName": "Ryan Currah",
    "active": true,
    "slug": "ryancurrah",
    "type": "NORMAL",
    "links": {
      "self": [
        {
          "href": "https://bitbucket.foobar.com/users/ryancurrah"
        }
      ]
    }
  },
  "repository": {
    "slug": "baz",
    "id": 222,
    "name": "baz",
    "hierarchyId": "12dcc1b4996b9c76cb12",
    "scmId": "git",
    "state": "AVAILABLE",
    "statusMessage": "Available",
    "forkable": true,
    "project": {
      "key": "TEST",
      "id": 6084,
      "name": "TEST",
      "public": false,
      "type": "NORMAL",
      "links": {
        "self": [
          {
            "href": "https://bitbucket.foobar.com/projects/TEST"
          }
        ]
      }
    },
    "public": false,
    "links": {
      "clone": [
        {
          "href": "ssh://[email protected]:7999/test/baz.git",
          "name": "ssh"
        },
        {
          "href": "https://bitbucket.foobar.com/scm/test/baz.git",
          "name": "http"
        }
      ],
      "self": [
        {
          "href": "https://bitbucket.foobar.com/projects/TEST/repos/baz/browse"
        }
      ]
    }
  },
  "changes": [
    {
      "ref": {
        "id": "refs/heads/test-tag-webhooks",
        "displayId": "test-tag-webhooks",
        "type": "BRANCH"
      },
      "refId": "refs/heads/test-tag-webhooks",
      "fromHash": "0000000000000000000000000000000000000000",
      "toHash": "165bb6a516be59cfc3bde5dce04bd206a18b9e7f",
      "type": "ADD"
    },
    {
      "ref": {
        "id": "refs/tags/v0.0.0-1",
        "displayId": "v0.0.0-1",
        "type": "TAG"
      },
      "refId": "refs/tags/v0.0.0-1",
      "fromHash": "0000000000000000000000000000000000000000",
      "toHash": "8b473721c4cad133e9cb3917c7b705d541041cdc",
      "type": "ADD"
    }
  ]
}

The above ☝️ webhook event would be turned into two BitbucketServerEventData events, .changes[] will not have a length greater than one.

Split Up Webhook Event 1 (Shortened for brevity)
{
  "eventKey": "repo:refs_changed",
  "date": "2024-05-15T23:58:18+0000",
  "actor": {
    "name": "ryancurrah",
    "emailAddress": "[email protected]",
    "id": 4127,
    "displayName": "Ryan Currah",
..........................................
    }
  },
  "changes": [
    {
      "ref": {
        "id": "refs/heads/test-tag-webhooks",
        "displayId": "test-tag-webhooks",
        "type": "BRANCH"
      },
      "refId": "refs/heads/test-tag-webhooks",
      "fromHash": "0000000000000000000000000000000000000000",
      "toHash": "165bb6a516be59cfc3bde5dce04bd206a18b9e7f",
      "type": "ADD"
    }
  ]
}
Split Up Webhook Event 2 (Shortened for brevity)
{
  "eventKey": "repo:refs_changed",
  "date": "2024-05-15T23:58:18+0000",
  "actor": {
    "name": "ryancurrah",
    "emailAddress": "[email protected]",
    "id": 4127,
    "displayName": "Ryan Currah",
..........................................
    }
  },
  "changes": [
    {
      "ref": {
        "id": "refs/tags/v0.0.0-1",
        "displayId": "v0.0.0-1",
        "type": "TAG"
      },
      "refId": "refs/tags/v0.0.0-1",
      "fromHash": "0000000000000000000000000000000000000000",
      "toHash": "8b473721c4cad133e9cb3917c7b705d541041cdc",
      "type": "ADD"
    }
  ]
}

Checklist:

@ryancurrah ryancurrah requested a review from whynowy as a code owner May 17, 2024 17:34
@ryancurrah ryancurrah force-pushed the bitbucket-server-support-one-event-per-change branch 2 times, most recently from c5db188 to 6cd8e4f Compare May 17, 2024 22:03
@@ -1051,7 +1051,7 @@ type BitbucketAuth struct {
OAuthToken *corev1.SecretKeySelector `json:"oauthToken,omitempty" protobuf:"bytes,2,opt,name=oauthToken"`
}

// BasicAuth holds the information required to authenticate user via basic auth mechanism
// BitbucketBasicAuth holds the information required to authenticate user via basic auth mechanism
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

// It returns a slice of byte slices containing the processed or original webhook events, and any error encountered during processing.
func webhookHandler(request *http.Request, logger *zap.SugaredLogger, body []byte, bitbucketserverEventSource *v1alpha1.BitbucketServerEventSource, router *Router) ([][]byte, error) {
if request.Header.Get("X-Event-Key") != "repo:refs_changed" {
// Don't continue if this is not a repo:refs_changed webhook event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the same as the previous logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty much the same except I learned that Bitbucket sends the event type/key in a HTTP header named X-Event-Key. Which means we don't have to decode every event now. Making the code easier to read and little more performant.

Screenshot 2024-05-29 at 10 21 33 PM Screenshot 2024-05-29 at 10 23 23 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whynowy does that explanation help?

@ryancurrah ryancurrah force-pushed the bitbucket-server-support-one-event-per-change branch from 340ab36 to be77d2c Compare June 25, 2024 22:47
@ryancurrah ryancurrah force-pushed the bitbucket-server-support-one-event-per-change branch 3 times, most recently from 665ae50 to 14674ae Compare June 26, 2024 19:06
@ryancurrah
Copy link
Contributor Author

Ive been using this change with and without webhooks in production and it is working fine.

@ryancurrah
Copy link
Contributor Author

ryancurrah commented Aug 26, 2024

Hey @whynowy just a note we have been running this change in production for a while now with no issues. Any chance you can review it? This change is what makes Argo project a viable alternative to Jenkins for Bitbucket.

@ryancurrah ryancurrah force-pushed the bitbucket-server-support-one-event-per-change branch 3 times, most recently from f806d73 to 31a4f20 Compare August 26, 2024 23:55
@ryancurrah
Copy link
Contributor Author

@whynowy ???

…on for webhook event handling

Add OneEventPerChange config option to control whether to process each change in a repo:refs_changed webhook event as a separate event. This allows independent processing of each tag or reference update in a single webhook event useful for triggering distinct workflows in Argo Workflows.

Signed-off-by: Ryan Currah <[email protected]>
@ryancurrah ryancurrah force-pushed the bitbucket-server-support-one-event-per-change branch from a37abde to 58f941f Compare September 25, 2024 21:47
Signed-off-by: Ryan Currah <[email protected]>
@ryancurrah ryancurrah force-pushed the bitbucket-server-support-one-event-per-change branch from 2f0f9d9 to cd9c028 Compare September 25, 2024 22:30
Signed-off-by: Ryan Currah <[email protected]>
Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I assume it has been well tested.

@whynowy whynowy merged commit 0d31a3e into argoproj:master Sep 26, 2024
7 checks passed
@ryancurrah
Copy link
Contributor Author

Yeah we have been using this branch for a while now. It will also resolve an issue @tooptoop4 reported.

@tooptoop4
Copy link
Contributor

when is next release? 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants