From 2ce566483b33a62cff2acce059846e03f9fbb264 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Sat, 11 Jun 2016 18:42:55 -0700 Subject: [PATCH 1/5] Handling some of the error in bitbucksetserver Fixed returning the http(s) version of the clone link which had the username in it however should not to work with netrc Quick typo fix --- agent/agent.go | 2 +- remote/bitbucketserver/bitbucketserver.go | 53 ++++++++++++++++++----- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index d3a83f8a8..a69fef1f5 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -95,7 +95,7 @@ func (a *Agent) prep(w *queue.Work) (*yaml.Config, error) { envs := toEnv(w) w.Yaml = expander.ExpandString(w.Yaml, envs) - // inject the netrc file into the clone plugin if the repositroy is + // inject the netrc file into the clone plugin if the repository is // private and requires authentication. var secrets []*model.Secret if w.Verified { diff --git a/remote/bitbucketserver/bitbucketserver.go b/remote/bitbucketserver/bitbucketserver.go index 6b73ad136..7560c4814 100644 --- a/remote/bitbucketserver/bitbucketserver.go +++ b/remote/bitbucketserver/bitbucketserver.go @@ -17,6 +17,7 @@ import ( "github.com/drone/drone/model" "github.com/drone/drone/remote" "github.com/mrjones/oauth" + "strings" ) // Opts defines configuration options. @@ -115,16 +116,24 @@ func (c *client) Login(res http.ResponseWriter, req *http.Request) (*model.User, // TODO errors should never be ignored like this response1, err := client.Get(fmt.Sprintf("%s/rest/api/1.0/users/%s", c.URL, login)) + if err != nil { + return nil, err + } contents, err := ioutil.ReadAll(response1.Body) + if err !=nil { + return nil, err + } defer response1.Body.Close() - var mUser User // TODO prefixing with m* is not a common convention in Go - json.Unmarshal(contents, &mUser) // TODO should not ignore error - + var user User + err = json.Unmarshal(contents, &user) + if err != nil { + return nil, err + } return &model.User{ Login: login, - Email: mUser.EmailAddress, + Email: user.EmailAddress, Token: accessToken.Token, - Avatar: avatarLink(mUser.EmailAddress), + Avatar: avatarLink(user.EmailAddress), }, nil } @@ -152,20 +161,28 @@ func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { defer response.Body.Close() contents, err := ioutil.ReadAll(response.Body) bsRepo := BSRepo{} - json.Unmarshal(contents, &bsRepo) - + err = json.Unmarshal(contents, &bsRepo) + if err !=nil { + return nil, err + } repo := &model.Repo{ Name: bsRepo.Slug, Owner: bsRepo.Project.Key, Branch: "master", Kind: model.RepoGit, - IsPrivate: !bsRepo.Project.Public, // TODO(josmo) verify this is corrrect + IsPrivate: true, // TODO(josmo) possibly set this as a setting - must always be private to use netrc FullName: fmt.Sprintf("%s/%s", bsRepo.Project.Key, bsRepo.Slug), } for _, item := range bsRepo.Links.Clone { if item.Name == "http" { - repo.Clone = item.Href + //TODO sdhould find a clean way to do this + //We are removing the username out fo the link to allow for Netrc to work + splitUrl := strings.SplitAfterN(item.Href,"@",2) + splitProtocal := strings.SplitAfterN(splitUrl[0],"//",2) + cleanUrl := fmt.Sprintf("%s%s",splitProtocal[0], splitUrl[1]) + + repo.Clone = cleanUrl } } for _, item := range bsRepo.Links.Self { @@ -189,8 +206,14 @@ func (c *client) Repos(u *model.User) ([]*model.RepoLite, error) { } defer response.Body.Close() contents, err := ioutil.ReadAll(response.Body) + if err != nil { + return nil, err + } var repoResponse Repos - json.Unmarshal(contents, &repoResponse) + err = json.Unmarshal(contents, &repoResponse) + if err != nil { + return nil, err + } for _, repo := range repoResponse.Values { repos = append(repos, &model.RepoLite{ @@ -240,12 +263,18 @@ func (*client) Status(*model.User, *model.Repo, *model.Build, string) error { } func (c *client) Netrc(user *model.User, r *model.Repo) (*model.Netrc, error) { - u, err := url.Parse(c.URL) // TODO strip port from url + u, err := url.Parse(c.URL) + //remove the port + tmp := strings.Split(u.Host, ":") + var host = tmp[0] + if err != nil { return nil, err } + log.Info(fmt.Sprintf("machine % login %s password %s", host, c.GitUserName, c.GitPassword)) + return &model.Netrc{ - Machine: u.Host, + Machine: host, Login: c.GitUserName, Password: c.GitPassword, }, nil From 6f4c4a37dc2392938dc247033319b33dd8986862 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Sat, 11 Jun 2016 18:53:41 -0700 Subject: [PATCH 2/5] Remove log and fix typo --- remote/bitbucketserver/bitbucketserver.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/remote/bitbucketserver/bitbucketserver.go b/remote/bitbucketserver/bitbucketserver.go index 7560c4814..460b01001 100644 --- a/remote/bitbucketserver/bitbucketserver.go +++ b/remote/bitbucketserver/bitbucketserver.go @@ -177,7 +177,7 @@ func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { for _, item := range bsRepo.Links.Clone { if item.Name == "http" { //TODO sdhould find a clean way to do this - //We are removing the username out fo the link to allow for Netrc to work + //We are removing the username out of the link to allow for Netrc to work splitUrl := strings.SplitAfterN(item.Href,"@",2) splitProtocal := strings.SplitAfterN(splitUrl[0],"//",2) cleanUrl := fmt.Sprintf("%s%s",splitProtocal[0], splitUrl[1]) @@ -271,8 +271,6 @@ func (c *client) Netrc(user *model.User, r *model.Repo) (*model.Netrc, error) { if err != nil { return nil, err } - log.Info(fmt.Sprintf("machine % login %s password %s", host, c.GitUserName, c.GitPassword)) - return &model.Netrc{ Machine: host, Login: c.GitUserName, From be1b75fd45f7423a2461f2ae5b2edb051f79e1d4 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Sun, 12 Jun 2016 22:18:31 -0700 Subject: [PATCH 3/5] Fix defer location and err check --- remote/bitbucketserver/bitbucketserver.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/remote/bitbucketserver/bitbucketserver.go b/remote/bitbucketserver/bitbucketserver.go index 460b01001..170946d20 100644 --- a/remote/bitbucketserver/bitbucketserver.go +++ b/remote/bitbucketserver/bitbucketserver.go @@ -119,11 +119,13 @@ func (c *client) Login(res http.ResponseWriter, req *http.Request) (*model.User, if err != nil { return nil, err } + defer response1.Body.Close() + contents, err := ioutil.ReadAll(response1.Body) if err !=nil { return nil, err } - defer response1.Body.Close() + var user User err = json.Unmarshal(contents, &user) if err != nil { @@ -264,6 +266,9 @@ func (*client) Status(*model.User, *model.Repo, *model.Build, string) error { func (c *client) Netrc(user *model.User, r *model.Repo) (*model.Netrc, error) { u, err := url.Parse(c.URL) + if err != nil { + return nil, err + } //remove the port tmp := strings.Split(u.Host, ":") var host = tmp[0] From 1152e430c42bc5b69ed7d3eb66425e29000164d2 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Mon, 13 Jun 2016 18:10:16 -0500 Subject: [PATCH 4/5] Cleaner way to remove the username from the git clone URL --- remote/bitbucketserver/bitbucketserver.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/remote/bitbucketserver/bitbucketserver.go b/remote/bitbucketserver/bitbucketserver.go index 170946d20..ab2d52984 100644 --- a/remote/bitbucketserver/bitbucketserver.go +++ b/remote/bitbucketserver/bitbucketserver.go @@ -154,9 +154,9 @@ func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { client := NewClientWithToken(&c.Consumer, u.Token) - url := fmt.Sprintf("%s/rest/api/1.0/projects/%s/repos/%s", c.URL, owner, name) + urlString := fmt.Sprintf("%s/rest/api/1.0/projects/%s/repos/%s", c.URL, owner, name) - response, err := client.Get(url) + response, err := client.Get(urlString) if err != nil { log.Error(err) } @@ -178,13 +178,12 @@ func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { for _, item := range bsRepo.Links.Clone { if item.Name == "http" { - //TODO sdhould find a clean way to do this - //We are removing the username out of the link to allow for Netrc to work - splitUrl := strings.SplitAfterN(item.Href,"@",2) - splitProtocal := strings.SplitAfterN(splitUrl[0],"//",2) - cleanUrl := fmt.Sprintf("%s%s",splitProtocal[0], splitUrl[1]) - - repo.Clone = cleanUrl + uri, err := url.Parse(item.Href) + if err != nil { + return err + } + uri.User = nil + repo.Clone = uri.String() } } for _, item := range bsRepo.Links.Self { From a4d28e39a0ed6d81b16cb72643ba02f3bf71df8b Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Mon, 13 Jun 2016 18:25:31 -0500 Subject: [PATCH 5/5] Missed an argument --- remote/bitbucketserver/bitbucketserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote/bitbucketserver/bitbucketserver.go b/remote/bitbucketserver/bitbucketserver.go index ab2d52984..8271d2b06 100644 --- a/remote/bitbucketserver/bitbucketserver.go +++ b/remote/bitbucketserver/bitbucketserver.go @@ -180,7 +180,7 @@ func (c *client) Repo(u *model.User, owner, name string) (*model.Repo, error) { if item.Name == "http" { uri, err := url.Parse(item.Href) if err != nil { - return err + return nil, err } uri.User = nil repo.Clone = uri.String()