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

Gitlab integration support #237

Merged
merged 13 commits into from
Apr 5, 2014
Merged

Gitlab integration support #237

merged 13 commits into from
Apr 5, 2014

Conversation

fudanchii
Copy link

This is the preliminary version of GitLab integration

Current features:

  • Drone may use either gitlab.com or self-hosted repo with custom domain
  • Paste GitLab user's private_token to link drone's user with gitlab's.
  • User may change this private_token later (re-link account)
  • Add private or public repository
  • Add deploy key on private repository, use ssh to clone at build.
  • Add post commit hook, and merge_request hook
  • Build on post-commit and merge-request

Known glitch (wip/open for contribution):

  • Re-registering repository may adds duplicated hooks and deploy key
    This may be fixed at go-gitlab-client ( Currently there's no function similar to go-github's CreateUpdate)
  • Building branch with / on its name may be broken,
    Need to encode / to %2F when calling GitLab API
  • merge_request build from forked repository doesn't work, it works if merge request came from the same repository.
    Unlike GitHub, GitLab doesn't provide MR/PR refs at parent repository,
    This should be fetched and merged manually at build which require Drone to also save source repository url from which the MR/PR came from, and this may bump into another problem when the source repository is marked as private. Possible fix is to propose a patch to GitLab to add refs at parent repository for each PR/MR, similar to how GitHub PR works. (I just noticed that we don't have this feature too for bitbucket, so nvm I guess?)
  • Build still trying to trigger notification to GitHub, this should get decoupled.
  • Some other unknown glitches, etc...

@bradrydzewski
I'd like to propose to vendorize go-gitlab-client for now and push back to upstream when things get stable here, or maybe a fork? What do you think?

Any inputs will be much appreciated,
Thanks!

(edit: s/private_key/private_token/g)

GitLab merge request hook payload doesn't provide
any information regarding source commits.

And since Drone currently doesn't support manual fetch from another
remote upstream, merge request hook only supported for maerge request
fro within the same repository.
May parse hook payload only once.
@bradrydzewski
Copy link

Looks like a really solid implementation! Great job!

There was an issue with the build related to the go-gitlab client. Take a look and let me know how to proceed. Thanks!

@fudanchii
Copy link
Author

This should be built with plouc/go-gitlab-client#8 applied.
Related to my proposal above.

To build it manually:

go get github.com/plouc/go-gitlab-client
cd $GOPATH/src/github.com/plouc/go-gitlab-client
git remote add fudanchii https://github.com/fudanchii/go-gitlab-client
git fetch fudanchii
git checkout -b raw-request fudanchii/raw-request
cd $GOPATH/src/github.com/drone/drone
make

Since this is temporary,
I don't think we want to edit our .drone.yml or Makefile to accomodate this. Another reason why I propose to vendorize the package. ;D

@bradrydzewski
Copy link

sorry, I should have read your comments more closely :)

vendoring packages
There are currently three different options: godep, goven and gom. I'm leaning toward gom but haven't had the chance to play around to determine pros and cons. Have you tried gom yet?

merge requests
Bitbucket's implementation is quite new and wasn't as polished as GitHub so I decided to omit the feature for now. I recommend the same for GitLab if it isn't quite ready. I'll leave this up to you, since I don't have much experience with the platform.

We should add issues for any known issues / glitches so that we can track going forward.

Again, great work on this!

@bradrydzewski
Copy link

I also forgot to mention this package manager:
https://github.com/gpmgo/docs/blob/master/en-US/Quickstart.md

… import path.

At least until plouc/go-gitlab-client#8
accepted or Drone officially fork it to `drone/go-gitlab` or
when we have package management which can do something like this:

gom 'github.com/plouc/go-gitlab-client',
  :github => 'fudanchii/go-gitlab-client', :branch => 'raw-request'
@fudanchii
Copy link
Author

I pushed some workaround for this. Refer to my last commit message.
discussion regarding which dependency management to use at #177

@kzaitsev
Copy link

kzaitsev commented Apr 4, 2014

@fudanchii i send small PR, to add support self-signed certificates fudanchii/go-gitlab-client#1

@fudanchii
Copy link
Author

@Bugagazavr It's been merged. Thank you!

bradrydzewski added a commit that referenced this pull request Apr 5, 2014
Gitlab integration support
@bradrydzewski bradrydzewski merged commit 0c9a765 into harness:master Apr 5, 2014
@bradrydzewski
Copy link

Merged! Thanks again and feel free to add issues for any outstanding items

@dosire
Copy link

dosire commented Apr 8, 2014

Thanks for supporting GitLab @fudanchii and @bradrydzewski !

@fudanchii
Copy link
Author

@dosire You're welcome! We've just started, so it might still rough at the edges. :D
It's an effort of the community, this support could not even started without @plouc's go-gitlab-client and awesome contributions from @arrowcircle, @luxifer and all fine folks at #53!

Kudos for all of them!

@luxifer
Copy link

luxifer commented Apr 9, 2014

thanks @fudanchii ;)

@bradrydzewski bradrydzewski added this to the v0.3 milestone Apr 10, 2014
@fudanchii fudanchii deleted the gitlab branch April 23, 2014 17:04
@swistaczek
Copy link

Great work folks!

@cindoum
Copy link

cindoum commented Nov 5, 2014

When will this feature be live? (on https://drone.io/)

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.

None yet

7 participants