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

Implement Bitbucket Server provider #67

Merged
merged 3 commits into from Dec 13, 2017

Conversation

Projects
None yet
2 participants
@ChrisTitos
Copy link
Contributor

ChrisTitos commented Dec 8, 2017

New Pull Request Checklist

  • Run go fmt on your files (e.g. go fmt ./service/common.go, or on the whole service folder: go fmt ./service/...)
  • Write tests for your code
    • The tests should cover both "success" and "error" cases.
    • The tests should also check all the returned variables, don't ignore any returned value!
    • Ideally the tests should be easily readable, we usually use tests to document our code instead of code comments.
      An example, if you'd write a comment like "Given X this function will return Y" or
      "Beware, if the input is X this function will return Y" then you should implement this as
      a unit test, instead of writing it as a comment.
  • If your Pull Request is more than a bug fix you should also check README.md and change/add the descriptions there - also
    feel free to add yourself as a contributor if you implement support for a new service ;)
  • Before creating the Pull Request you should also run bitrise run test with the Bitrise CLI,
    to perform all the automatic checks (which will run on your Pull Request when you open it).

Summary of Pull Request

To continue with #65 and #66 , I had some time to simply add a dedicated Bitbucket Server provider, as the webhook payloads of Server and Cloud are (still) completely different.
I used the bitbucket-v2 code and tests as a base for this (as I have no prior Go experience).

The events that are supported are:

  • repo:refs_changed , which include pushes and tag creation
  • pr:opened, Pull Request opened
@viktorbenei

This comment has been minimized.

Copy link
Member

viktorbenei commented Dec 9, 2017

Thanks for the PR @ChrisTitos !

Please run the tests locally on your mac, via:

bitrise run test

Linter issues found:

+------------------------------------------------------------------------------+

| (5) Go Lint                                                                  |
+------------------------------------------------------------------------------+
| id: script                                                                   |
| version: 1.1.5                                                               |
| collection: https://github.com/bitrise-io/bitrise-steplib.git                |
| toolkit: bash                                                                |
| time: 2017-12-08T20:34:39Z                                                   |
+------------------------------------------------------------------------------+
|                                                                              |
-> Linting: github.com/bitrise-io/bitrise-webhooks
-> Linting: github.com/bitrise-io/bitrise-webhooks/bitriseapi
-> Linting: github.com/bitrise-io/bitrise-webhooks/config
-> Linting: github.com/bitrise-io/bitrise-webhooks/metrics
-> Linting: github.com/bitrise-io/bitrise-webhooks/service
-> Linting: github.com/bitrise-io/bitrise-webhooks/service/hook
-> Linting: github.com/bitrise-io/bitrise-webhooks/service/hook/assembla
-> Linting: github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver
=> Golint issues found:
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:23:1: comment on exported type PushEventModel should be of the form "PushEventModel ..." (with optional leading article)
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:33:6: exported type ChangeItemModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:34:2: struct field RefId should be RefID
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:41:6: exported type RefModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:42:2: struct field Id should be ID
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:43:2: struct field DisplayId should be DisplayID
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:47:6: exported type UserInfoModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:51:6: exported type RepositoryInfoModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:53:2: struct field Id should be ID
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:60:6: exported type ProjectInfoModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:62:2: struct field Id should be ID
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:68:6: exported type PullRequestInfoModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:81:6: exported type PullRequestEventModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:88:6: exported type PullRequestRefModel should have comment or be unexported
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:89:2: struct field Id should be ID
/bitrise/go/src/github.com/bitrise-io/bitrise-webhooks/service/hook/bitbucketserver/bitbucketserver.go:90:2: struct field DisplayId should be DisplayID
|                                                                              |
+---+---------------------------------------------------------------+----------+
| x | Go Lint (exit code: 1)                                        | 0.75 sec |
+---+---------------------------------------------------------------+----------+
| Issue tracker: https://github.com/bitrise-io/steps-script/issues             |
| Source: https://github.com/bitrise-io/steps-script                           |
+---+---------------------------------------------------------------+----------+

@viktorbenei viktorbenei self-requested a review Dec 9, 2017

@ChrisTitos

This comment has been minimized.

Copy link
Contributor

ChrisTitos commented Dec 9, 2017

Done :)

@viktorbenei

This comment has been minimized.

Copy link
Member

viktorbenei commented Dec 12, 2017

Thanks for the update & sorry for the delay @ChrisTitos ; I'll try to review this ASAP! :)

@viktorbenei
Copy link
Member

viktorbenei left a comment

LGTM, seems like a "good start" ;)

@viktorbenei viktorbenei merged commit 4048ef8 into bitrise-io:master Dec 13, 2017

1 check passed

ci/bitrise/8497e83cdf562ebb/pr Passed - bitrise-webhooks #core
Details
@viktorbenei

This comment has been minimized.

Copy link
Member

viktorbenei commented Dec 13, 2017

Huge thanks @ChrisTitos for the PR! :)

Just curious, did you try to run the server locally on your Mac/PC or on your own Heroku? It helps a lot with debugging :)

@viktorbenei

This comment has been minimized.

Copy link
Member

viktorbenei commented Dec 13, 2017

I also added you to the "Contributors" section, I hope you don't mind (if you would, just let me know!) ;)

https://github.com/bitrise-io/bitrise-webhooks/pull/68/files

@viktorbenei

This comment has been minimized.

Copy link
Member

viktorbenei commented Dec 13, 2017

Update: release went out, it's in production now on https://hooks.bitrise.io/ ;)

@ChrisTitos

This comment has been minimized.

Copy link
Contributor

ChrisTitos commented Dec 18, 2017

Thanks for merging. It seems to be working in our pipeline :)

I first made the test cases, and then I ran the server locally, and manually made the HTTP request with postman to my local server. The difficulty was more finding out what exactly what the Bitbucket payload was for the different event types

@viktorbenei

This comment has been minimized.

Copy link
Member

viktorbenei commented Dec 18, 2017

Got it, thanks for the infos @ChrisTitos ! :)

There's probably only one this which could be improved, the Pull Request handling / build.

Does PR work too? I think it should if the PR is from the same repository (not from a fork), as at https://github.com/bitrise-io/bitrise-webhooks/pull/67/files#diff-aa6666f77f055f632dd806bda0f58f1dR198 you specify the PR ID and two branches, but not the "Pull Request Repository" (

PullRequestRepositoryURL: sourceRepositoryURL,
). This should be enough to do a "pull request build" from a single repo (when both the source & the target are in the same repo, not in a fork) but definitely won't work if the PR is coming from a fork. That said I'm not sure how frequent it is to use forks in a self hosted Bitbucket Server environment.

(Tech note about PR builds: PR builds are meant to test a "pre-merged" code, the state of the code which would be the result if you'd click "merge" on the PR, to provide a more complete test, compared to just simply testing the PRs branch (e.g. feature/a) which might pass tests in itself but might not (in rare cases) after merging it into e.g. "master").

@viktorbenei

This comment has been minimized.

Copy link
Member

viktorbenei commented Dec 18, 2017

In any case huge thanks again @ChrisTitos for your PR & contrib! ;)

erikpoort pushed a commit to erikpoort/bitrise-webhooks that referenced this pull request Jan 10, 2018

Erik Poort
Merge branch 'upstream/master'
* upstream/master:
  Update README.md (bitrise-io#68)
  v1.1.38
  Implement Bitbucket Server provider (bitrise-io#67)
  v1.1.37
  Bitbucket v2: return default value when attempt number header is missing (bitrise-io#66)
  v1.1.36
  README: note about the new 200 http status code in case of build could not be started
  Respond with 200 if build was not triggered of known reason (bitrise-io#64)
  v1.1.35
  migration to new Gopkg.toml based Heroku def (bitrise-io#63)
  v1.1.34
  Update assembla webhook (bitrise-io#62)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment