Merge pull request #104 from svanharmelen/dev

Version v0.6.2

CHANGELOG.md

@@ -1,6 +1,15 @@
 Chef-Guard CHANGELOG
 ====================
 
+0.6.2
+-----
+- Do not log empty error messages but instead try to capture a more useful message based on the given error 
+- Add a shared `insecureTransport` transport that can be used by all connections, instead of creating seperate ones for each connection
+- Fix a bug that could cause a nil pointer exception when the Git diff was too large to include the patch in the commit details
+- Replaced the multisyncer package with an updated version hosted outside the Chef-Guard repo
+- Fix a bug that could cause a nil pointer exception while handling GitHub and GitLab error responses
+- Update to a new oauth2 package as the old one wasn't supported anymore
+
 0.6.1
 -----
 - Fixed a small regression where the `metadata.rb` and/or `metadata.json` are not handled correctly

VERSION

@@ -1 +1 @@
-0.6.1
+0.6.2

changes.go

@@ -28,10 +28,11 @@ import (
 	"github.com/gorilla/mux"
 )
 
+// Name of the entity we are changing
 type Name struct {
 	Name    string `json:"name"`
 	RawData struct {
-		Id string `json:"id"`
+		ID string `json:"id"`
 	} `json:"raw_data"`
 }
 
@@ -41,8 +42,8 @@ func unmarshalName(body []byte) (*Name, error) {
 		return nil, err
 	}
 	// Needed to get the correct name from data bag items
-	if n.RawData.Id != "" {
-		n.Name = n.RawData.Id
+	if n.RawData.ID != "" {
+		n.Name = n.RawData.ID
 	}
 	return &n, nil
 }
@@ -83,16 +84,17 @@ func processChange(p *httputil.ReverseProxy) func(http.ResponseWriter, *http.Req
 			return
 		}
 
-		r.URL, err = url.Parse(fmt.Sprintf(
-			"http://%s:%d%s?%s", cfg.Chef.ErchefIP, cfg.Chef.ErchefPort, r.URL.Path, r.URL.RawQuery))
+		u := fmt.Sprintf(
+			"http://%s:%d%s?%s",
+			cfg.Chef.ErchefIP,
+			cfg.Chef.ErchefPort,
+			r.URL.Path,
+			r.URL.RawQuery,
+		)
+
+		r.URL, err = url.Parse(u)
 		if err != nil {
-			errorHandler(w, fmt.Sprintf(
-				"Failed to parse URL %s: %s", fmt.Sprintf(
-					"http://%s:%d%s?%s",
-					cfg.Chef.ErchefIP,
-					cfg.Chef.ErchefPort,
-					r.URL.Path,
-					r.URL.RawQuery), err), http.StatusBadGateway)
+			errorHandler(w, fmt.Sprintf("Failed to parse URL %s: %s", u, err), http.StatusBadGateway)
 			return
 		}
 
@@ -104,18 +106,22 @@ func processChange(p *httputil.ReverseProxy) func(http.ResponseWriter, *http.Req
 		}
 		defer resp.Body.Close()
 
+		if err := checkHTTPResponse(resp, []int{http.StatusOK, http.StatusCreated}); err != nil {
+			if resp.StatusCode == http.StatusForbidden {
+				err = fmt.Errorf("%s %s for %s", r.Header.Get("X-Ops-Userid"), err, r.URL.Path)
+			}
+
+			errorHandler(w, err.Error(), resp.StatusCode)
+			return
+		}
+
 		respBody, err := ioutil.ReadAll(resp.Body)
 		if err != nil {
 			errorHandler(w, fmt.Sprintf(
 				"Failed to get body from call to %s: %s", r.URL.String(), err), http.StatusBadGateway)
 			return
 		}
 
-		if err := checkHTTPResponse(resp, []int{http.StatusOK, http.StatusCreated}); err != nil {
-			errorHandler(w, err.Error(), resp.StatusCode)
-			return
-		}
-
 		cg.ChangeDetails, err = getChangeDetails(r, reqBody)
 		if err != nil {
 			errorHandler(w, fmt.Sprintf(

chef-guard.go

@@ -17,23 +17,37 @@
 package main
 
 import (
+	"crypto/tls"
 	"flag"
 	"fmt"
 	"log"
+	"net"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
 	"os"
 	"os/signal"
 	"syscall"
+	"time"
 
 	"github.com/gorilla/mux"
 	"github.com/icub3d/graceful"
 	"github.com/marpaia/chef-golang"
 	"github.com/xanzy/chef-guard/git"
 )
 
-const VERSION = "0.6.0-UNRELEASED"
+// VERSION holds the current version
+const VERSION = "0.6.2"
+
+var insecureTransport = &http.Transport{
+	Proxy: http.ProxyFromEnvironment,
+	Dial: (&net.Dialer{
+		Timeout:   30 * time.Second,
+		KeepAlive: 30 * time.Second,
+	}).Dial,
+	TLSClientConfig:     &tls.Config{InsecureSkipVerify: true},
+	TLSHandshakeTimeout: 10 * time.Second,
+}
 
 // The ChefGuard struct holds all required info needed to process a request made through Chef-Guard
 type ChefGuard struct {
@@ -192,9 +206,9 @@ func startSignalHandler() chan struct{} {
 
 func errorHandler(w http.ResponseWriter, err string, statusCode int) {
 	switch statusCode {
-	case http.StatusNotFound:
-		// No need to write anything to the log for this one...
 	case http.StatusPreconditionFailed:
+		// No need to write anything to the log for this one...
+	case http.StatusNotFound:
 		WARNING.Println(err)
 	default:
 		ERROR.Println(err)

clients.go

@@ -96,7 +96,6 @@ func processDownload(w http.ResponseWriter, r *http.Request) {
 
 			targetmd5 := md5.Sum(data)
 			targetsha := sha256.Sum256(data)
-			data = nil
 
 			fmt.Fprintf(w, "url %s\nmd5 %x\nsha256 %x", targeturl, targetmd5, targetsha)
 		}

config.go

@@ -24,7 +24,7 @@ import (
 	"regexp"
 	"strings"
 
-	"code.google.com/p/gcfg"
+	"gopkg.in/gcfg.v1"
 	"github.com/mitchellh/osext"
 	"github.com/xanzy/chef-guard/git"
 )

cookbook.go

@@ -23,9 +23,9 @@ import (
 	"crypto/hmac"
 	"crypto/md5"
 	"crypto/sha1"
-	"crypto/tls"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -111,15 +111,16 @@ func (cg *ChefGuard) processCookbookFiles() error {
 	gw := gzip.NewWriter(buf)
 	tw := tar.NewWriter(gw)
 
-	t := &http.Transport{
-		TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.Chef.SSLNoVerify},
+	client := http.DefaultClient
+
+	if cfg.Chef.SSLNoVerify {
+		client = &http.Client{Transport: insecureTransport}
 	}
-	c := &http.Client{Transport: t}
 
 	// Let's first find and save the .gitignore and chefignore files
 	for _, f := range cg.Cookbook.RootFiles {
 		if f.Name == ".gitignore" || f.Name == "chefignore" {
-			content, err := downloadCookbookFile(c, *cg.OrganizationID, f.Checksum)
+			content, err := downloadCookbookFile(client, *cg.OrganizationID, f.Checksum)
 			if err != nil {
 				return fmt.Errorf("Failed to dowload %s from the %s cookbook: %s", f.Path, cg.Cookbook.Name, err)
 			}
@@ -143,7 +144,7 @@ func (cg *ChefGuard) processCookbookFiles() error {
 			continue
 		}
 
-		content, err := downloadCookbookFile(c, *cg.OrganizationID, f.Checksum)
+		content, err := downloadCookbookFile(client, *cg.OrganizationID, f.Checksum)
 		if err != nil {
 			return fmt.Errorf("Failed to dowload %s from the %s cookbook: %s", f.Path, cg.Cookbook.Name, err)
 		}
@@ -395,27 +396,49 @@ func addMetadataJSON(tw *tar.Writer, cb *chef.CookbookVersion) error {
 	return nil
 }
 
+// ErrorInfo is single type used for several different types of errors
+type ErrorInfo struct {
+	Error         []string `json:"error,omitempty"`
+	Errors        []string `json:"errors,omitempty"`
+	ErrorMessages []string `json:"error_messages,omitempty"`
+}
+
 func checkHTTPResponse(resp *http.Response, allowedStates []int) error {
 	for _, s := range allowedStates {
 		if resp.StatusCode == s {
 			return nil
 		}
 	}
-	errInfo := new(ErrorInfo)
+
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
 		return fmt.Errorf("Failed to get body from call to %s: %s", resp.Request.URL.String(), err)
 	}
+
+	// Make sure we return an error, even if we have no error details
+	if len(body) == 0 {
+		return errors.New("No error details found")
+	}
+
 	// If this returns an error the return body is probably not JSON,
 	// in which case we just move on and return the raw body instead.
+	// Otherwise let's see if we parsed out some error details and
+	// return those.
+	errInfo := &ErrorInfo{}
 	if err := json.Unmarshal(body, errInfo); err == nil {
+		if errInfo.Error != nil {
+			return fmt.Errorf(strings.Join(errInfo.Error, ";"))
+		}
 		if errInfo.Errors != nil {
 			return fmt.Errorf(strings.Join(errInfo.Errors, ";"))
 		}
 		if errInfo.ErrorMessages != nil {
 			return fmt.Errorf(strings.Join(errInfo.ErrorMessages, ";"))
 		}
 	}
+
+	// If we could not marshal the body or we didn't parse any errors
+	// just return the raw body.
 	return fmt.Errorf(string(body))
 }
 

git.go

@@ -25,7 +25,7 @@ import (
 	"strings"
 
 	"github.com/xanzy/chef-guard/git"
-	"github.com/xanzy/chef-guard/multisyncer"
+	"github.com/xanzy/multisyncer"
 )
 
 var ms multisyncer.MultiSyncer
@@ -35,10 +35,8 @@ func (cg *ChefGuard) syncedGitUpdate(action string, body []byte) {
 		ms = multisyncer.New()
 	}
 
-	token := <-ms.GetToken(cg.Repo)
-	defer func() {
-		ms.ReturnToken(cg.Repo) <- token
-	}()
+	ms.Lock(cg.Repo)
+	defer ms.Unlock(cg.Repo)
 
 	config, err := remarshalConfig(action, body)
 	if err != nil {

git/git.go

@@ -17,18 +17,29 @@
 package git
 
 import (
+	"crypto/tls"
 	"fmt"
+	"net"
 	"net/http"
 	"net/url"
 	"strings"
+	"time"
 
-	"crypto/tls"
-
-	"code.google.com/p/goauth2/oauth"
 	"github.com/google/go-github/github"
 	"github.com/xanzy/go-gitlab"
+	"golang.org/x/oauth2"
 )
 
+var insecureTransport = &http.Transport{
+	Proxy: http.ProxyFromEnvironment,
+	Dial: (&net.Dialer{
+		Timeout:   30 * time.Second,
+		KeepAlive: 30 * time.Second,
+	}).Dial,
+	TLSClientConfig:     &tls.Config{InsecureSkipVerify: true},
+	TLSHandshakeTimeout: 10 * time.Second,
+}
+
 // Git is an interface that must be implemented by any git service
 // that can be used with Chef-Guard
 type Git interface {
@@ -92,6 +103,7 @@ type GitHub struct {
 // GitLab represents a GitLab client
 type GitLab struct {
 	client *gitlab.Client
+	token  string
 }
 
 // NewGitClient returns either a GitHub or GitLab client as Git interface
@@ -107,16 +119,18 @@ func NewGitClient(c *Config) (Git, error) {
 }
 
 func newGitHubClient(c *Config) (Git, error) {
-	t := &oauth.Transport{
-		Token: &oauth.Token{AccessToken: c.Token},
-		Transport: &http.Transport{
-			Proxy:           http.ProxyFromEnvironment,
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: c.SSLNoVerify},
+	client := &http.Client{
+		Transport: &oauth2.Transport{
+			Source: oauth2.StaticTokenSource(&oauth2.Token{AccessToken: c.Token}),
 		},
 	}
 
+	if c.SSLNoVerify {
+		client.Transport.(*oauth2.Transport).Base = insecureTransport
+	}
+
 	g := new(GitHub)
-	g.client = github.NewClient(t.Client())
+	g.client = github.NewClient(client)
 
 	if c.ServerURL != "" {
 		// Make sure the URL ends with a single forward slash as the go-github package requires that
@@ -132,18 +146,19 @@ func newGitHubClient(c *Config) (Git, error) {
 }
 
 func newGitLabClient(c *Config) (Git, error) {
-	client := &http.Client{
-		Transport: &http.Transport{
-			Proxy:           http.ProxyFromEnvironment,
-			TLSClientConfig: &tls.Config{InsecureSkipVerify: c.SSLNoVerify},
-		},
+	client := http.DefaultClient
+
+	if c.SSLNoVerify {
+		client = &http.Client{Transport: insecureTransport}
 	}
 
-	g := new(GitLab)
+	g := &GitLab{token: c.Token}
 	g.client = gitlab.NewClient(client, c.Token)
 
 	if c.ServerURL != "" {
-		g.client.SetBaseURL(c.ServerURL)
+		if err := g.client.SetBaseURL(c.ServerURL); err != nil {
+			return nil, err
+		}
 	}
 
 	return g, nil