From 04eec7106a21ae3d2dc13acadf15f5511a43b8b3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 15:46:27 -0400 Subject: [PATCH 1/5] Move httputils/mimtype to the single consumser, and remove unused function. Signed-off-by: Daniel Nephin Upstream-commit: c91521be68182b450a22f018ccc9d780571a5936 Component: engine --- .../engine/builder/remotecontext/detect.go | 3 +- .../remotecontext}/mimetype.go | 12 ++--- .../builder/remotecontext/mimetype_test.go | 16 ++++++ .../engine/builder/remotecontext/remote.go | 4 +- .../builder/remotecontext/remote_test.go | 3 +- components/engine/pkg/httputils/httputils.go | 29 ----------- .../engine/pkg/httputils/httputils_test.go | 51 ------------------- .../engine/pkg/httputils/mimetype_test.go | 13 ----- 8 files changed, 25 insertions(+), 106 deletions(-) rename components/engine/{pkg/httputils => builder/remotecontext}/mimetype.go (73%) create mode 100644 components/engine/builder/remotecontext/mimetype_test.go delete mode 100644 components/engine/pkg/httputils/mimetype_test.go diff --git a/components/engine/builder/remotecontext/detect.go b/components/engine/builder/remotecontext/detect.go index 448782dcca..efac27f7ff 100644 --- a/components/engine/builder/remotecontext/detect.go +++ b/components/engine/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 }, diff --git a/components/engine/pkg/httputils/mimetype.go b/components/engine/builder/remotecontext/mimetype.go similarity index 73% rename from components/engine/pkg/httputils/mimetype.go rename to components/engine/builder/remotecontext/mimetype.go index abef9e9e83..083d609978 100644 --- a/components/engine/pkg/httputils/mimetype.go +++ b/components/engine/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 } diff --git a/components/engine/builder/remotecontext/mimetype_test.go b/components/engine/builder/remotecontext/mimetype_test.go new file mode 100644 index 0000000000..8c00ec2860 --- /dev/null +++ b/components/engine/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) +} diff --git a/components/engine/builder/remotecontext/remote.go b/components/engine/builder/remotecontext/remote.go index 6be464fff1..5a8ba0f35d 100644 --- a/components/engine/builder/remotecontext/remote.go +++ b/components/engine/builder/remotecontext/remote.go @@ -94,8 +94,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 } diff --git a/components/engine/builder/remotecontext/remote_test.go b/components/engine/builder/remotecontext/remote_test.go index af9db4b927..b0b17a02b6 100644 --- a/components/engine/builder/remotecontext/remote_test.go +++ b/components/engine/builder/remotecontext/remote_test.go @@ -11,7 +11,6 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/pkg/archive" - "github.com/docker/docker/pkg/httputils" ) var binaryContext = []byte{0xFD, 0x37, 0x7A, 0x58, 0x5A, 0x00} //xz magic @@ -188,7 +187,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 diff --git a/components/engine/pkg/httputils/httputils.go b/components/engine/pkg/httputils/httputils.go index af86835bd9..1e44ee4854 100644 --- a/components/engine/pkg/httputils/httputils.go +++ b/components/engine/pkg/httputils/httputils.go @@ -1,20 +1,12 @@ 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 { @@ -33,24 +25,3 @@ func NewHTTPRequestError(msg string, res *http.Response) error { 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 -} diff --git a/components/engine/pkg/httputils/httputils_test.go b/components/engine/pkg/httputils/httputils_test.go index 725c68f415..c439a91610 100644 --- a/components/engine/pkg/httputils/httputils_test.go +++ b/components/engine/pkg/httputils/httputils_test.go @@ -62,54 +62,3 @@ func TestNewHTTPRequestError(t *testing.T) { 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) - } - - } - -} diff --git a/components/engine/pkg/httputils/mimetype_test.go b/components/engine/pkg/httputils/mimetype_test.go deleted file mode 100644 index a6cc047db8..0000000000 --- a/components/engine/pkg/httputils/mimetype_test.go +++ /dev/null @@ -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") - } -} From 7636bfd2c7f6b1b4563738d68e0e909a1532557f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 15:59:24 -0400 Subject: [PATCH 2/5] Move httputils/reasumablerequestreader to the single consumer. Signed-off-by: Daniel Nephin Upstream-commit: 65515af075ce5f8037750c253b2834008840afc1 Component: engine --- .../resumable}/resumablerequestreader.go | 24 ++-- .../resumable}/resumablerequestreader_test.go | 132 ++++++------------ components/engine/registry/session.go | 3 +- 3 files changed, 53 insertions(+), 106 deletions(-) rename components/engine/{pkg/httputils => registry/resumable}/resumablerequestreader.go (66%) rename components/engine/{pkg/httputils => registry/resumable}/resumablerequestreader_test.go (69%) diff --git a/components/engine/pkg/httputils/resumablerequestreader.go b/components/engine/registry/resumable/resumablerequestreader.go similarity index 66% rename from components/engine/pkg/httputils/resumablerequestreader.go rename to components/engine/registry/resumable/resumablerequestreader.go index de488fb531..5403c76847 100644 --- a/components/engine/pkg/httputils/resumablerequestreader.go +++ b/components/engine/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 diff --git a/components/engine/pkg/httputils/resumablerequestreader_test.go b/components/engine/registry/resumable/resumablerequestreader_test.go similarity index 69% rename from components/engine/pkg/httputils/resumablerequestreader_test.go rename to components/engine/registry/resumable/resumablerequestreader_test.go index 81cafae579..a632bc673a 100644 --- a/components/engine/pkg/httputils/resumablerequestreader_test.go +++ b/components/engine/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) } diff --git a/components/engine/registry/session.go b/components/engine/registry/session.go index c71e778037..39bd80acc4 100644 --- a/components/engine/registry/session.go +++ b/components/engine/registry/session.go @@ -27,6 +27,7 @@ import ( "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/tarsum" + "github.com/docker/docker/registry/resumable" ) var ( @@ -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 From 16c0836e84fc43d41ebae60a9c6925fe3cf0a192 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 17:00:00 -0400 Subject: [PATCH 3/5] Move an error helper to registry/session. Signed-off-by: Daniel Nephin Upstream-commit: a6ac5495e11d34b1887476ca89f841df1056249a Component: engine --- components/engine/pkg/httputils/httputils.go | 10 ----- .../engine/pkg/httputils/httputils_test.go | 37 +++++-------------- components/engine/registry/session.go | 37 +++++++++++-------- 3 files changed, 32 insertions(+), 52 deletions(-) diff --git a/components/engine/pkg/httputils/httputils.go b/components/engine/pkg/httputils/httputils.go index 1e44ee4854..fe84d34d9b 100644 --- a/components/engine/pkg/httputils/httputils.go +++ b/components/engine/pkg/httputils/httputils.go @@ -3,8 +3,6 @@ package httputils import ( "fmt" "net/http" - - "github.com/docker/docker/pkg/jsonmessage" ) // Download requests a given URL and returns an io.Reader. @@ -17,11 +15,3 @@ func Download(url string) (resp *http.Response, err error) { } 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, - } -} diff --git a/components/engine/pkg/httputils/httputils_test.go b/components/engine/pkg/httputils/httputils_test.go index c439a91610..6c62ba7838 100644 --- a/components/engine/pkg/httputils/httputils_test.go +++ b/components/engine/pkg/httputils/httputils_test.go @@ -5,8 +5,11 @@ import ( "io/ioutil" "net/http" "net/http/httptest" - "strings" "testing" + + "github.com/docker/docker/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDownload(t *testing.T) { @@ -22,10 +25,8 @@ func TestDownload(t *testing.T) { 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)) - } + require.NoError(t, err) + assert.Equal(t, expected, string(actual)) } func TestDownload400Errors(t *testing.T) { @@ -36,29 +37,11 @@ func TestDownload400Errors(t *testing.T) { })) 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) - } + _, err := Download(ts.URL) + assert.EqualError(t, err, expectedError) } 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) - } + _, err := Download("I'm not an url..") + testutil.ErrorContains(t, err, "unsupported protocol scheme") } diff --git a/components/engine/registry/session.go b/components/engine/registry/session.go index 39bd80acc4..746b8fb5b5 100644 --- a/components/engine/registry/session.go +++ b/components/engine/registry/session.go @@ -23,8 +23,8 @@ 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" @@ -227,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 @@ -247,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 } @@ -260,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) @@ -445,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 @@ -538,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 { @@ -551,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 } @@ -592,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)) @@ -620,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 } @@ -684,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) @@ -702,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) } } @@ -751,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) @@ -782,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, + } +} From b9c4f53912c3999bd0067883ec36b7b41ee234ce Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 17:05:44 -0400 Subject: [PATCH 4/5] Remove the last of pkg/httputil Signed-off-by: Daniel Nephin Upstream-commit: 4060d6ee0b130cf74294c309dfbd3c860fd2a7f8 Component: engine --- components/engine/builder/dockerfile/copy.go | 5 +- .../engine/builder/remotecontext/remote.go | 24 ++++++++-- .../builder/remotecontext/remote_test.go | 43 +++++++++++++++++ components/engine/daemon/import.go | 4 +- components/engine/pkg/httputils/httputils.go | 17 ------- .../engine/pkg/httputils/httputils_test.go | 47 ------------------- 6 files changed, 67 insertions(+), 73 deletions(-) delete mode 100644 components/engine/pkg/httputils/httputils.go delete mode 100644 components/engine/pkg/httputils/httputils_test.go diff --git a/components/engine/builder/dockerfile/copy.go b/components/engine/builder/dockerfile/copy.go index 77f0a44d55..db98eb92e1 100644 --- a/components/engine/builder/dockerfile/copy.go +++ b/components/engine/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 } diff --git a/components/engine/builder/remotecontext/remote.go b/components/engine/builder/remotecontext/remote.go index 5a8ba0f35d..bf45fca064 100644 --- a/components/engine/builder/remotecontext/remote.go +++ b/components/engine/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: diff --git a/components/engine/builder/remotecontext/remote_test.go b/components/engine/builder/remotecontext/remote_test.go index b0b17a02b6..37c2740674 100644 --- a/components/engine/builder/remotecontext/remote_test.go +++ b/components/engine/builder/remotecontext/remote_test.go @@ -11,6 +11,9 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/pkg/archive" + "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 @@ -231,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) + } + } +} diff --git a/components/engine/daemon/import.go b/components/engine/daemon/import.go index 17b9d870a3..0687533a0b 100644 --- a/components/engine/daemon/import.go +++ b/components/engine/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 } diff --git a/components/engine/pkg/httputils/httputils.go b/components/engine/pkg/httputils/httputils.go deleted file mode 100644 index fe84d34d9b..0000000000 --- a/components/engine/pkg/httputils/httputils.go +++ /dev/null @@ -1,17 +0,0 @@ -package httputils - -import ( - "fmt" - "net/http" -) - -// 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 -} diff --git a/components/engine/pkg/httputils/httputils_test.go b/components/engine/pkg/httputils/httputils_test.go deleted file mode 100644 index 6c62ba7838..0000000000 --- a/components/engine/pkg/httputils/httputils_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package httputils - -import ( - "fmt" - "io/ioutil" - "net/http" - "net/http/httptest" - "testing" - - "github.com/docker/docker/pkg/testutil" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -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() - require.NoError(t, err) - assert.Equal(t, expected, 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 - _, err := Download(ts.URL) - assert.EqualError(t, err, expectedError) -} - -func TestDownloadOtherErrors(t *testing.T) { - _, err := Download("I'm not an url..") - testutil.ErrorContains(t, err, "unsupported protocol scheme") -} From b8e386fca43658dc8b5c4a1c13313e0301f7a6ba Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 17:15:13 -0400 Subject: [PATCH 5/5] Move pkg/gitutils to remotecontext/git Signed-off-by: Daniel Nephin Upstream-commit: b9d85ac58db8b8d4480fca93f8b952d084ea36f5 Component: engine --- .../engine/builder/remotecontext/git.go | 4 +- .../remotecontext/git}/gitutils.go | 2 +- .../remotecontext/git}/gitutils_test.go | 104 +++++++----------- 3 files changed, 40 insertions(+), 70 deletions(-) rename components/engine/{pkg/gitutils => builder/remotecontext/git}/gitutils.go (99%) rename components/engine/{pkg/gitutils => builder/remotecontext/git}/gitutils_test.go (65%) diff --git a/components/engine/builder/remotecontext/git.go b/components/engine/builder/remotecontext/git.go index 4204c9f7bb..7a55bfa9d4 100644 --- a/components/engine/builder/remotecontext/git.go +++ b/components/engine/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 } diff --git a/components/engine/pkg/gitutils/gitutils.go b/components/engine/builder/remotecontext/git/gitutils.go similarity index 99% rename from components/engine/pkg/gitutils/gitutils.go rename to components/engine/builder/remotecontext/git/gitutils.go index 4e09e05239..fd8250dbc3 100644 --- a/components/engine/pkg/gitutils/gitutils.go +++ b/components/engine/builder/remotecontext/git/gitutils.go @@ -1,4 +1,4 @@ -package gitutils +package git import ( "fmt" diff --git a/components/engine/pkg/gitutils/gitutils_test.go b/components/engine/builder/remotecontext/git/gitutils_test.go similarity index 65% rename from components/engine/pkg/gitutils/gitutils_test.go rename to components/engine/builder/remotecontext/git/gitutils_test.go index b5fbd18128..eb771f8c98 100644 --- a/components/engine/pkg/gitutils/gitutils_test.go +++ b/components/engine/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)) } }