From 10cda9824503416fbb655bf501ab74c68ea76cb5 Mon Sep 17 00:00:00 2001 From: Colin Wood Date: Tue, 4 Apr 2017 11:27:23 -0700 Subject: [PATCH] Refactoring of bitbucket provider with betters. Also doesnt use decode/encode anymore since those methods are more intended for streams. So this goes to the more standed marshal/unmarshal of data. This also adds better error support and will try to give the user better errors from the api if it can. Or any issues with the bitbucket service. --- builtin/providers/bitbucket/client.go | 125 +++++++++++------ builtin/providers/bitbucket/provider.go | 7 +- .../bitbucket/resource_default_reviewers.go | 19 +-- builtin/providers/bitbucket/resource_hook.go | 132 +++++++++--------- .../providers/bitbucket/resource_hook_test.go | 9 +- .../bitbucket/resource_repository.go | 104 ++++++-------- .../bitbucket/resource_repository_test.go | 10 +- 7 files changed, 215 insertions(+), 191 deletions(-) diff --git a/builtin/providers/bitbucket/client.go b/builtin/providers/bitbucket/client.go index 02b9ed0db..bd2cebcce 100644 --- a/builtin/providers/bitbucket/client.go +++ b/builtin/providers/bitbucket/client.go @@ -2,64 +2,107 @@ package bitbucket import ( "bytes" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "log" "net/http" ) +// Error represents a error from the bitbucket api. +type Error struct { + APIError struct { + Message string `json:"message,omitempty"` + } `json:"error,omitempty"` + Type string `json:"type,omitempty"` + StatusCode int + Endpoint string +} + +func (e Error) Error() string { + return fmt.Sprintf("API Error: %d %s %s", e.StatusCode, e.Endpoint, e.APIError.Message) +} + +const ( + // BitbucketEndpoint is the fqdn used to talk to bitbucket + BitbucketEndpoint string = "https://api.bitbucket.org/" +) + type BitbucketClient struct { - Username string - Password string + Username string + Password string + HTTPClient *http.Client +} + +func (c *BitbucketClient) Do(method, endpoint string, payload *bytes.Buffer) (*http.Response, error) { + + absoluteendpoint := BitbucketEndpoint + endpoint + log.Printf("[DEBUG] Sending request to %s %s", method, absoluteendpoint) + + var bodyreader io.Reader + + if payload != nil { + log.Printf("[DEBUG] With payload %s", payload.String()) + bodyreader = payload + } + + req, err := http.NewRequest(method, absoluteendpoint, bodyreader) + if err != nil { + return nil, err + } + + req.SetBasicAuth(c.Username, c.Password) + + if payload != nil { + // Can cause bad request when putting default reviews if set. + req.Header.Add("Content-Type", "application/json") + } + + req.Close = true + + resp, err := c.HTTPClient.Do(req) + log.Printf("[DEBUG] Resp: %v Err: %v", resp, err) + if resp.StatusCode >= 400 || resp.StatusCode < 200 { + apiError := Error{ + StatusCode: resp.StatusCode, + Endpoint: endpoint, + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + log.Printf("[DEBUG] Resp Body: %s", string(body)) + + err = json.Unmarshal(body, &apiError) + if err != nil { + apiError.APIError.Message = string(body) + } + + return resp, error(apiError) + + } + return resp, err } func (c *BitbucketClient) Get(endpoint string) (*http.Response, error) { - client := &http.Client{} - req, err := http.NewRequest("GET", "https://api.bitbucket.org/"+endpoint, nil) - if err != nil { - return nil, err - } - - req.SetBasicAuth(c.Username, c.Password) - return client.Do(req) - + return c.Do("GET", endpoint, nil) } func (c *BitbucketClient) Post(endpoint string, jsonpayload *bytes.Buffer) (*http.Response, error) { - client := &http.Client{} - req, err := http.NewRequest("POST", "https://api.bitbucket.org/"+endpoint, jsonpayload) - if err != nil { - return nil, err - } - req.SetBasicAuth(c.Username, c.Password) - req.Header.Add("content-type", "application/json") - return client.Do(req) + return c.Do("POST", endpoint, jsonpayload) } func (c *BitbucketClient) Put(endpoint string, jsonpayload *bytes.Buffer) (*http.Response, error) { - client := &http.Client{} - req, err := http.NewRequest("PUT", "https://api.bitbucket.org/"+endpoint, jsonpayload) - if err != nil { - return nil, err - } - req.SetBasicAuth(c.Username, c.Password) - req.Header.Add("content-type", "application/json") - return client.Do(req) + return c.Do("PUT", endpoint, jsonpayload) } func (c *BitbucketClient) PutOnly(endpoint string) (*http.Response, error) { - client := &http.Client{} - req, err := http.NewRequest("PUT", "https://api.bitbucket.org/"+endpoint, nil) - if err != nil { - return nil, err - } - req.SetBasicAuth(c.Username, c.Password) - return client.Do(req) + return c.Do("PUT", endpoint, nil) } func (c *BitbucketClient) Delete(endpoint string) (*http.Response, error) { - client := &http.Client{} - req, err := http.NewRequest("DELETE", "https://api.bitbucket.org/"+endpoint, nil) - if err != nil { - return nil, err - } - req.SetBasicAuth(c.Username, c.Password) - return client.Do(req) + return c.Do("DELETE", endpoint, nil) } diff --git a/builtin/providers/bitbucket/provider.go b/builtin/providers/bitbucket/provider.go index afeb71249..e50f9295f 100644 --- a/builtin/providers/bitbucket/provider.go +++ b/builtin/providers/bitbucket/provider.go @@ -1,6 +1,8 @@ package bitbucket import ( + "net/http" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) @@ -30,8 +32,9 @@ func Provider() terraform.ResourceProvider { func providerConfigure(d *schema.ResourceData) (interface{}, error) { client := &BitbucketClient{ - Username: d.Get("username").(string), - Password: d.Get("password").(string), + Username: d.Get("username").(string), + Password: d.Get("password").(string), + HTTPClient: &http.Client{}, } return client, nil diff --git a/builtin/providers/bitbucket/resource_default_reviewers.go b/builtin/providers/bitbucket/resource_default_reviewers.go index 37a0f5883..9fc5d1e0a 100644 --- a/builtin/providers/bitbucket/resource_default_reviewers.go +++ b/builtin/providers/bitbucket/resource_default_reviewers.go @@ -3,6 +3,7 @@ package bitbucket import ( "encoding/json" "fmt" + "github.com/hashicorp/terraform/helper/schema" ) @@ -49,7 +50,7 @@ func resourceDefaultReviewersCreate(d *schema.ResourceData, m interface{}) error client := m.(*BitbucketClient) for _, user := range d.Get("reviewers").(*schema.Set).List() { - reviewer_resp, err := client.PutOnly(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers/%s", + reviewerResp, err := client.PutOnly(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers/%s", d.Get("owner").(string), d.Get("repository").(string), user, @@ -59,11 +60,11 @@ func resourceDefaultReviewersCreate(d *schema.ResourceData, m interface{}) error return err } - if reviewer_resp.StatusCode != 200 { - return fmt.Errorf("Failed to create reviewer %s got code %d", user.(string), reviewer_resp.StatusCode) + if reviewerResp.StatusCode != 200 { + return fmt.Errorf("Failed to create reviewer %s got code %d", user.(string), reviewerResp.StatusCode) } - defer reviewer_resp.Body.Close() + defer reviewerResp.Body.Close() } d.SetId(fmt.Sprintf("%s/%s/reviewers", d.Get("owner").(string), d.Get("repository").(string))) @@ -72,26 +73,26 @@ func resourceDefaultReviewersCreate(d *schema.ResourceData, m interface{}) error func resourceDefaultReviewersRead(d *schema.ResourceData, m interface{}) error { client := m.(*BitbucketClient) - reviewers_response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers", + reviewersResponse, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/default-reviewers", d.Get("owner").(string), d.Get("repository").(string), )) var reviewers PaginatedReviewers - decoder := json.NewDecoder(reviewers_response.Body) + decoder := json.NewDecoder(reviewersResponse.Body) err = decoder.Decode(&reviewers) if err != nil { return err } - terraform_reviewers := make([]string, 0, len(reviewers.Values)) + terraformReviewers := make([]string, 0, len(reviewers.Values)) for _, reviewer := range reviewers.Values { - terraform_reviewers = append(terraform_reviewers, reviewer.Username) + terraformReviewers = append(terraformReviewers, reviewer.Username) } - d.Set("reviewers", terraform_reviewers) + d.Set("reviewers", terraformReviewers) return nil } diff --git a/builtin/providers/bitbucket/resource_hook.go b/builtin/providers/bitbucket/resource_hook.go index c93862798..745292ad1 100644 --- a/builtin/providers/bitbucket/resource_hook.go +++ b/builtin/providers/bitbucket/resource_hook.go @@ -4,6 +4,10 @@ import ( "bytes" "encoding/json" "fmt" + "io/ioutil" + "log" + "net/url" + "github.com/hashicorp/terraform/helper/schema" ) @@ -81,86 +85,89 @@ func resourceHookCreate(d *schema.ResourceData, m interface{}) error { client := m.(*BitbucketClient) hook := createHook(d) - var jsonbuffer []byte - - jsonpayload := bytes.NewBuffer(jsonbuffer) - enc := json.NewEncoder(jsonpayload) - enc.Encode(hook) - - hook_req, err := client.Post(fmt.Sprintf("2.0/repositories/%s/%s/hooks", - d.Get("owner").(string), - d.Get("repository").(string), - ), jsonpayload) - - decoder := json.NewDecoder(hook_req.Body) - err = decoder.Decode(&hook) + payload, err := json.Marshal(hook) if err != nil { return err } + hook_req, err := client.Post(fmt.Sprintf("2.0/repositories/%s/%s/hooks", + d.Get("owner").(string), + d.Get("repository").(string), + ), bytes.NewBuffer(payload)) + + if err != nil { + return err + } + + body, readerr := ioutil.ReadAll(hook_req.Body) + if readerr != nil { + return readerr + } + + decodeerr := json.Unmarshal(body, &hook) + if decodeerr != nil { + return decodeerr + } + d.SetId(hook.Uuid) - d.Set("uuid", hook.Uuid) return resourceHookRead(d, m) } func resourceHookRead(d *schema.ResourceData, m interface{}) error { client := m.(*BitbucketClient) - hook_req, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", + + hook_req, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", d.Get("owner").(string), d.Get("repository").(string), - d.Get("uuid").(string), + url.PathEscape(d.Id()), )) - if err != nil { - return err + log.Printf("ID: %s", url.PathEscape(d.Id())) + + if hook_req.StatusCode == 200 { + var hook Hook + + body, readerr := ioutil.ReadAll(hook_req.Body) + if readerr != nil { + return readerr + } + + decodeerr := json.Unmarshal(body, &hook) + if decodeerr != nil { + return decodeerr + } + + d.Set("uuid", hook.Uuid) + d.Set("description", hook.Description) + d.Set("active", hook.Active) + d.Set("url", hook.Url) + + eventsList := make([]string, 0, len(hook.Events)) + + for _, event := range hook.Events { + eventsList = append(eventsList, event) + } + + d.Set("events", eventsList) } - var hook Hook - - decoder := json.NewDecoder(hook_req.Body) - err = decoder.Decode(&hook) - if err != nil { - return err - } - - d.Set("uuid", hook.Uuid) - d.Set("description", hook.Description) - d.Set("active", hook.Active) - d.Set("url", hook.Url) - - eventsList := make([]string, 0, len(hook.Events)) - - for _, event := range hook.Events { - eventsList = append(eventsList, event) - } - - d.Set("events", eventsList) - return nil } func resourceHookUpdate(d *schema.ResourceData, m interface{}) error { client := m.(*BitbucketClient) hook := createHook(d) - - var jsonbuffer []byte - - jsonpayload := bytes.NewBuffer(jsonbuffer) - enc := json.NewEncoder(jsonpayload) - enc.Encode(hook) - - hook_req, err := client.Put(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", - d.Get("owner").(string), - d.Get("repository").(string), - d.Get("uuid").(string), - ), jsonpayload) - + payload, err := json.Marshal(hook) if err != nil { return err } - decoder := json.NewDecoder(hook_req.Body) - err = decoder.Decode(&hook) + _, err = client.Put(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", + d.Get("owner").(string), + d.Get("repository").(string), + url.PathEscape(d.Id()), + ), bytes.NewBuffer(payload)) + if err != nil { return err } @@ -174,7 +181,7 @@ func resourceHookExists(d *schema.ResourceData, m interface{}) (bool, error) { hook_req, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", d.Get("owner").(string), d.Get("repository").(string), - d.Get("uuid").(string), + url.PathEscape(d.Id()), )) if err != nil { @@ -182,15 +189,14 @@ func resourceHookExists(d *schema.ResourceData, m interface{}) (bool, error) { } if hook_req.StatusCode != 200 { - d.SetId("") - return false, nil + return false, err } return true, nil - } else { - return false, nil } + return false, nil + } func resourceHookDelete(d *schema.ResourceData, m interface{}) error { @@ -198,11 +204,9 @@ func resourceHookDelete(d *schema.ResourceData, m interface{}) error { _, err := client.Delete(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", d.Get("owner").(string), d.Get("repository").(string), - d.Get("uuid").(string), + url.PathEscape(d.Id()), )) - if err != nil { - return err - } - return nil + return err + } diff --git a/builtin/providers/bitbucket/resource_hook_test.go b/builtin/providers/bitbucket/resource_hook_test.go index 178ebf27b..59a719b87 100644 --- a/builtin/providers/bitbucket/resource_hook_test.go +++ b/builtin/providers/bitbucket/resource_hook_test.go @@ -2,6 +2,7 @@ package bitbucket import ( "fmt" + "net/url" "os" "testing" @@ -16,7 +17,7 @@ func TestAccBitbucketHook_basic(t *testing.T) { testAccBitbucketHookConfig := fmt.Sprintf(` resource "bitbucket_repository" "test_repo" { owner = "%s" - name = "test-repo" + name = "test-repo-for-webhook-test" } resource "bitbucket_hook" "test_repo_hook" { owner = "%s" @@ -51,10 +52,10 @@ func testAccCheckBitbucketHookDestroy(s *terraform.State) error { return fmt.Errorf("Not found %s", "bitbucket_hook.test_repo_hook") } - response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["repository"], rs.Primary.Attributes["uuid"])) + response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["repository"], url.PathEscape(rs.Primary.Attributes["uuid"]))) - if err != nil { - return err + if err == nil { + return fmt.Errorf("The resource was found should have errored") } if response.StatusCode != 404 { diff --git a/builtin/providers/bitbucket/resource_repository.go b/builtin/providers/bitbucket/resource_repository.go index 050ddd9b2..f57db3ea8 100644 --- a/builtin/providers/bitbucket/resource_repository.go +++ b/builtin/providers/bitbucket/resource_repository.go @@ -4,8 +4,9 @@ import ( "bytes" "encoding/json" "fmt" + "io/ioutil" + "github.com/hashicorp/terraform/helper/schema" - "log" ) type CloneUrl struct { @@ -131,7 +132,7 @@ func resourceRepositoryUpdate(d *schema.ResourceData, m interface{}) error { enc := json.NewEncoder(jsonpayload) enc.Encode(repository) - repository_response, err := client.Put(fmt.Sprintf("2.0/repositories/%s/%s", + _, err := client.Put(fmt.Sprintf("2.0/repositories/%s/%s", d.Get("owner").(string), d.Get("name").(string), ), jsonpayload) @@ -140,16 +141,6 @@ func resourceRepositoryUpdate(d *schema.ResourceData, m interface{}) error { return err } - if repository_response.StatusCode == 200 { - decoder := json.NewDecoder(repository_response.Body) - err = decoder.Decode(&repository) - if err != nil { - return err - } - } else { - return fmt.Errorf("Failed to put: %d", repository_response.StatusCode) - } - return resourceRepositoryRead(d, m) } @@ -157,29 +148,19 @@ func resourceRepositoryCreate(d *schema.ResourceData, m interface{}) error { client := m.(*BitbucketClient) repo := newRepositoryFromResource(d) - var jsonbuffer []byte + bytedata, err := json.Marshal(repo) - jsonpayload := bytes.NewBuffer(jsonbuffer) - enc := json.NewEncoder(jsonpayload) - enc.Encode(repo) - - log.Printf("Sending %s \n", jsonpayload) - - repo_req, err := client.Post(fmt.Sprintf("2.0/repositories/%s/%s", - d.Get("owner").(string), - d.Get("name").(string), - ), jsonpayload) - - decoder := json.NewDecoder(repo_req.Body) - err = decoder.Decode(&repo) if err != nil { return err } - log.Printf("Received %s \n", repo_req.Body) + _, err = client.Post(fmt.Sprintf("2.0/repositories/%s/%s", + d.Get("owner").(string), + d.Get("name").(string), + ), bytes.NewBuffer(bytedata)) - if repo_req.StatusCode != 200 { - return fmt.Errorf("Failed to create repository got status code %d", repo_req.StatusCode) + if err != nil { + return err } d.SetId(string(fmt.Sprintf("%s/%s", d.Get("owner").(string), d.Get("name").(string)))) @@ -189,39 +170,42 @@ func resourceRepositoryCreate(d *schema.ResourceData, m interface{}) error { func resourceRepositoryRead(d *schema.ResourceData, m interface{}) error { client := m.(*BitbucketClient) - repo_req, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s", + repo_req, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s", d.Get("owner").(string), d.Get("name").(string), )) - if err != nil { - return err - } + if repo_req.StatusCode == 200 { - var repo Repository + var repo Repository - decoder := json.NewDecoder(repo_req.Body) - err = decoder.Decode(&repo) - if err != nil { - return err - } + body, readerr := ioutil.ReadAll(repo_req.Body) + if readerr != nil { + return readerr + } - d.Set("scm", repo.SCM) - d.Set("is_private", repo.IsPrivate) - d.Set("has_wiki", repo.HasWiki) - d.Set("has_issues", repo.HasIssues) - d.Set("name", repo.Name) - d.Set("language", repo.Language) - d.Set("fork_policy", repo.ForkPolicy) - d.Set("website", repo.Website) - d.Set("description", repo.Description) - d.Set("project_key", repo.Project.Key) + decodeerr := json.Unmarshal(body, &repo) + if decodeerr != nil { + return decodeerr + } - for _, clone_url := range repo.Links.Clone { - if clone_url.Name == "https" { - d.Set("clone_https", clone_url.Href) - } else { - d.Set("clone_ssh", clone_url.Href) + d.Set("scm", repo.SCM) + d.Set("is_private", repo.IsPrivate) + d.Set("has_wiki", repo.HasWiki) + d.Set("has_issues", repo.HasIssues) + d.Set("name", repo.Name) + d.Set("language", repo.Language) + d.Set("fork_policy", repo.ForkPolicy) + d.Set("website", repo.Website) + d.Set("description", repo.Description) + d.Set("project_key", repo.Project.Key) + + for _, clone_url := range repo.Links.Clone { + if clone_url.Name == "https" { + d.Set("clone_https", clone_url.Href) + } else { + d.Set("clone_ssh", clone_url.Href) + } } } @@ -230,18 +214,10 @@ func resourceRepositoryRead(d *schema.ResourceData, m interface{}) error { func resourceRepositoryDelete(d *schema.ResourceData, m interface{}) error { client := m.(*BitbucketClient) - delete_response, err := client.Delete(fmt.Sprintf("2.0/repositories/%s/%s", + _, err := client.Delete(fmt.Sprintf("2.0/repositories/%s/%s", d.Get("owner").(string), d.Get("name").(string), )) - if err != nil { - return err - } - - if delete_response.StatusCode != 204 { - return fmt.Errorf("Failed to delete the repository got status code %d", delete_response.StatusCode) - } - - return nil + return err } diff --git a/builtin/providers/bitbucket/resource_repository_test.go b/builtin/providers/bitbucket/resource_repository_test.go index 47d4f4405..1fa47a71f 100644 --- a/builtin/providers/bitbucket/resource_repository_test.go +++ b/builtin/providers/bitbucket/resource_repository_test.go @@ -16,9 +16,9 @@ func TestAccBitbucketRepository_basic(t *testing.T) { testAccBitbucketRepositoryConfig := fmt.Sprintf(` resource "bitbucket_repository" "test_repo" { owner = "%s" - name = "%s" + name = "test-repo-for-repository-test" } - `, testUser, testRepo) + `, testUser) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -42,11 +42,7 @@ func testAccCheckBitbucketRepositoryDestroy(s *terraform.State) error { return fmt.Errorf("Not found %s", "bitbucket_repository.test_repo") } - response, err := client.Get(fmt.Sprintf("2.0/repositories/%s/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["name"])) - - if err != nil { - return err - } + response, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["name"])) if response.StatusCode != 404 { return fmt.Errorf("Repository still exists")