Merge pull request #13390 from cwood/cwood/refactor-bitbucket-provider

Refactoring of bitbucket provider with better error support and general improvments

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
+	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{}
+	return c.Do("GET", endpoint, nil)
-	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)
-
 }
 
 func (c *BitbucketClient) Post(endpoint string, jsonpayload *bytes.Buffer) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("POST", endpoint, jsonpayload)
-	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)
 }
 
 func (c *BitbucketClient) Put(endpoint string, jsonpayload *bytes.Buffer) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("PUT", endpoint, jsonpayload)
-	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)
 }
 
 func (c *BitbucketClient) PutOnly(endpoint string) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("PUT", endpoint, nil)
-	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)
 }
 
 func (c *BitbucketClient) Delete(endpoint string) (*http.Response, error) {
-	client := &http.Client{}
+	return c.Do("DELETE", endpoint, nil)
-	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)
 }

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"
 )
@@ -32,6 +34,7 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 	client := &BitbucketClient{
 		Username:   d.Get("username").(string),
 		Password:   d.Get("password").(string),
+		HTTPClient: &http.Client{},
 	}
 
 	return client, nil

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 {
+		if reviewerResp.StatusCode != 200 {
-			return fmt.Errorf("Failed to create reviewer %s got code %d", user.(string), reviewer_resp.StatusCode)
+			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
 }

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,46 +85,56 @@ func resourceHookCreate(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
 	hook := createHook(d)
 
-	var jsonbuffer []byte
+	payload, err := json.Marshal(hook)
-
-	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)
 	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 {
+	log.Printf("ID: %s", url.PathEscape(d.Id()))
-		return err
-	}
 
+	if hook_req.StatusCode == 200 {
 		var hook Hook
 
-	decoder := json.NewDecoder(hook_req.Body)
+		body, readerr := ioutil.ReadAll(hook_req.Body)
-	err = decoder.Decode(&hook)
+		if readerr != nil {
-	if err != nil {
+			return readerr
-		return err
+		}
+
+		decodeerr := json.Unmarshal(body, &hook)
+		if decodeerr != nil {
+			return decodeerr
 		}
 
 		d.Set("uuid", hook.Uuid)
@@ -135,6 +149,7 @@ func resourceHookRead(d *schema.ResourceData, m interface{}) error {
 		}
 
 		d.Set("events", eventsList)
+	}
 
 	return nil
 }
@@ -142,25 +157,17 @@ func resourceHookRead(d *schema.ResourceData, m interface{}) error {
 func resourceHookUpdate(d *schema.ResourceData, m interface{}) error {
 	client := m.(*BitbucketClient)
 	hook := createHook(d)
-
+	payload, err := json.Marshal(hook)
-	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)
-
 	if err != nil {
 		return err
 	}
 
-	decoder := json.NewDecoder(hook_req.Body)
+	_, err = client.Put(fmt.Sprintf("2.0/repositories/%s/%s/hooks/%s",
-	err = decoder.Decode(&hook)
+		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, err
-			return false, nil
 		}
 
 		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
 }

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 {
+	if err == nil {
-		return err
+		return fmt.Errorf("The resource was found should have errored")
 	}
 
 	if response.StatusCode != 404 {

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 {
+	if err != nil {
-		return fmt.Errorf("Failed to create repository got status code %d", repo_req.StatusCode)
+		return err
 	}
 
 	d.SetId(string(fmt.Sprintf("%s/%s", d.Get("owner").(string), d.Get("name").(string))))
@@ -189,21 +170,23 @@ 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 {
+	if repo_req.StatusCode == 200 {
-		return err
-	}
 
 		var repo Repository
 
-	decoder := json.NewDecoder(repo_req.Body)
+		body, readerr := ioutil.ReadAll(repo_req.Body)
-	err = decoder.Decode(&repo)
+		if readerr != nil {
-	if err != nil {
+			return readerr
-		return err
+		}
+
+		decodeerr := json.Unmarshal(body, &repo)
+		if decodeerr != nil {
+			return decodeerr
 		}
 
 		d.Set("scm", repo.SCM)
@@ -224,24 +207,17 @@ func resourceRepositoryRead(d *schema.ResourceData, m interface{}) error {
 				d.Set("clone_ssh", clone_url.Href)
 			}
 		}
+	}
 
 	return nil
 }
 
 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
-}

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"]))
+	response, _ := client.Get(fmt.Sprintf("2.0/repositories/%s/%s", rs.Primary.Attributes["owner"], rs.Primary.Attributes["name"]))
-
-	if err != nil {
-		return err
-	}
 
 	if response.StatusCode != 404 {
 		return fmt.Errorf("Repository still exists")