Merge pull request #33477 from dnephin/move-builder-pkgs

Split and remove pkg/gitutils and pkg/httputils

builder/dockerfile/copy.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/remotecontext"
-	"github.com/docker/docker/pkg/httputils"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
@@ -292,7 +291,6 @@ func errOnSourceDownload(_ string) (builder.Source, string, error) {
 }
 
 func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote builder.Source, p string, err error) {
-	// get filename from URL
 	u, err := url.Parse(srcURL)
 	if err != nil {
 		return
@@ -303,8 +301,7 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b
 		return
 	}
 
-	// Initiate the download
-	resp, err := httputils.Download(srcURL)
+	resp, err := remotecontext.GetWithStatusError(srcURL)
 	if err != nil {
 		return
 	}

builder/remotecontext/detect.go

@@ -14,7 +14,6 @@ import (
 	"github.com/docker/docker/builder/dockerfile/parser"
 	"github.com/docker/docker/builder/dockerignore"
 	"github.com/docker/docker/pkg/fileutils"
-	"github.com/docker/docker/pkg/httputils"
 	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/urlutil"
 	"github.com/pkg/errors"
@@ -93,7 +92,7 @@ func newURLRemote(url string, dockerfilePath string, progressReader func(in io.R
 	var dockerfile io.ReadCloser
 	dockerfileFoundErr := errors.New("found-dockerfile")
 	c, err := MakeRemoteContext(url, map[string]func(io.ReadCloser) (io.ReadCloser, error){
-		httputils.MimeTypes.TextPlain: func(rc io.ReadCloser) (io.ReadCloser, error) {
+		mimeTypes.TextPlain: func(rc io.ReadCloser) (io.ReadCloser, error) {
 			dockerfile = rc
 			return nil, dockerfileFoundErr
 		},

builder/remotecontext/git.go

@@ -4,13 +4,13 @@ import (
 	"os"
 
 	"github.com/docker/docker/builder"
+	"github.com/docker/docker/builder/remotecontext/git"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/gitutils"
 )
 
 // MakeGitContext returns a Context from gitURL that is cloned in a temporary directory.
 func MakeGitContext(gitURL string) (builder.Source, error) {
-	root, err := gitutils.Clone(gitURL)
+	root, err := git.Clone(gitURL)
 	if err != nil {
 		return nil, err
 	}

pkg/gitutils/gitutils.go → builder/remotecontext/git/gitutils.go

@@ -1,4 +1,4 @@
-package gitutils
+package git
 
 import (
 	"fmt"

pkg/gitutils/gitutils_test.go → builder/remotecontext/git/gitutils_test.go

@@ -1,4 +1,4 @@
-package gitutils
+package git
 
 import (
 	"fmt"
@@ -8,10 +8,12 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"reflect"
 	"runtime"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCloneArgsSmartHttp(t *testing.T) {
@@ -28,9 +30,7 @@ func TestCloneArgsSmartHttp(t *testing.T) {
 
 	args := fetchArgs(serverURL, "master")
 	exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"}
-	if !reflect.DeepEqual(args, exp) {
-		t.Fatalf("Expected %v, got %v", exp, args)
-	}
+	assert.Equal(t, exp, args)
 }
 
 func TestCloneArgsDumbHttp(t *testing.T) {
@@ -46,18 +46,14 @@ func TestCloneArgsDumbHttp(t *testing.T) {
 
 	args := fetchArgs(serverURL, "master")
 	exp := []string{"fetch", "--recurse-submodules=yes", "origin", "master"}
-	if !reflect.DeepEqual(args, exp) {
-		t.Fatalf("Expected %v, got %v", exp, args)
-	}
+	assert.Equal(t, exp, args)
 }
 
 func TestCloneArgsGit(t *testing.T) {
 	u, _ := url.Parse("git://github.com/docker/docker")
 	args := fetchArgs(u, "master")
 	exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"}
-	if !reflect.DeepEqual(args, exp) {
-		t.Fatalf("Expected %v, got %v", exp, args)
-	}
+	assert.Equal(t, exp, args)
 }
 
 func gitGetConfig(name string) string {
@@ -72,9 +68,7 @@ func gitGetConfig(name string) string {
 
 func TestCheckoutGit(t *testing.T) {
 	root, err := ioutil.TempDir("", "docker-build-git-checkout")
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 	defer os.RemoveAll(root)
 
 	autocrlf := gitGetConfig("core.autocrlf")
@@ -89,30 +83,22 @@ func TestCheckoutGit(t *testing.T) {
 
 	gitDir := filepath.Join(root, "repo")
 	_, err = git("init", gitDir)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
-	if _, err = gitWithinDir(gitDir, "config", "user.email", "test@docker.com"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "config", "user.email", "test@docker.com")
+	require.NoError(t, err)
 
-	if _, err = gitWithinDir(gitDir, "config", "user.name", "Docker test"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "config", "user.name", "Docker test")
+	require.NoError(t, err)
 
-	if err = ioutil.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644); err != nil {
-		t.Fatal(err)
-	}
+	err = ioutil.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch"), 0644)
+	require.NoError(t, err)
 
 	subDir := filepath.Join(gitDir, "subdir")
-	if err = os.Mkdir(subDir, 0755); err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, os.Mkdir(subDir, 0755))
 
-	if err = ioutil.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 5000"), 0644); err != nil {
-		t.Fatal(err)
-	}
+	err = ioutil.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 5000"), 0644)
+	require.NoError(t, err)
 
 	if runtime.GOOS != "windows" {
 		if err = os.Symlink("../subdir", filepath.Join(gitDir, "parentlink")); err != nil {
@@ -124,37 +110,29 @@ func TestCheckoutGit(t *testing.T) {
 		}
 	}
 
-	if _, err = gitWithinDir(gitDir, "add", "-A"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "add", "-A")
+	require.NoError(t, err)
 
-	if _, err = gitWithinDir(gitDir, "commit", "-am", "First commit"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "commit", "-am", "First commit")
+	require.NoError(t, err)
 
-	if _, err = gitWithinDir(gitDir, "checkout", "-b", "test"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "checkout", "-b", "test")
+	require.NoError(t, err)
 
-	if err = ioutil.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 3000"), 0644); err != nil {
-		t.Fatal(err)
-	}
+	err = ioutil.WriteFile(filepath.Join(gitDir, "Dockerfile"), []byte("FROM scratch\nEXPOSE 3000"), 0644)
+	require.NoError(t, err)
 
-	if err = ioutil.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM busybox\nEXPOSE 5000"), 0644); err != nil {
-		t.Fatal(err)
-	}
+	err = ioutil.WriteFile(filepath.Join(subDir, "Dockerfile"), []byte("FROM busybox\nEXPOSE 5000"), 0644)
+	require.NoError(t, err)
 
-	if _, err = gitWithinDir(gitDir, "add", "-A"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "add", "-A")
+	require.NoError(t, err)
 
-	if _, err = gitWithinDir(gitDir, "commit", "-am", "Branch commit"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "commit", "-am", "Branch commit")
+	require.NoError(t, err)
 
-	if _, err = gitWithinDir(gitDir, "checkout", "master"); err != nil {
-		t.Fatal(err)
-	}
+	_, err = gitWithinDir(gitDir, "checkout", "master")
+	require.NoError(t, err)
 
 	type singleCase struct {
 		frag string
@@ -190,21 +168,13 @@ func TestCheckoutGit(t *testing.T) {
 		ref, subdir := getRefAndSubdir(c.frag)
 		r, err := checkoutGit(gitDir, ref, subdir)
 
-		fail := err != nil
-		if fail != c.fail {
-			t.Fatalf("Expected %v failure, error was %v\n", c.fail, err)
-		}
 		if c.fail {
+			assert.Error(t, err)
 			continue
 		}
 
 		b, err := ioutil.ReadFile(filepath.Join(r, "Dockerfile"))
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		if string(b) != c.exp {
-			t.Fatalf("Expected %v, was %v\n", c.exp, string(b))
-		}
+		require.NoError(t, err)
+		assert.Equal(t, c.exp, string(b))
 	}
 }

pkg/httputils/mimetype.go → builder/remotecontext/mimetype.go

@@ -1,29 +1,27 @@
-package httputils
+package remotecontext
 
 import (
 	"mime"
 	"net/http"
 )
 
-// MimeTypes stores the MIME content type.
-var MimeTypes = struct {
+// mimeTypes stores the MIME content type.
+var mimeTypes = struct {
 	TextPlain   string
 	OctetStream string
 }{"text/plain", "application/octet-stream"}
 
-// DetectContentType returns a best guess representation of the MIME
+// detectContentType returns a best guess representation of the MIME
 // content type for the bytes at c.  The value detected by
 // http.DetectContentType is guaranteed not be nil, defaulting to
 // application/octet-stream when a better guess cannot be made. The
 // result of this detection is then run through mime.ParseMediaType()
 // which separates the actual MIME string from any parameters.
-func DetectContentType(c []byte) (string, map[string]string, error) {
-
+func detectContentType(c []byte) (string, map[string]string, error) {
 	ct := http.DetectContentType(c)
 	contentType, args, err := mime.ParseMediaType(ct)
 	if err != nil {
 		return "", nil, err
 	}
-
 	return contentType, args, nil
 }

builder/remotecontext/mimetype_test.go

@@ -0,0 +1,16 @@
+package remotecontext
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDetectContentType(t *testing.T) {
+	input := []byte("That is just a plain text")
+
+	contentType, _, err := detectContentType(input)
+	require.NoError(t, err)
+	assert.Equal(t, "text/plain", contentType)
+}

builder/remotecontext/remote.go

@@ -2,14 +2,14 @@ package remotecontext
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/http"
 	"regexp"
 
 	"github.com/docker/docker/builder"
-	"github.com/docker/docker/pkg/httputils"
+	"github.com/pkg/errors"
 )
 
 // When downloading remote contexts, limit the amount (in bytes)
@@ -30,7 +30,7 @@ var mimeRe = regexp.MustCompile(acceptableRemoteMIME)
 // to be returned. If no match is found, it is assumed the body is a tar stream (compressed or not).
 // In either case, an (assumed) tar stream is passed to MakeTarSumContext whose result is returned.
 func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io.ReadCloser) (io.ReadCloser, error)) (builder.Source, error) {
-	f, err := httputils.Download(remoteURL)
+	f, err := GetWithStatusError(remoteURL)
 	if err != nil {
 		return nil, fmt.Errorf("error downloading remote context %s: %v", remoteURL, err)
 	}
@@ -66,6 +66,24 @@ func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io.
 	return MakeTarSumContext(contextReader)
 }
 
+// GetWithStatusError does an http.Get() and returns an error if the
+// status code is 4xx or 5xx.
+func GetWithStatusError(url string) (resp *http.Response, err error) {
+	if resp, err = http.Get(url); err != nil {
+		return nil, err
+	}
+	if resp.StatusCode < 400 {
+		return resp, nil
+	}
+	msg := fmt.Sprintf("failed to GET %s with status %s", url, resp.Status)
+	body, err := ioutil.ReadAll(resp.Body)
+	resp.Body.Close()
+	if err != nil {
+		return nil, errors.Wrapf(err, msg+": error reading body")
+	}
+	return nil, errors.Errorf(msg+": %s", bytes.TrimSpace(body))
+}
+
 // inspectResponse looks into the http response data at r to determine whether its
 // content-type is on the list of acceptable content types for remote build contexts.
 // This function returns:
@@ -94,8 +112,8 @@ func inspectResponse(ct string, r io.ReadCloser, clen int64) (string, io.ReadClo
 	// content type for files without an extension (e.g. 'Dockerfile')
 	// so if we receive this value we better check for text content
 	contentType := ct
-	if len(ct) == 0 || ct == httputils.MimeTypes.OctetStream {
-		contentType, _, err = httputils.DetectContentType(preamble)
+	if len(ct) == 0 || ct == mimeTypes.OctetStream {
+		contentType, _, err = detectContentType(preamble)
 		if err != nil {
 			return contentType, bodyReader, err
 		}

builder/remotecontext/remote_test.go

@@ -11,7 +11,9 @@ import (
 
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/httputils"
+	"github.com/docker/docker/pkg/testutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var binaryContext = []byte{0xFD, 0x37, 0x7A, 0x58, 0x5A, 0x00} //xz magic
@@ -188,7 +190,7 @@ func TestMakeRemoteContext(t *testing.T) {
 	mux.Handle("/", http.FileServer(http.Dir(contextDir)))
 
 	remoteContext, err := MakeRemoteContext(remoteURL, map[string]func(io.ReadCloser) (io.ReadCloser, error){
-		httputils.MimeTypes.TextPlain: func(rc io.ReadCloser) (io.ReadCloser, error) {
+		mimeTypes.TextPlain: func(rc io.ReadCloser) (io.ReadCloser, error) {
 			dockerfile, err := ioutil.ReadAll(rc)
 			if err != nil {
 				return nil, err
@@ -232,3 +234,43 @@ func TestMakeRemoteContext(t *testing.T) {
 		t.Fatalf("File %s should have position 0, got %d", builder.DefaultDockerfileName, fileInfo.Pos())
 	}
 }
+
+func TestGetWithStatusError(t *testing.T) {
+	var testcases = []struct {
+		err          error
+		statusCode   int
+		expectedErr  string
+		expectedBody string
+	}{
+		{
+			statusCode:   200,
+			expectedBody: "THE BODY",
+		},
+		{
+			statusCode:   400,
+			expectedErr:  "with status 400 Bad Request: broke",
+			expectedBody: "broke",
+		},
+	}
+	for _, testcase := range testcases {
+		ts := httptest.NewServer(
+			http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				buffer := bytes.NewBufferString(testcase.expectedBody)
+				w.WriteHeader(testcase.statusCode)
+				w.Write(buffer.Bytes())
+			}),
+		)
+		defer ts.Close()
+		response, err := GetWithStatusError(ts.URL)
+
+		if testcase.expectedErr == "" {
+			require.NoError(t, err)
+
+			body, err := testutil.ReadBody(response.Body)
+			require.NoError(t, err)
+			assert.Contains(t, string(body), testcase.expectedBody)
+		} else {
+			testutil.ErrorContains(t, err, testcase.expectedErr)
+		}
+	}
+}

daemon/import.go

@@ -12,11 +12,11 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/dockerfile"
+	"github.com/docker/docker/builder/remotecontext"
 	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/httputils"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/pkg/errors"
@@ -67,7 +67,7 @@ func (daemon *Daemon) ImportImage(src string, repository, tag string, msg string
 			return err
 		}
 
-		resp, err = httputils.Download(u.String())
+		resp, err = remotecontext.GetWithStatusError(u.String())
 		if err != nil {
 			return err
 		}

pkg/httputils/httputils.go

@@ -1,56 +0,0 @@
-package httputils
-
-import (
-	"errors"
-	"fmt"
-	"net/http"
-	"regexp"
-	"strings"
-
-	"github.com/docker/docker/pkg/jsonmessage"
-)
-
-var (
-	headerRegexp     = regexp.MustCompile(`^(?:(.+)/(.+?))\((.+)\).*$`)
-	errInvalidHeader = errors.New("Bad header, should be in format `docker/version (platform)`")
-)
-
-// Download requests a given URL and returns an io.Reader.
-func Download(url string) (resp *http.Response, err error) {
-	if resp, err = http.Get(url); err != nil {
-		return nil, err
-	}
-	if resp.StatusCode >= 400 {
-		return nil, fmt.Errorf("Got HTTP status code >= 400: %s", resp.Status)
-	}
-	return resp, nil
-}
-
-// NewHTTPRequestError returns a JSON response error.
-func NewHTTPRequestError(msg string, res *http.Response) error {
-	return &jsonmessage.JSONError{
-		Message: msg,
-		Code:    res.StatusCode,
-	}
-}
-
-// ServerHeader contains the server information.
-type ServerHeader struct {
-	App string // docker
-	Ver string // 1.8.0-dev
-	OS  string // windows or linux
-}
-
-// ParseServerHeader extracts pieces from an HTTP server header
-// which is in the format "docker/version (os)" e.g. docker/1.8.0-dev (windows).
-func ParseServerHeader(hdr string) (*ServerHeader, error) {
-	matches := headerRegexp.FindStringSubmatch(hdr)
-	if len(matches) != 4 {
-		return nil, errInvalidHeader
-	}
-	return &ServerHeader{
-		App: strings.TrimSpace(matches[1]),
-		Ver: strings.TrimSpace(matches[2]),
-		OS:  strings.TrimSpace(matches[3]),
-	}, nil
-}

pkg/httputils/httputils_test.go

@@ -1,115 +0,0 @@
-package httputils
-
-import (
-	"fmt"
-	"io/ioutil"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-)
-
-func TestDownload(t *testing.T) {
-	expected := "Hello, docker !"
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		fmt.Fprintf(w, expected)
-	}))
-	defer ts.Close()
-	response, err := Download(ts.URL)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	actual, err := ioutil.ReadAll(response.Body)
-	response.Body.Close()
-
-	if err != nil || string(actual) != expected {
-		t.Fatalf("Expected the response %q, got err:%q, actual:%q", expected, err, string(actual))
-	}
-}
-
-func TestDownload400Errors(t *testing.T) {
-	expectedError := "Got HTTP status code >= 400: 403 Forbidden"
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// 403
-		http.Error(w, "something failed (forbidden)", http.StatusForbidden)
-	}))
-	defer ts.Close()
-	// Expected status code = 403
-	if _, err := Download(ts.URL); err == nil || err.Error() != expectedError {
-		t.Fatalf("Expected the error %q, got %q", expectedError, err)
-	}
-}
-
-func TestDownloadOtherErrors(t *testing.T) {
-	if _, err := Download("I'm not an url.."); err == nil || !strings.Contains(err.Error(), "unsupported protocol scheme") {
-		t.Fatalf("Expected an error with 'unsupported protocol scheme', got %q", err)
-	}
-}
-
-func TestNewHTTPRequestError(t *testing.T) {
-	errorMessage := "Some error message"
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// 403
-		http.Error(w, errorMessage, http.StatusForbidden)
-	}))
-	defer ts.Close()
-	httpResponse, err := http.Get(ts.URL)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := NewHTTPRequestError(errorMessage, httpResponse); err.Error() != errorMessage {
-		t.Fatalf("Expected err to be %q, got %q", errorMessage, err)
-	}
-}
-
-func TestParseServerHeader(t *testing.T) {
-	inputs := map[string][]string{
-		"bad header":           {"error"},
-		"(bad header)":         {"error"},
-		"(without/spaces)":     {"error"},
-		"(header/with spaces)": {"error"},
-		"foo/bar (baz)":        {"foo", "bar", "baz"},
-		"foo/bar":              {"error"},
-		"foo":                  {"error"},
-		"foo/bar (baz space)":           {"foo", "bar", "baz space"},
-		"  f  f  /  b  b  (  b  s  )  ": {"f  f", "b  b", "b  s"},
-		"foo/bar (baz) ignore":          {"foo", "bar", "baz"},
-		"foo/bar ()":                    {"error"},
-		"foo/bar()":                     {"error"},
-		"foo/bar(baz)":                  {"foo", "bar", "baz"},
-		"foo/bar/zzz(baz)":              {"foo/bar", "zzz", "baz"},
-		"foo/bar(baz/abc)":              {"foo", "bar", "baz/abc"},
-		"foo/bar(baz (abc))":            {"foo", "bar", "baz (abc)"},
-	}
-
-	for header, values := range inputs {
-		serverHeader, err := ParseServerHeader(header)
-		if err != nil {
-			if err != errInvalidHeader {
-				t.Fatalf("Failed to parse %q, and got some unexpected error: %q", header, err)
-			}
-			if values[0] == "error" {
-				continue
-			}
-			t.Fatalf("Header %q failed to parse when it shouldn't have", header)
-		}
-		if values[0] == "error" {
-			t.Fatalf("Header %q parsed ok when it should have failed(%q).", header, serverHeader)
-		}
-
-		if serverHeader.App != values[0] {
-			t.Fatalf("Expected serverHeader.App for %q to equal %q, got %q", header, values[0], serverHeader.App)
-		}
-
-		if serverHeader.Ver != values[1] {
-			t.Fatalf("Expected serverHeader.Ver for %q to equal %q, got %q", header, values[1], serverHeader.Ver)
-		}
-
-		if serverHeader.OS != values[2] {
-			t.Fatalf("Expected serverHeader.OS for %q to equal %q, got %q", header, values[2], serverHeader.OS)
-		}
-
-	}
-
-}

pkg/httputils/mimetype_test.go

@@ -1,13 +0,0 @@
-package httputils
-
-import (
-	"testing"
-)
-
-func TestDetectContentType(t *testing.T) {
-	input := []byte("That is just a plain text")
-
-	if contentType, _, err := DetectContentType(input); err != nil || contentType != "text/plain" {
-		t.Error("TestDetectContentType failed")
-	}
-}

pkg/httputils/resumablerequestreader.go → registry/resumable/resumablerequestreader.go

@@ -1,4 +1,4 @@
-package httputils
+package resumable
 
 import (
 	"fmt"
@@ -9,7 +9,7 @@ import (
 	"github.com/Sirupsen/logrus"
 )
 
-type resumableRequestReader struct {
+type requestReader struct {
 	client          *http.Client
 	request         *http.Request
 	lastRange       int64
@@ -20,22 +20,22 @@ type resumableRequestReader struct {
 	waitDuration    time.Duration
 }
 
-// ResumableRequestReader makes it possible to resume reading a request's body transparently
+// NewRequestReader makes it possible to resume reading a request's body transparently
 // maxfail is the number of times we retry to make requests again (not resumes)
 // totalsize is the total length of the body; auto detect if not provided
-func ResumableRequestReader(c *http.Client, r *http.Request, maxfail uint32, totalsize int64) io.ReadCloser {
-	return &resumableRequestReader{client: c, request: r, maxFailures: maxfail, totalSize: totalsize, waitDuration: 5 * time.Second}
+func NewRequestReader(c *http.Client, r *http.Request, maxfail uint32, totalsize int64) io.ReadCloser {
+	return &requestReader{client: c, request: r, maxFailures: maxfail, totalSize: totalsize, waitDuration: 5 * time.Second}
 }
 
-// ResumableRequestReaderWithInitialResponse makes it possible to resume
+// NewRequestReaderWithInitialResponse makes it possible to resume
 // reading the body of an already initiated request.
-func ResumableRequestReaderWithInitialResponse(c *http.Client, r *http.Request, maxfail uint32, totalsize int64, initialResponse *http.Response) io.ReadCloser {
-	return &resumableRequestReader{client: c, request: r, maxFailures: maxfail, totalSize: totalsize, currentResponse: initialResponse, waitDuration: 5 * time.Second}
+func NewRequestReaderWithInitialResponse(c *http.Client, r *http.Request, maxfail uint32, totalsize int64, initialResponse *http.Response) io.ReadCloser {
+	return &requestReader{client: c, request: r, maxFailures: maxfail, totalSize: totalsize, currentResponse: initialResponse, waitDuration: 5 * time.Second}
 }
 
-func (r *resumableRequestReader) Read(p []byte) (n int, err error) {
+func (r *requestReader) Read(p []byte) (n int, err error) {
 	if r.client == nil || r.request == nil {
-		return 0, fmt.Errorf("client and request can't be nil\n")
+		return 0, fmt.Errorf("client and request can't be nil")
 	}
 	isFreshRequest := false
 	if r.lastRange != 0 && r.currentResponse == nil {
@@ -81,14 +81,14 @@ func (r *resumableRequestReader) Read(p []byte) (n int, err error) {
 	return n, err
 }
 
-func (r *resumableRequestReader) Close() error {
+func (r *requestReader) Close() error {
 	r.cleanUpResponse()
 	r.client = nil
 	r.request = nil
 	return nil
 }
 
-func (r *resumableRequestReader) cleanUpResponse() {
+func (r *requestReader) cleanUpResponse() {
 	if r.currentResponse != nil {
 		r.currentResponse.Body.Close()
 		r.currentResponse = nil

pkg/httputils/resumablerequestreader_test.go → registry/resumable/resumablerequestreader_test.go

@@ -1,7 +1,9 @@
-package httputils
+package resumable
 
 import (
 	"fmt"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -21,28 +23,19 @@ func TestResumableRequestHeaderSimpleErrors(t *testing.T) {
 
 	var req *http.Request
 	req, err := http.NewRequest("GET", ts.URL, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
-	expectedError := "client and request can't be nil\n"
-	resreq := &resumableRequestReader{}
+	resreq := &requestReader{}
 	_, err = resreq.Read([]byte{})
-	if err == nil || err.Error() != expectedError {
-		t.Fatalf("Expected an error with '%s', got %v.", expectedError, err)
-	}
+	assert.EqualError(t, err, "client and request can't be nil")
 
-	resreq = &resumableRequestReader{
+	resreq = &requestReader{
 		client:    client,
 		request:   req,
 		totalSize: -1,
 	}
-	expectedError = "failed to auto detect content length"
 	_, err = resreq.Read([]byte{})
-	if err == nil || err.Error() != expectedError {
-		t.Fatalf("Expected an error with '%s', got %v.", expectedError, err)
-	}
-
+	assert.EqualError(t, err, "failed to auto detect content length")
 }
 
 // Not too much failures, bails out after some wait
@@ -51,11 +44,9 @@ func TestResumableRequestHeaderNotTooMuchFailures(t *testing.T) {
 
 	var badReq *http.Request
 	badReq, err := http.NewRequest("GET", "I'm not an url", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
-	resreq := &resumableRequestReader{
+	resreq := &requestReader{
 		client:       client,
 		request:      badReq,
 		failures:     0,
@@ -63,9 +54,8 @@ func TestResumableRequestHeaderNotTooMuchFailures(t *testing.T) {
 		waitDuration: 10 * time.Millisecond,
 	}
 	read, err := resreq.Read([]byte{})
-	if err != nil || read != 0 {
-		t.Fatalf("Expected no error and no byte read, got err:%v, read:%v.", err, read)
-	}
+	require.NoError(t, err)
+	assert.Equal(t, 0, read)
 }
 
 // Too much failures, returns the error
@@ -74,11 +64,9 @@ func TestResumableRequestHeaderTooMuchFailures(t *testing.T) {
 
 	var badReq *http.Request
 	badReq, err := http.NewRequest("GET", "I'm not an url", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
-	resreq := &resumableRequestReader{
+	resreq := &requestReader{
 		client:      client,
 		request:     badReq,
 		failures:    0,
@@ -88,9 +76,8 @@ func TestResumableRequestHeaderTooMuchFailures(t *testing.T) {
 
 	expectedError := `Get I%27m%20not%20an%20url: unsupported protocol scheme ""`
 	read, err := resreq.Read([]byte{})
-	if err == nil || err.Error() != expectedError || read != 0 {
-		t.Fatalf("Expected the error '%s', got err:%v, read:%v.", expectedError, err, read)
-	}
+	assert.EqualError(t, err, expectedError)
+	assert.Equal(t, 0, read)
 }
 
 type errorReaderCloser struct{}
@@ -105,9 +92,7 @@ func (errorReaderCloser) Read(p []byte) (n int, err error) {
 func TestResumableRequestReaderWithReadError(t *testing.T) {
 	var req *http.Request
 	req, err := http.NewRequest("GET", "", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	client := &http.Client{}
 
@@ -119,7 +104,7 @@ func TestResumableRequestReaderWithReadError(t *testing.T) {
 		Body:          errorReaderCloser{},
 	}
 
-	resreq := &resumableRequestReader{
+	resreq := &requestReader{
 		client:          client,
 		request:         req,
 		currentResponse: response,
@@ -130,21 +115,15 @@ func TestResumableRequestReaderWithReadError(t *testing.T) {
 
 	buf := make([]byte, 1)
 	read, err := resreq.Read(buf)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
-	if read != 0 {
-		t.Fatalf("Expected to have read nothing, but read %v", read)
-	}
+	assert.Equal(t, 0, read)
 }
 
 func TestResumableRequestReaderWithEOFWith416Response(t *testing.T) {
 	var req *http.Request
 	req, err := http.NewRequest("GET", "", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	client := &http.Client{}
 
@@ -156,7 +135,7 @@ func TestResumableRequestReaderWithEOFWith416Response(t *testing.T) {
 		Body:          ioutil.NopCloser(strings.NewReader("")),
 	}
 
-	resreq := &resumableRequestReader{
+	resreq := &requestReader{
 		client:          client,
 		request:         req,
 		currentResponse: response,
@@ -167,9 +146,7 @@ func TestResumableRequestReaderWithEOFWith416Response(t *testing.T) {
 
 	buf := make([]byte, 1)
 	_, err = resreq.Read(buf)
-	if err == nil || err != io.EOF {
-		t.Fatalf("Expected an io.EOF error, got %v", err)
-	}
+	assert.EqualError(t, err, io.EOF.Error())
 }
 
 func TestResumableRequestReaderWithServerDoesntSupportByteRanges(t *testing.T) {
@@ -182,29 +159,23 @@ func TestResumableRequestReaderWithServerDoesntSupportByteRanges(t *testing.T) {
 
 	var req *http.Request
 	req, err := http.NewRequest("GET", ts.URL, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	client := &http.Client{}
 
-	resreq := &resumableRequestReader{
+	resreq := &requestReader{
 		client:    client,
 		request:   req,
 		lastRange: 1,
 	}
 	defer resreq.Close()
 
-	expectedError := "the server doesn't support byte ranges"
 	buf := make([]byte, 2)
 	_, err = resreq.Read(buf)
-	if err == nil || err.Error() != expectedError {
-		t.Fatalf("Expected an error '%s', got %v", expectedError, err)
-	}
+	assert.EqualError(t, err, "the server doesn't support byte ranges")
 }
 
 func TestResumableRequestReaderWithZeroTotalSize(t *testing.T) {
-
 	srvtxt := "some response text data"
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -214,30 +185,22 @@ func TestResumableRequestReaderWithZeroTotalSize(t *testing.T) {
 
 	var req *http.Request
 	req, err := http.NewRequest("GET", ts.URL, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	client := &http.Client{}
 	retries := uint32(5)
 
-	resreq := ResumableRequestReader(client, req, retries, 0)
+	resreq := NewRequestReader(client, req, retries, 0)
 	defer resreq.Close()
 
 	data, err := ioutil.ReadAll(resreq)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	resstr := strings.TrimSuffix(string(data), "\n")
-
-	if resstr != srvtxt {
-		t.Error("resstr != srvtxt")
-	}
+	assert.Equal(t, srvtxt, resstr)
 }
 
 func TestResumableRequestReader(t *testing.T) {
-
 	srvtxt := "some response text data"
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -247,31 +210,23 @@ func TestResumableRequestReader(t *testing.T) {
 
 	var req *http.Request
 	req, err := http.NewRequest("GET", ts.URL, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	client := &http.Client{}
 	retries := uint32(5)
 	imgSize := int64(len(srvtxt))
 
-	resreq := ResumableRequestReader(client, req, retries, imgSize)
+	resreq := NewRequestReader(client, req, retries, imgSize)
 	defer resreq.Close()
 
 	data, err := ioutil.ReadAll(resreq)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	resstr := strings.TrimSuffix(string(data), "\n")
-
-	if resstr != srvtxt {
-		t.Error("resstr != srvtxt")
-	}
+	assert.Equal(t, srvtxt, resstr)
 }
 
 func TestResumableRequestReaderWithInitialResponse(t *testing.T) {
-
 	srvtxt := "some response text data"
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -281,30 +236,21 @@ func TestResumableRequestReaderWithInitialResponse(t *testing.T) {
 
 	var req *http.Request
 	req, err := http.NewRequest("GET", ts.URL, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	client := &http.Client{}
 	retries := uint32(5)
 	imgSize := int64(len(srvtxt))
 
 	res, err := client.Do(req)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
-	resreq := ResumableRequestReaderWithInitialResponse(client, req, retries, imgSize, res)
+	resreq := NewRequestReaderWithInitialResponse(client, req, retries, imgSize, res)
 	defer resreq.Close()
 
 	data, err := ioutil.ReadAll(resreq)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 
 	resstr := strings.TrimSuffix(string(data), "\n")
-
-	if resstr != srvtxt {
-		t.Error("resstr != srvtxt")
-	}
+	assert.Equal(t, srvtxt, resstr)
 }

registry/session.go

@@ -23,10 +23,11 @@ import (
 	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/docker/api/types"
 	registrytypes "github.com/docker/docker/api/types/registry"
-	"github.com/docker/docker/pkg/httputils"
 	"github.com/docker/docker/pkg/ioutils"
+	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/tarsum"
+	"github.com/docker/docker/registry/resumable"
 )
 
 var (
@@ -226,7 +227,7 @@ func (r *Session) GetRemoteHistory(imgID, registry string) ([]string, error) {
 		if res.StatusCode == 401 {
 			return nil, errcode.ErrorCodeUnauthorized.WithArgs()
 		}
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Server error: %d trying to fetch remote history for %s", res.StatusCode, imgID), res)
+		return nil, newJSONError(fmt.Sprintf("Server error: %d trying to fetch remote history for %s", res.StatusCode, imgID), res)
 	}
 
 	var history []string
@@ -246,7 +247,7 @@ func (r *Session) LookupRemoteImage(imgID, registry string) error {
 	}
 	res.Body.Close()
 	if res.StatusCode != 200 {
-		return httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
+		return newJSONError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
 	}
 	return nil
 }
@@ -259,7 +260,7 @@ func (r *Session) GetRemoteImageJSON(imgID, registry string) ([]byte, int64, err
 	}
 	defer res.Body.Close()
 	if res.StatusCode != 200 {
-		return nil, -1, httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
+		return nil, -1, newJSONError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
 	}
 	// if the size header is not present, then set it to '-1'
 	imageSize := int64(-1)
@@ -313,7 +314,7 @@ func (r *Session) GetRemoteImageLayer(imgID, registry string, imgSize int64) (io
 
 	if res.Header.Get("Accept-Ranges") == "bytes" && imgSize > 0 {
 		logrus.Debug("server supports resume")
-		return httputils.ResumableRequestReaderWithInitialResponse(r.client, req, 5, imgSize, res), nil
+		return resumable.NewRequestReaderWithInitialResponse(r.client, req, 5, imgSize, res), nil
 	}
 	logrus.Debug("server doesn't support resume")
 	return res.Body, nil
@@ -444,13 +445,13 @@ func (r *Session) GetRepositoryData(name reference.Named) (*RepositoryData, erro
 	// TODO: Right now we're ignoring checksums in the response body.
 	// In the future, we need to use them to check image validity.
 	if res.StatusCode == 404 {
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code: %d", res.StatusCode), res)
+		return nil, newJSONError(fmt.Sprintf("HTTP code: %d", res.StatusCode), res)
 	} else if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
 			logrus.Debugf("Error reading response body: %s", err)
 		}
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to pull repository %s: %q", res.StatusCode, reference.Path(name), errBody), res)
+		return nil, newJSONError(fmt.Sprintf("Error: Status %d trying to pull repository %s: %q", res.StatusCode, reference.Path(name), errBody), res)
 	}
 
 	var endpoints []string
@@ -537,12 +538,12 @@ func (r *Session) PushImageJSONRegistry(imgData *ImgData, jsonRaw []byte, regist
 	}
 	defer res.Body.Close()
 	if res.StatusCode == 401 && strings.HasPrefix(registry, "http://") {
-		return httputils.NewHTTPRequestError("HTTP code 401, Docker will not send auth headers over HTTP.", res)
+		return newJSONError("HTTP code 401, Docker will not send auth headers over HTTP.", res)
 	}
 	if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
+			return newJSONError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
 		}
 		var jsonBody map[string]string
 		if err := json.Unmarshal(errBody, &jsonBody); err != nil {
@@ -550,7 +551,7 @@ func (r *Session) PushImageJSONRegistry(imgData *ImgData, jsonRaw []byte, regist
 		} else if jsonBody["error"] == "Image already exists" {
 			return ErrAlreadyExists
 		}
-		return httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata: %q", res.StatusCode, errBody), res)
+		return newJSONError(fmt.Sprintf("HTTP code %d while uploading metadata: %q", res.StatusCode, errBody), res)
 	}
 	return nil
 }
@@ -591,9 +592,9 @@ func (r *Session) PushImageLayerRegistry(imgID string, layer io.Reader, registry
 	if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return "", "", httputils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
+			return "", "", newJSONError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
 		}
-		return "", "", httputils.NewHTTPRequestError(fmt.Sprintf("Received HTTP code %d while uploading layer: %q", res.StatusCode, errBody), res)
+		return "", "", newJSONError(fmt.Sprintf("Received HTTP code %d while uploading layer: %q", res.StatusCode, errBody), res)
 	}
 
 	checksumPayload = "sha256:" + hex.EncodeToString(h.Sum(nil))
@@ -619,7 +620,7 @@ func (r *Session) PushRegistryTag(remote reference.Named, revision, tag, registr
 	}
 	res.Body.Close()
 	if res.StatusCode != 200 && res.StatusCode != 201 {
-		return httputils.NewHTTPRequestError(fmt.Sprintf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, reference.Path(remote)), res)
+		return newJSONError(fmt.Sprintf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, reference.Path(remote)), res)
 	}
 	return nil
 }
@@ -683,7 +684,7 @@ func (r *Session) PushImageJSONIndex(remote reference.Named, imgList []*ImgData,
 			if err != nil {
 				logrus.Debugf("Error reading response body: %s", err)
 			}
-			return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to push repository %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
+			return nil, newJSONError(fmt.Sprintf("Error: Status %d trying to push repository %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
 		}
 		tokens = res.Header["X-Docker-Token"]
 		logrus.Debugf("Auth token: %v", tokens)
@@ -701,7 +702,7 @@ func (r *Session) PushImageJSONIndex(remote reference.Named, imgList []*ImgData,
 			if err != nil {
 				logrus.Debugf("Error reading response body: %s", err)
 			}
-			return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to push checksums %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
+			return nil, newJSONError(fmt.Sprintf("Error: Status %d trying to push checksums %s: %q", res.StatusCode, reference.Path(remote), errBody), res)
 		}
 	}
 
@@ -750,7 +751,7 @@ func (r *Session) SearchRepositories(term string, limit int) (*registrytypes.Sea
 	}
 	defer res.Body.Close()
 	if res.StatusCode != 200 {
-		return nil, httputils.NewHTTPRequestError(fmt.Sprintf("Unexpected status code %d", res.StatusCode), res)
+		return nil, newJSONError(fmt.Sprintf("Unexpected status code %d", res.StatusCode), res)
 	}
 	result := new(registrytypes.SearchResults)
 	return result, json.NewDecoder(res.Body).Decode(result)
@@ -781,3 +782,10 @@ func isTimeout(err error) bool {
 	t, ok := e.(timeout)
 	return ok && t.Timeout()
 }
+
+func newJSONError(msg string, res *http.Response) error {
+	return &jsonmessage.JSONError{
+		Message: msg,
+		Code:    res.StatusCode,
+	}
+}