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

Add azure devops webhook support #401

Merged
merged 3 commits into from
Jan 15, 2024
Merged

Conversation

raulcabello
Copy link
Contributor

@raulcabello raulcabello commented Jan 4, 2024

Add support for Azure DevOps webhook. Azure will notify gitjob when a new commit has been pushed, then a Job will be created with the latest changes. It works the same way as the webhooks for other git providers.

The https://github.com/go-playground/webhooks library we use for parsing the webhooks doesn't provide verification in Azure. I created a PR for adding basic auth verification in Azure. The file from this PR is copied in fleet in the meantime.

refers to rancher/fleet#1998

Process multiple git push events in azure and bitbucket.
@raulcabello raulcabello changed the title [wip] Add azure devops webhook support Add azure devops webhook support Jan 8, 2024
@raulcabello raulcabello marked this pull request as ready for review January 8, 2024 16:19
@raulcabello raulcabello requested a review from a team as a code owner January 8, 2024 16:19
Copy link
Contributor

@weyfonk weyfonk left a comment

Choose a reason for hiding this comment

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

Looking good! With a couple of questions :)

pkg/webhook/webhook.go Outdated Show resolved Hide resolved
pkg/webhook/webhook_test.go Show resolved Hide resolved
Copy link
Contributor

@weyfonk weyfonk left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@@ -0,0 +1,110 @@
// File copied from https://github.com/go-playground/webhooks/blob/master/azuredevops/azuredevops.go
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it could make sense to copy the corresponding test file over as well, just to be on the safe side. How do you see it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we would also need to copy all the testdata files.. https://github.com/go-playground/webhooks/blob/master/azuredevops/azuredevops_test.go#L63

I would avoid copying all those files. Should we leave it without the test or maybe we can just transfer my fork to the fleet org and use the fork instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave it without the test for now; in the meantime, I'll be watching your PR so that we can switch back to using upstream ASAP. Thanks for clarifying :)

@raulcabello raulcabello merged commit 51765b2 into release/fleet/v0.8 Jan 15, 2024
4 checks passed
@raulcabello raulcabello deleted the azure-webhooks-v0.8 branch January 15, 2024 14:22
This was referenced Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants