Stash/Bitbucket Server initial support #1581

josmo · 2016-04-19T05:48:16Z

This is the first go to get Stash/Bitbucket support working(continuation of #567). The "happy path" works and there's a bit of clean up (First time I've written anything in golang) but if folks want to try it out or give some feedback on the dos and don'ts I'll update it.

CLAassistant · 2016-04-19T05:48:16Z

All committers have accepted the CLA.

tboerger · 2016-04-19T06:29:13Z

remote/bitbucketserver/bitbucketserver.go

+func Load(env envconfig.Env) *BitbucketServer{
+
+ //Read
+ config := env.String("REMOTE_CONFIG", "")


Instead of adding it as separate env variables I would add it as part of the url and drop it from there while parsing

I am talking about the separate username and password

Thanks I'll make the change.

bradrydzewski · 2016-04-19T07:35:37Z

@josmo really good start! I know you are new to Go so I've provided some minor style and coding suggestions above. You will also need to resync with master since the code has merge conflicts.

bradrydzewski · 2016-04-19T07:37:42Z

remote/bitbucketserver/bitbucketserver.go

+ bitbucketserver := BitbucketServer{}
+ bitbucketserver.URL = url_.String()
+ bitbucketserver.GitUserName = gitUserName
+ bitbucketserver.GitPassword = gitUserPassword


as @tboerger mentioned above we should pass all parameters through the configuration string like this:

- bitbucketserver.GitUserName = gitUserName - bitbucketserver.GitPassword = gitUserPassword + bitbucketserver.GitUserName = params.Get("git_username") + bitbucketserver.GitPassword = params.Get("git_password")

Moving git username and password to the url Removing un-needed setting of Allows Moving log.Fatal to log.Error Removing panics Moving to https for gravatar # Conflicts: # remote/remote.go

…till it's configurable

josmo · 2016-04-19T18:21:29Z

@bradrydzewski I think I have all the comments in and merged in master. The only thing is on how to handle the private key. For right now I put it in /var/lib/bitbucketserver/private_key.pem - it's about 50ish lines so if you think there's a better way to handle it I'll look into it.

bradrydzewski · 2016-04-19T19:00:25Z

@josmo I recommend making the path configurable as part of the REMOTE_CONFIG connection string. For example REMOTE_CONFIG=https://bitbucket.mycompany.com?consumer_key=xxx&consumer_rsa=/path/to/file.pem this lets the developer choose the path and mount the file into the container. If the value is empty it should fail using log.fatal with the appropriate error message. It should probably also fail if the git username and password are empty

bradrydzewski · 2016-04-19T19:02:49Z

Makefile

@@ -12,6 +12,7 @@ deps:
 go get -u github.com/elazarl/go-bindata-assetfs/...
 go get -u github.com/dchest/jsmin
 go get -u github.com/franela/goblin
+ go get -u github.com/mrjones/oauth


also can you remove this line?

we vendor our dependencies using the govendor tool. You can download the govendor tool with the following command:

go get -u github.com/kardianos/govendor

and then you can add the oauth dependency with the following line:

govendor add github.com/mrjones/oauth

note that if this is vendored properly you should only see mrjones/oauth added to the vendor directory and nothing else

Cool made the change. Had no idea about it

kzaitsev · 2016-04-20T18:51:54Z

need go-fmt

kzaitsev · 2016-04-20T19:04:28Z

remote/bitbucketserver/bitbucketserver.go

@@ -111,7 +105,7 @@ func (bs *BitbucketServer) Login(res http.ResponseWriter, req *http.Request) (*m
 bits, err := ioutil.ReadAll(response.Body)
 userName := string(bits)

- response1, err := client.Get(bs.URL + "/rest/api/1.0/users/" +userName)
+ response1, err := client.Get(bs.URL + "/rest/api/1.0/users/" + userName)


fmt.Sprintf("%s/rest/api/1.0/users/%s", bs.URL, userName) for string interpolation

mvdkleijn · 2016-04-21T12:55:59Z

I've corrected this line in PR #1589

valichek · 2016-04-23T10:55:50Z

Got the same error when tested against latest

time="2016-04-23T10:45:28Z" level=info msg="Starting to login for bitbucketServer"
time="2016-04-23T10:45:28Z" level=info msg="getting the requestToken"
time="2016-04-23T10:45:28Z" level=info msg="method: POST url: https://stash.myserver.de/plugins/servlet/oauth/request-token authparams: &{map[oauth_signature_method:[RSA-SHA1] oauth_timestamp:[1461408328] oauth_nonce:[2546162481240149727] oauth_consumer_key:[consumer_key] oauth_callback:[oob] oauth_version:[1.0]] [oauth_version oauth_signature_method oauth_timestamp oauth_nonce oauth_consumer_key oauth_callback]}"
...
same error here

it may be creadential issue, but no readable error showed

josmo · 2016-04-23T14:29:59Z

@valichek I'll have to add a bit insightful errors in it. From where that errors takes places it's probably due to the private key. Did you generate a private/public key, something like

openssl genrsa -out private_key.pem 4096
openssl rsa -pubout -in private_key.pem -out public_key.pem

and then made sure the private key was in the location for ${consumer_rsa} that the container has access to?

valichek · 2016-04-24T10:09:44Z

@josmo I managed to authenticate stash, but it was really tricky, would be nice to have some pics in docs because the workflow differs much from github auth

mvdkleijn · 2016-04-24T14:47:27Z

Hi, I would like to report that it was working fine for authentication and more up to activation of a repo. Activation failed due to a typo which is why I submitted my PR.

The commits after my PR seem to have broken something though... now I can't even start drone due to some sort of failure related to the stash/bitbucketserver plugin. I was using the exact same config as when it worked. I just pulled the latest docker image.

valichek · 2016-04-24T15:13:02Z

@mvdkleijn I had to add AGENT_SECRET=dummy to dronerc to avoid errors that I got from commits after your PR

valichek · 2016-04-24T16:34:01Z

@josmo I have authorized drone for stash, I can browse repos, I have activated one, but when I push new commit, drone does nothing, no build started, no errors in log, do you have an idea what is going on? How does drone handles pushes to stash? And sure, I have added .drone.yml

josmo · 2016-04-24T16:42:16Z

@valichek The first thing I would check is under that repos settings in stash under hooks. There should be a web-hook plugin thats enabled and has the url to call drone on pushes. What version of stash are you on? I've only tested on 4.5

valichek · 2016-04-24T17:01:18Z

@josmo The only webhook I have is for Jenkins, could you show how it should look like? Screenshot? Stash has 4.4.1 version

josmo · 2016-04-24T17:12:20Z

@valichek That last one is the webhook post I believe is included with Stash (at least it's included in all the instances I'm working with ). It should work on 4.4.1

valichek · 2016-04-24T17:18:42Z

@josmo and what is an url for webhook?

josmo · 2016-04-24T17:21:59Z

it should be something like http:https://{baseurl to your drone instance}/hook?access_token={the access token drone gave stash when setting up the hook}

valichek · 2016-04-24T17:26:33Z

@josmo So this webhook should be automatically created when creating app link?

josmo · 2016-04-24T17:31:45Z

yep yep as long as the account you are using to link has permissions on that repository

valichek · 2016-04-24T17:43:38Z

@josmo is that means I need admin account for stash to activate repo

josmo · 2016-04-24T17:44:49Z

no just the repo not all of stash

mvdkleijn · 2016-04-24T17:49:32Z

Thanks @valicheck, I'll try that

valichek · 2016-04-25T16:52:28Z

There is a bug here https://github.com/drone/drone/blob/master/remote/bitbucketserver/bitbucketserver.go#L216
File is always fetched from master branch
It should be

fileURL := fmt.Sprintf("%s/projects/%s/repos/%s/browse/%s?raw&at=%s", bs.URL, r.Owner, r.Name, f, b.Commit)

mvdkleijn · 2016-04-26T09:14:28Z

@josmo - The post receive webhook you're referring to is a plugin that's not installed by default. It is created by Atlassian Labs (and therefore unsupported). You can find it here: https://marketplace.atlassian.com/plugins/com.atlassian.stash.plugin.stash-web-post-receive-hooks-plugin/versions#b312

mvdkleijn · 2016-04-26T09:26:19Z

Adding AGENT_SECRET works nicely by the way...

josmo · 2016-04-26T16:46:59Z

@mvdkleijn Oh good to know. It's been included on each bitbucket server I've used. I'll add that to the docs when I get a chance (unless you want to add a create a PR :))

@valichek Oh yeah I think I remember thinking that should be in there and forgot about it. I'll have some time later this week to fix this unless you want to do it :)

bradrydzewski · 2016-05-01T22:24:44Z

@josmo I just wanted to follow up and see if you are planning on a second set of pull requests for Bitbucket Server? I do have some concerns with the initial implementation, such as all repositories being marked public, all users being granted admin access, and skipping ssl verification by default. Since these are security related we'll need to get them resolved or else I'll be forced to disable the implementation (not trying to be a jerk but I don't want someone getting hacked and blaming drone)

josmo · 2016-05-01T22:45:54Z

@bradrydzewski Sorry, i've gotten caught on up a few things and haven't had a chance to work on this for a few. I'm definitely planning on adding a new set of changes to add perms. I just haven't had time to dig down into how the perms are used but I'll try to get to it soon :)

josmo · 2016-05-08T03:58:46Z

@bradrydzewski I was starting look at the permissions and address the comments but it looks like it's a bit in the middle of .5/.4 release status. Should I wait after .5 is out? I don't want to step on anything and make sure everything builds :)

jesseops · 2016-05-26T21:17:30Z

@josmo I'm having difficulty getting this remote driver to work using BitbucketServer 4.6.1 and Drone 0.4.2. I manually generated my public/private keypair and setup the application link in BitbucketServer, then configured my dronerc with the appropriate consumer_key and path to drone.pem.

I navigate to $dronehost/login, hit login and get redirected to Stash where I successfully allow drone access to stash. I'm then sent to a confirmation page saying "Access Approved" with a verification code. I don't see anything in the drone logs after the redirect to Bitbucket, and don't see drone in my users application links. I'm wondering if I need more than just the application authorize url in my Bitbucket application link configuration?

josmo · 2016-05-27T21:16:40Z

@dreadpirate15 I haven't looked at Bitbucket 4.6 yet, although I don't think anything would have changed to break it. Did you set up the redirect back to drone in the application link under incoming authentication (http:https://{drone host name}/authorize/)? You shouldn't see the verfication code, that should be sent back to drone on the authorize. Normally you should see drone in your user authorized applications.

gdubicki · 2016-06-13T10:10:52Z

What is the status of this integration's security improvement, @josmo ?

bradrydzewski · 2016-06-13T21:52:25Z

This is a list of things I'd like to see improved for the Stash implementation. Note that @josmo could probably use some help. I know there has been a lot of interest in this feature, so I would welcome others to contribute to this implementation.

API

the bitbucket API calls should be encapsulated in a library similar to bitbucket cloud in an internal package. See https://github.com/drone/drone/tree/master/remote/bitbucket/internal for a reference implementation

Security

the bitbucket api should be used to fetch permissions instead of granting all users default, admin access to repositories. See https://github.com/drone/drone/blob/ebd547deacce9de774d609261d4049c99883945c/remote/bitbucketserver/bitbucketserver.go#L206

Testing

see the bitbucket cloud package for reference. This package has near 100% coverage

Style

the overall code style is very different from the other remote implementations. The stash implementation should try to closely resemble these other packages. I recommend looking at the bitbucket cloud package and trying to emulate the same style and structure as much as possible.

josmo · 2016-06-13T22:28:47Z

@gdubicki I was in process of working on it for .5 when I got side tracked to fix the clone issue on it.

I'll take a look at the security next. Then I"ll see about matching the API and style to bitbucket cloud (with testing). For security I imagine I'll have to do the same hack as bitbucket cloud since there isn't an api but will have to look at the hook permissions to it.

@bradrydzewski For the Security to be considered ok. If I follow the same suite a bitbucket cloud does that seem ok to you?

I definitely wouldn't mind having some help on it since I'm pretty new at this. Thanks @bradrydzewski

josmo · 2016-07-03T19:18:54Z

FYI #1674 is the first go at addressing security issues and #1690 is looking to clean up the API and style to be closer to bitbucket cloud.

josmo added 4 commits April 19, 2016 00:40

Initial take at the bitbucket server remote additions.

59edcd3

Fixing the url to gravatar to have hex encoding

4f9f567

fixing to use the commit hash instead of the refID

4b31bab

added a few comments

df45c84

tboerger reviewed Apr 19, 2016
View reviewed changes

bradrydzewski reviewed Apr 19, 2016
View reviewed changes

josmo added 3 commits April 19, 2016 09:47

Merge branch 'master' into bitbucketserver

02c87d0

Moving git username and password to the url Removing un-needed setting of Allows Moving log.Fatal to log.Error Removing panics Moving to https for gravatar # Conflicts: # remote/remote.go

Removing no longer needed import

b89b7cf

moving the private key out of root and into a more sensical location …

16ddb75

…till it's configurable

bradrydzewski reviewed Apr 19, 2016
View reviewed changes

josmo added 5 commits April 19, 2016 13:26

Configurable consumer rsa file location

0a20d9c

Must have a git username, password, consumer key, and consumer rsa

d982f73

Typo

c250926

Using vendor for the oauth lib

ef4d41c

Moving the NewClient to load instead of on every call

262f505

Running gofmt on remote for bitbucket server

95b1f34

kzaitsev reviewed Apr 20, 2016
View reviewed changes

using fmt.Sprintf for string interpolation.

06003bf

Stash/Bitbucket Server initial support #1581

Stash/Bitbucket Server initial support #1581

Conversation

josmo commented Apr 19, 2016

CLAassistant commented Apr 19, 2016

tboerger Apr 19, 2016

Choose a reason for hiding this comment

tboerger Apr 19, 2016

Choose a reason for hiding this comment

josmo Apr 19, 2016

Choose a reason for hiding this comment

bradrydzewski commented Apr 19, 2016

bradrydzewski Apr 19, 2016 • edited Loading

Choose a reason for hiding this comment

josmo commented Apr 19, 2016

bradrydzewski commented Apr 19, 2016 • edited Loading

bradrydzewski Apr 19, 2016

Choose a reason for hiding this comment

josmo Apr 19, 2016

Choose a reason for hiding this comment

kzaitsev commented Apr 20, 2016

kzaitsev Apr 20, 2016

Choose a reason for hiding this comment

mvdkleijn commented Apr 21, 2016

valichek commented Apr 23, 2016 • edited Loading

josmo commented Apr 23, 2016

valichek commented Apr 24, 2016

mvdkleijn commented Apr 24, 2016

valichek commented Apr 24, 2016

valichek commented Apr 24, 2016 • edited Loading

josmo commented Apr 24, 2016

valichek commented Apr 24, 2016 • edited Loading

josmo commented Apr 24, 2016 • edited Loading

valichek commented Apr 24, 2016

josmo commented Apr 24, 2016

valichek commented Apr 24, 2016

josmo commented Apr 24, 2016

valichek commented Apr 24, 2016

josmo commented Apr 24, 2016

mvdkleijn commented Apr 24, 2016

valichek commented Apr 25, 2016

mvdkleijn commented Apr 26, 2016

mvdkleijn commented Apr 26, 2016

josmo commented Apr 26, 2016 • edited Loading

bradrydzewski commented May 1, 2016 • edited Loading

josmo commented May 1, 2016 • edited Loading

josmo commented May 8, 2016

jesseops commented May 26, 2016

josmo commented May 27, 2016

gdubicki commented Jun 13, 2016

bradrydzewski commented Jun 13, 2016

josmo commented Jun 13, 2016

josmo commented Jul 3, 2016

bradrydzewski Apr 19, 2016 •

edited

Loading

bradrydzewski commented Apr 19, 2016 •

edited

Loading

valichek commented Apr 23, 2016 •

edited

Loading

valichek commented Apr 24, 2016 •

edited

Loading

valichek commented Apr 24, 2016 •

edited

Loading

josmo commented Apr 24, 2016 •

edited

Loading

josmo commented Apr 26, 2016 •

edited

Loading

bradrydzewski commented May 1, 2016 •

edited

Loading

josmo commented May 1, 2016 •

edited

Loading