Signed-off-by: Daniel Nephin <dnephin@docker.com>
Daniel Nephin authored on 2017/06/02 06:05:44... | ... |
@@ -13,7 +13,6 @@ import ( |
13 | 13 |
|
14 | 14 |
"github.com/docker/docker/builder" |
15 | 15 |
"github.com/docker/docker/builder/remotecontext" |
16 |
- "github.com/docker/docker/pkg/httputils" |
|
17 | 16 |
"github.com/docker/docker/pkg/ioutils" |
18 | 17 |
"github.com/docker/docker/pkg/progress" |
19 | 18 |
"github.com/docker/docker/pkg/streamformatter" |
... | ... |
@@ -292,7 +291,6 @@ func errOnSourceDownload(_ string) (builder.Source, string, error) { |
292 | 292 |
} |
293 | 293 |
|
294 | 294 |
func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote builder.Source, p string, err error) { |
295 |
- // get filename from URL |
|
296 | 295 |
u, err := url.Parse(srcURL) |
297 | 296 |
if err != nil { |
298 | 297 |
return |
... | ... |
@@ -303,8 +301,7 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b |
303 | 303 |
return |
304 | 304 |
} |
305 | 305 |
|
306 |
- // Initiate the download |
|
307 |
- resp, err := httputils.Download(srcURL) |
|
306 |
+ resp, err := remotecontext.GetWithStatusError(srcURL) |
|
308 | 307 |
if err != nil { |
309 | 308 |
return |
310 | 309 |
} |
... | ... |
@@ -2,14 +2,14 @@ package remotecontext |
2 | 2 |
|
3 | 3 |
import ( |
4 | 4 |
"bytes" |
5 |
- "errors" |
|
6 | 5 |
"fmt" |
7 | 6 |
"io" |
8 | 7 |
"io/ioutil" |
8 |
+ "net/http" |
|
9 | 9 |
"regexp" |
10 | 10 |
|
11 | 11 |
"github.com/docker/docker/builder" |
12 |
- "github.com/docker/docker/pkg/httputils" |
|
12 |
+ "github.com/pkg/errors" |
|
13 | 13 |
) |
14 | 14 |
|
15 | 15 |
// When downloading remote contexts, limit the amount (in bytes) |
... | ... |
@@ -30,7 +30,7 @@ var mimeRe = regexp.MustCompile(acceptableRemoteMIME) |
30 | 30 |
// to be returned. If no match is found, it is assumed the body is a tar stream (compressed or not). |
31 | 31 |
// In either case, an (assumed) tar stream is passed to MakeTarSumContext whose result is returned. |
32 | 32 |
func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io.ReadCloser) (io.ReadCloser, error)) (builder.Source, error) { |
33 |
- f, err := httputils.Download(remoteURL) |
|
33 |
+ f, err := GetWithStatusError(remoteURL) |
|
34 | 34 |
if err != nil { |
35 | 35 |
return nil, fmt.Errorf("error downloading remote context %s: %v", remoteURL, err) |
36 | 36 |
} |
... | ... |
@@ -66,6 +66,24 @@ func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io. |
66 | 66 |
return MakeTarSumContext(contextReader) |
67 | 67 |
} |
68 | 68 |
|
69 |
+// GetWithStatusError does an http.Get() and returns an error if the |
|
70 |
+// status code is 4xx or 5xx. |
|
71 |
+func GetWithStatusError(url string) (resp *http.Response, err error) { |
|
72 |
+ if resp, err = http.Get(url); err != nil { |
|
73 |
+ return nil, err |
|
74 |
+ } |
|
75 |
+ if resp.StatusCode < 400 { |
|
76 |
+ return resp, nil |
|
77 |
+ } |
|
78 |
+ msg := fmt.Sprintf("failed to GET %s with status %s", url, resp.Status) |
|
79 |
+ body, err := ioutil.ReadAll(resp.Body) |
|
80 |
+ resp.Body.Close() |
|
81 |
+ if err != nil { |
|
82 |
+ return nil, errors.Wrapf(err, msg+": error reading body") |
|
83 |
+ } |
|
84 |
+ return nil, errors.Errorf(msg+": %s", bytes.TrimSpace(body)) |
|
85 |
+} |
|
86 |
+ |
|
69 | 87 |
// inspectResponse looks into the http response data at r to determine whether its |
70 | 88 |
// content-type is on the list of acceptable content types for remote build contexts. |
71 | 89 |
// This function returns: |
... | ... |
@@ -11,6 +11,9 @@ import ( |
11 | 11 |
|
12 | 12 |
"github.com/docker/docker/builder" |
13 | 13 |
"github.com/docker/docker/pkg/archive" |
14 |
+ "github.com/docker/docker/pkg/testutil" |
|
15 |
+ "github.com/stretchr/testify/assert" |
|
16 |
+ "github.com/stretchr/testify/require" |
|
14 | 17 |
) |
15 | 18 |
|
16 | 19 |
var binaryContext = []byte{0xFD, 0x37, 0x7A, 0x58, 0x5A, 0x00} //xz magic |
... | ... |
@@ -231,3 +234,43 @@ func TestMakeRemoteContext(t *testing.T) { |
231 | 231 |
t.Fatalf("File %s should have position 0, got %d", builder.DefaultDockerfileName, fileInfo.Pos()) |
232 | 232 |
} |
233 | 233 |
} |
234 |
+ |
|
235 |
+func TestGetWithStatusError(t *testing.T) { |
|
236 |
+ var testcases = []struct { |
|
237 |
+ err error |
|
238 |
+ statusCode int |
|
239 |
+ expectedErr string |
|
240 |
+ expectedBody string |
|
241 |
+ }{ |
|
242 |
+ { |
|
243 |
+ statusCode: 200, |
|
244 |
+ expectedBody: "THE BODY", |
|
245 |
+ }, |
|
246 |
+ { |
|
247 |
+ statusCode: 400, |
|
248 |
+ expectedErr: "with status 400 Bad Request: broke", |
|
249 |
+ expectedBody: "broke", |
|
250 |
+ }, |
|
251 |
+ } |
|
252 |
+ for _, testcase := range testcases { |
|
253 |
+ ts := httptest.NewServer( |
|
254 |
+ http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
|
255 |
+ buffer := bytes.NewBufferString(testcase.expectedBody) |
|
256 |
+ w.WriteHeader(testcase.statusCode) |
|
257 |
+ w.Write(buffer.Bytes()) |
|
258 |
+ }), |
|
259 |
+ ) |
|
260 |
+ defer ts.Close() |
|
261 |
+ response, err := GetWithStatusError(ts.URL) |
|
262 |
+ |
|
263 |
+ if testcase.expectedErr == "" { |
|
264 |
+ require.NoError(t, err) |
|
265 |
+ |
|
266 |
+ body, err := testutil.ReadBody(response.Body) |
|
267 |
+ require.NoError(t, err) |
|
268 |
+ assert.Contains(t, string(body), testcase.expectedBody) |
|
269 |
+ } else { |
|
270 |
+ testutil.ErrorContains(t, err, testcase.expectedErr) |
|
271 |
+ } |
|
272 |
+ } |
|
273 |
+} |
... | ... |
@@ -12,11 +12,11 @@ import ( |
12 | 12 |
"github.com/docker/distribution/reference" |
13 | 13 |
"github.com/docker/docker/api/types/container" |
14 | 14 |
"github.com/docker/docker/builder/dockerfile" |
15 |
+ "github.com/docker/docker/builder/remotecontext" |
|
15 | 16 |
"github.com/docker/docker/dockerversion" |
16 | 17 |
"github.com/docker/docker/image" |
17 | 18 |
"github.com/docker/docker/layer" |
18 | 19 |
"github.com/docker/docker/pkg/archive" |
19 |
- "github.com/docker/docker/pkg/httputils" |
|
20 | 20 |
"github.com/docker/docker/pkg/progress" |
21 | 21 |
"github.com/docker/docker/pkg/streamformatter" |
22 | 22 |
"github.com/pkg/errors" |
... | ... |
@@ -67,7 +67,7 @@ func (daemon *Daemon) ImportImage(src string, repository, tag string, msg string |
67 | 67 |
return err |
68 | 68 |
} |
69 | 69 |
|
70 |
- resp, err = httputils.Download(u.String()) |
|
70 |
+ resp, err = remotecontext.GetWithStatusError(u.String()) |
|
71 | 71 |
if err != nil { |
72 | 72 |
return err |
73 | 73 |
} |
74 | 74 |
deleted file mode 100644 |
... | ... |
@@ -1,17 +0,0 @@ |
1 |
-package httputils |
|
2 |
- |
|
3 |
-import ( |
|
4 |
- "fmt" |
|
5 |
- "net/http" |
|
6 |
-) |
|
7 |
- |
|
8 |
-// Download requests a given URL and returns an io.Reader. |
|
9 |
-func Download(url string) (resp *http.Response, err error) { |
|
10 |
- if resp, err = http.Get(url); err != nil { |
|
11 |
- return nil, err |
|
12 |
- } |
|
13 |
- if resp.StatusCode >= 400 { |
|
14 |
- return nil, fmt.Errorf("Got HTTP status code >= 400: %s", resp.Status) |
|
15 |
- } |
|
16 |
- return resp, nil |
|
17 |
-} |
18 | 1 |
deleted file mode 100644 |
... | ... |
@@ -1,47 +0,0 @@ |
1 |
-package httputils |
|
2 |
- |
|
3 |
-import ( |
|
4 |
- "fmt" |
|
5 |
- "io/ioutil" |
|
6 |
- "net/http" |
|
7 |
- "net/http/httptest" |
|
8 |
- "testing" |
|
9 |
- |
|
10 |
- "github.com/docker/docker/pkg/testutil" |
|
11 |
- "github.com/stretchr/testify/assert" |
|
12 |
- "github.com/stretchr/testify/require" |
|
13 |
-) |
|
14 |
- |
|
15 |
-func TestDownload(t *testing.T) { |
|
16 |
- expected := "Hello, docker !" |
|
17 |
- ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
|
18 |
- fmt.Fprintf(w, expected) |
|
19 |
- })) |
|
20 |
- defer ts.Close() |
|
21 |
- response, err := Download(ts.URL) |
|
22 |
- if err != nil { |
|
23 |
- t.Fatal(err) |
|
24 |
- } |
|
25 |
- |
|
26 |
- actual, err := ioutil.ReadAll(response.Body) |
|
27 |
- response.Body.Close() |
|
28 |
- require.NoError(t, err) |
|
29 |
- assert.Equal(t, expected, string(actual)) |
|
30 |
-} |
|
31 |
- |
|
32 |
-func TestDownload400Errors(t *testing.T) { |
|
33 |
- expectedError := "Got HTTP status code >= 400: 403 Forbidden" |
|
34 |
- ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
|
35 |
- // 403 |
|
36 |
- http.Error(w, "something failed (forbidden)", http.StatusForbidden) |
|
37 |
- })) |
|
38 |
- defer ts.Close() |
|
39 |
- // Expected status code = 403 |
|
40 |
- _, err := Download(ts.URL) |
|
41 |
- assert.EqualError(t, err, expectedError) |
|
42 |
-} |
|
43 |
- |
|
44 |
-func TestDownloadOtherErrors(t *testing.T) { |
|
45 |
- _, err := Download("I'm not an url..") |
|
46 |
- testutil.ErrorContains(t, err, "unsupported protocol scheme") |
|
47 |
-} |