GitLab service interface #1159

kzaitsev · 2015-08-22T04:25:09Z

Allow to work with gitlab as service interface

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1184

kzaitsev · 2015-08-22T19:11:22Z

Currently gitlab not support it, but if gitlab merge my MR, this can be merged too

bradrydzewski · 2015-08-22T23:07:00Z

@Bugagazavr cool. In the mean time, we should capture enough data in the Build object to clone without merge ref. This is what the Build object looks like when we receive a pull request from GitHub which includes enough do manually clone origin and upstream ...

{
  "ID": 10,
  "number": 3,
  "status": "success",
  "started_at": 1440022534,
  "finished_at": 0,
  "head_commit": {
    "sha": "ededa54d3968051ef18ae3bebe2903bd1bb061d0",
    "ref": "refs\/pull\/1\/merge",
    "branch": "fix\/clone-into-non-empty-dir",
    "timestamp": "2015-08-19 22:04:07.457360262 +0000 UTC",
    "remote": "https:\/\/github.com\/donny-dont\/drone-git.git",
    "author": {
      "login": "donny-dont"
    }
  },
  "pull_request": {
    "number": 1,
    "title": "Use git init instead of git clone",
    "base_commit": {
      "sha": "7138816569c347afef5f62d899a1677b59037fd6",
      "ref": "master",
      "remote": "https:\/\/github.com\/drone-plugins\/drone-git.git",
      "author": { }
    }
  }
}

just wanted to throw out as an option in case it takes a while for your pull request to be accepted by GitLab. Also, we'll need similar logic for Stash, Bitbucket, etc anyway.

kzaitsev · 2015-08-22T23:18:45Z

How you want to clone source repository without ref pathspec?

bradrydzewski · 2015-08-24T03:17:30Z

using the above json example, we could do something like this:

git fetch origin # this would pull from drone/drone
git fetch https://github.com/donny-dont/drone-git.git fix/clone-into-non-empty-dir
git checkout FETCH_HEAD

I'm sure we can refine the above to even clone the specific sha. Everything we need to clone the pull request should be in the payload. If you look at the sample json you'll see the head_commit for a pull request contains most of the relevant data, including the remote url, branch and sha.

The ref doesn't have to be refs/pull/1/merge. That is something we do specifically for GitHub. We don't have to use the same approach for other remote systems. For Bitbucket, we'll need to use the actual ref from the merge request commit

kzaitsev · 2015-08-24T03:23:55Z

Github, Gitlab, Stash support refs, Bitbucket - not, Your case works only for public repos, but not for:

Private -> Private
Private -> Public

Ofc if you use netrc, maybe you can have access to some private repositories, but maybe not, there is no guarantee that source repo will be accessible.

And so Gitlab doesn't present head commit, only source.

bradrydzewski · 2015-08-24T03:36:22Z

Github, Gitlab, Stash support refs, Bitbucket - not

Sorry, I think I misunderstood. I thought that refs weren't supported in GitLab until your pull request was merged

bradrydzewski · 2015-08-24T03:38:51Z

I would, however, like to split this into two separate pull requests. Let's merge the gitlab.go changes right away to enable pull request functionality. I would like to understand and discuss some of the other REST and database-related changes

kzaitsev · 2015-08-24T04:09:23Z

Sorry, I think I misunderstood. I thought that refs weren't supported in GitLab until your pull request was merged

Since 7.14.0 supported, if my pull request has been merged:

Drone can activate self as gitlab service via gitlab API
Drone no need add deploy key and add manually hook
Drone can clone repository with repository token as drone-ci-token:drone-repo-token

Let's merge the gitlab.go changes right away to enable pull request functionality. I would like to understand and discuss some of the other REST and database-related changes

thats a breaking functionality, after merge this we need reactivate, all builds to get it works

Database changes:

added a sql queries to get last build from pull request number
added a sql query to get build from sha and branch.

REST changes:

added method for gitlab, to get current build status protected by repository token

/api/repos/:owner/:name/pr/:number

added method, to got a redirect to build page from pull request number

/redirect/:owner/:name/pr/:number

added method, to get a redirect to build page from sha and branch

/redirect/:owner/:name/pr/:number

Remote changes:

Removed hook and deploy key manipulations
Added service API instead hook and deploy key ( faster, easy )
Added support for push tag ( requires reactivation, because by default in 0.4.0 disabled ) and merge request events
Gitlab remote tests

I have a plan come to september ( from 1st to 7th ) merge window, if not, i create a pull request to drone with only push tags and pull requests support

bradrydzewski · 2015-08-24T05:07:32Z

pkg/store/builtin/build_sql.go

@@ -596,6 +596,79 @@ WHERE build_repo_id = ?
 AND build_number = ?
 `

+const stmtBuildSelectPullRequestNumber = `
+SELECT


since the build_sql.go is auto-generated, let's move these non-generated queries to build.go so they don't overwritten. I haven't published the sql generator yet, but I'll try to do that next week :)

also, I will probably adjust some of the non-generated queries I wrote to use the following:

const stmtBuildSelectPullRequestNumber = stmtBuildSelectList + ` WHERE build_repo_id = ? AND build_pull_request_number = ? ORDER BY build_number DESC LIMIT 1 `

... the benefit to the above is that we don't have to worry about column changes in our non-generated queries

bradrydzewski · 2015-08-24T05:26:22Z

Thanks for the explanation, sorry I didn't mention this previously, but there is a ton of great work here! I've added some (hopefully) minor notes in the code review section. Once those are complete we are good to merge. Also, you get bonus points for porting the unit tests back over to the 0.4 branch :)

I'll take care of updating the swagger file to include the new endpoints once this is complete. It is a pain in the ass to update, and I don't want to punish you after submitting such a nice patch :)

kzaitsev · 2015-08-29T22:27:05Z

@bradrydzewski so i have finally complete this PR.

but @DouweM requests to merge this PR before gitlab merge own ( https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1215 ), this breaks current compatibility, and can be used after september 22 gitlab 8.0 release. Need to add description about this and create current drone snapshot if someone wanna to play with drone and gitlab less then 8.0 (:

DouweM · 2015-08-29T22:55:43Z

pkg/remote/builtin/gitlab/gitlab.go

+ hook.Commit.Sha = parsed.ObjectAttributes.LastCommit.Id
+ hook.Commit.Remote = parsed.ObjectAttributes.Source.HttpUrl
+
+ if parsed.ObjectAttributes.Source.HttpUrl == parsed.ObjectAttributes.Target.HttpUrl {


It's probably safer to compare source_project_id and target_project_id.

yep, it most simple compare then:

itarget.Name == source.Name && target.Namespace == source.Namespace

@Bugagazavr 👍

DouweM · 2015-08-29T23:11:29Z

@Bugagazavr https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/1215 implements access over HTTP with drone-ci-token/<token> as username/password, but I don't see that used anywhere here. What am I missing?

kzaitsev · 2015-08-29T23:16:48Z

@DouweM you use incorrect basic auth look this:

#git clone http:https://drone-ci-token:121212@localhost:3000/gitlab-org/gitlab-ci.git
Cloning into 'gitlab-ci'...
remote: Counting objects: 9723, done.
remote: Compressing objects: 100% (3570/3570), done.
remote: Total 9723 (delta 5862), reused 9717 (delta 5859)
Receiving objects: 100% (9723/9723), 2.50 MiB | 0 bytes/s, done.
Resolving deltas: 100% (5862/5862), done.
Checking connectivity... done.

DouweM · 2015-08-29T23:17:39Z

pkg/remote/builtin/gitlab/gitlab.go

+ hook.Repo.Owner = req.FormValue("owner")
+ hook.Repo.Name = req.FormValue("name")
+ hook.Repo.FullName = fmt.Sprintf("%s/%s", hook.Repo.Owner, hook.Repo.Name)
+ hook.Repo.Link = re.ReplaceAllString(parsed.ObjectAttributes.Target.HttpUrl, "$1")


If you want, you can add web_url to the project's hook_attrs in your MR. The method already exists and it's already returned by the API. It's a little cleaner than doing this replacement here :)

looks like hook does not contains web_url key

Yeah, I was suggesting that you can add it to the GitLab hook_attrs method on Project, so that you can use it here.

DouweM · 2015-08-29T23:19:45Z

@Bugagazavr I understand how Basic Auth works, but I don't see the drone-ci-token username used anywhere in this PR?

kzaitsev · 2015-08-29T23:22:20Z

@DouweM in this PR no, because it works on Netrc method and we need to pass repository data to netrc method ( it requires a change github remote plugin, because used in one interface ), this pull request is too large i planed do this after this merged.

DouweM · 2015-08-29T23:23:42Z

@Bugagazavr All right, I hope you'll get everything ready and merged into Drone before GitLab 8.0 is released :)

kzaitsev · 2015-08-30T00:40:28Z

@DouweM thanks for the review, I have tried to fulfill all your wishes (:

so, i have find a bug in drone model, i have plan to cover drone model tomorrow, now i send minor fixes to gitlab

kzaitsev · 2015-09-01T10:52:34Z

@bradrydzewski hi brad, any news, on this?

bradrydzewski · 2015-09-02T01:50:44Z

@Bugagazavr sorry for the delay on my end. The PR looks awesome. I do have 1 final request, and I promise we can merge :)

I'd like to take the gitlab-specific routes and wrap them in a /gitlab router group. This is similar to what the Jenkins implementation does as well. For example:

    gitlab := r.Group("/gitlab")
    {
        // all gitlab-specific routes here
    }

I'd also like to move all GitLab routes into a server/gitlab.go file. The primary reason is that I'm worried that myself (or someone else) won't know what these routes are for, and will end up changing or removing them. By wrapping in a /gitlab router group, and by putting in a server/gitlab.go file it will help caution anyone from changing.

I hope that makes sense, and that it isn't too big of a change. Thanks again!

kzaitsev · 2015-09-02T02:19:44Z

@bradrydzewski done

bradrydzewski · 2015-09-02T02:26:10Z

awesome, thanks!!!!!

GitLab service interface

bradrydzewski reviewed Aug 24, 2015
View reviewed changes

kzaitsev added 5 commits August 30, 2015 00:33

GitLab service interface

fd98e26

FIxes tests

7716c70

Gitlab testing

00284dd

Added push tag event support

ce50e0f

Additional fixes

6854d13

kzaitsev force-pushed the gitlab-service branch from 7b4e480 to 6854d13 Compare August 29, 2015 22:19

kzaitsev changed the title ~~[WIP] GitLab service interface~~ GitLab service interface Aug 29, 2015

DouweM reviewed Aug 29, 2015
View reviewed changes

Some optimizations

9ded21d

Move gitlab routes to gitlab group

654dd31

bradrydzewski added a commit that referenced this pull request Sep 2, 2015

Merge pull request #1159 from Bugagazavr/gitlab-service

cc86f52

GitLab service interface

bradrydzewski merged commit cc86f52 into harness:0.4.0 Sep 2, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitLab service interface #1159

GitLab service interface #1159

kzaitsev commented Aug 22, 2015

kzaitsev commented Aug 22, 2015

bradrydzewski commented Aug 22, 2015

kzaitsev commented Aug 22, 2015

bradrydzewski commented Aug 24, 2015

kzaitsev commented Aug 24, 2015

bradrydzewski commented Aug 24, 2015

bradrydzewski commented Aug 24, 2015

kzaitsev commented Aug 24, 2015

bradrydzewski Aug 24, 2015

bradrydzewski commented Aug 24, 2015

kzaitsev commented Aug 29, 2015

DouweM Aug 29, 2015

kzaitsev Aug 29, 2015

DouweM Aug 29, 2015

DouweM commented Aug 29, 2015

kzaitsev commented Aug 29, 2015

DouweM Aug 29, 2015

kzaitsev Aug 30, 2015

DouweM Aug 30, 2015

DouweM commented Aug 29, 2015

kzaitsev commented Aug 29, 2015

DouweM commented Aug 29, 2015

kzaitsev commented Aug 30, 2015

kzaitsev commented Sep 1, 2015

bradrydzewski commented Sep 2, 2015

kzaitsev commented Sep 2, 2015

bradrydzewski commented Sep 2, 2015

GitLab service interface #1159

GitLab service interface #1159

Conversation

kzaitsev commented Aug 22, 2015

kzaitsev commented Aug 22, 2015

bradrydzewski commented Aug 22, 2015

kzaitsev commented Aug 22, 2015

bradrydzewski commented Aug 24, 2015

kzaitsev commented Aug 24, 2015

bradrydzewski commented Aug 24, 2015

bradrydzewski commented Aug 24, 2015

kzaitsev commented Aug 24, 2015

bradrydzewski Aug 24, 2015

Choose a reason for hiding this comment

bradrydzewski commented Aug 24, 2015

kzaitsev commented Aug 29, 2015

DouweM Aug 29, 2015

Choose a reason for hiding this comment

kzaitsev Aug 29, 2015

Choose a reason for hiding this comment

DouweM Aug 29, 2015

Choose a reason for hiding this comment

DouweM commented Aug 29, 2015

kzaitsev commented Aug 29, 2015

DouweM Aug 29, 2015

Choose a reason for hiding this comment

kzaitsev Aug 30, 2015

Choose a reason for hiding this comment

DouweM Aug 30, 2015

Choose a reason for hiding this comment

DouweM commented Aug 29, 2015

kzaitsev commented Aug 29, 2015

DouweM commented Aug 29, 2015

kzaitsev commented Aug 30, 2015

kzaitsev commented Sep 1, 2015

bradrydzewski commented Sep 2, 2015

kzaitsev commented Sep 2, 2015

bradrydzewski commented Sep 2, 2015