From 4060d6ee0b130cf74294c309dfbd3c860fd2a7f8 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 1 Jun 2017 17:05:44 -0400 Subject: [PATCH] Remove the last of pkg/httputil Signed-off-by: Daniel Nephin --- builder/dockerfile/copy.go | 5 +-- builder/remotecontext/remote.go | 24 ++++++++++++-- builder/remotecontext/remote_test.go | 43 +++++++++++++++++++++++++ daemon/import.go | 4 +-- pkg/httputils/httputils.go | 17 ---------- pkg/httputils/httputils_test.go | 47 ---------------------------- 6 files changed, 67 insertions(+), 73 deletions(-) delete mode 100644 pkg/httputils/httputils.go delete mode 100644 pkg/httputils/httputils_test.go diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index 77f0a44d55..db98eb92e1 100644 --- a/builder/dockerfile/copy.go +++ b/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/builder/remotecontext/remote.go b/builder/remotecontext/remote.go index 5a8ba0f35d..bf45fca064 100644 --- a/builder/remotecontext/remote.go +++ b/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/builder/remotecontext/remote_test.go b/builder/remotecontext/remote_test.go index b0b17a02b6..37c2740674 100644 --- a/builder/remotecontext/remote_test.go +++ b/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/daemon/import.go b/daemon/import.go index 17b9d870a3..0687533a0b 100644 --- a/daemon/import.go +++ b/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/pkg/httputils/httputils.go b/pkg/httputils/httputils.go deleted file mode 100644 index fe84d34d9b..0000000000 --- a/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/pkg/httputils/httputils_test.go b/pkg/httputils/httputils_test.go deleted file mode 100644 index 6c62ba7838..0000000000 --- a/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") -}