diff --git a/builder/remotecontext/archive.go b/builder/remotecontext/archive.go index b62d9dd0b7..c670235b61 100644 --- a/builder/remotecontext/archive.go +++ b/builder/remotecontext/archive.go @@ -79,7 +79,6 @@ func FromArchive(tarStream io.Reader) (builder.Source, error) { } tsc.sums = sum.GetSums() - return tsc, nil } diff --git a/builder/remotecontext/detect.go b/builder/remotecontext/detect.go index 38aff67985..fe5cd967fb 100644 --- a/builder/remotecontext/detect.go +++ b/builder/remotecontext/detect.go @@ -97,26 +97,23 @@ func newGitRemote(gitURL string, dockerfilePath string) (builder.Source, *parser } func newURLRemote(url string, dockerfilePath string, progressReader func(in io.ReadCloser) io.ReadCloser) (builder.Source, *parser.Result, error) { - var dockerfile io.ReadCloser - dockerfileFoundErr := errors.New("found-dockerfile") - c, err := MakeRemoteContext(url, map[string]func(io.ReadCloser) (io.ReadCloser, error){ - mimeTypes.TextPlain: func(rc io.ReadCloser) (io.ReadCloser, error) { - dockerfile = rc - return nil, dockerfileFoundErr - }, - // fallback handler (tar context) - "": func(rc io.ReadCloser) (io.ReadCloser, error) { - return progressReader(rc), nil - }, - }) - switch { - case err == dockerfileFoundErr: - res, err := parser.Parse(dockerfile) - return nil, res, err - case err != nil: + contentType, content, err := downloadRemote(url) + if err != nil { return nil, nil, err } - return withDockerfileFromContext(c.(modifiableContext), dockerfilePath) + defer content.Close() + + switch contentType { + case mimeTypes.TextPlain: + res, err := parser.Parse(progressReader(content)) + return nil, res, err + default: + source, err := FromArchive(progressReader(content)) + if err != nil { + return nil, nil, err + } + return withDockerfileFromContext(source.(modifiableContext), dockerfilePath) + } } func removeDockerfile(c modifiableContext, filesToRemove ...string) error { diff --git a/builder/remotecontext/remote.go b/builder/remotecontext/remote.go index 6c4073bd4c..ee0282f706 100644 --- a/builder/remotecontext/remote.go +++ b/builder/remotecontext/remote.go @@ -10,7 +10,7 @@ import ( "net/url" "regexp" - "github.com/docker/docker/builder" + "github.com/docker/docker/pkg/ioutils" "github.com/pkg/errors" ) @@ -22,50 +22,23 @@ const acceptableRemoteMIME = `(?:application/(?:(?:x\-)?tar|octet\-stream|((?:x\ var mimeRe = regexp.MustCompile(acceptableRemoteMIME) -// MakeRemoteContext downloads a context from remoteURL and returns it. -// -// If contentTypeHandlers is non-nil, then the Content-Type header is read along with a maximum of -// maxPreambleLength bytes from the body to help detecting the MIME type. -// Look at acceptableRemoteMIME for more details. -// -// If a match is found, then the body is sent to the contentType handler and a (potentially compressed) tar stream is expected -// 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 FromArchive whose result is returned. -func MakeRemoteContext(remoteURL string, contentTypeHandlers map[string]func(io.ReadCloser) (io.ReadCloser, error)) (builder.Source, error) { - f, err := GetWithStatusError(remoteURL) +// downloadRemote context from a url and returns it, along with the parsed content type +func downloadRemote(remoteURL string) (string, io.ReadCloser, error) { + response, err := GetWithStatusError(remoteURL) if err != nil { - return nil, fmt.Errorf("error downloading remote context %s: %v", remoteURL, err) - } - defer f.Body.Close() - - var contextReader io.ReadCloser - if contentTypeHandlers != nil { - contentType := f.Header.Get("Content-Type") - clen := f.ContentLength - - contentType, contextReader, err = inspectResponse(contentType, f.Body, clen) - if err != nil { - return nil, fmt.Errorf("error detecting content type for remote %s: %v", remoteURL, err) - } - defer contextReader.Close() - - // This loop tries to find a content-type handler for the detected content-type. - // If it could not find one from the caller-supplied map, it tries the empty content-type `""` - // which is interpreted as a fallback handler (usually used for raw tar contexts). - for _, ct := range []string{contentType, ""} { - if fn, ok := contentTypeHandlers[ct]; ok { - defer contextReader.Close() - if contextReader, err = fn(contextReader); err != nil { - return nil, err - } - break - } - } + return "", nil, fmt.Errorf("error downloading remote context %s: %v", remoteURL, err) } - // Pass through - this is a pre-packaged context, presumably - // with a Dockerfile with the right name inside it. - return FromArchive(contextReader) + contentType, contextReader, err := inspectResponse( + response.Header.Get("Content-Type"), + response.Body, + response.ContentLength) + if err != nil { + response.Body.Close() + return "", nil, fmt.Errorf("error detecting content type for remote %s: %v", remoteURL, err) + } + + return contentType, ioutils.NewReadCloserWrapper(contextReader, response.Body.Close), nil } // GetWithStatusError does an http.Get() and returns an error if the @@ -110,7 +83,7 @@ func GetWithStatusError(address string) (resp *http.Response, err error) { // - an io.Reader for the response body // - an error value which will be non-nil either when something goes wrong while // reading bytes from r or when the detected content-type is not acceptable. -func inspectResponse(ct string, r io.Reader, clen int64) (string, io.ReadCloser, error) { +func inspectResponse(ct string, r io.Reader, clen int64) (string, io.Reader, error) { plen := clen if plen <= 0 || plen > maxPreambleLength { plen = maxPreambleLength @@ -119,14 +92,14 @@ func inspectResponse(ct string, r io.Reader, clen int64) (string, io.ReadCloser, preamble := make([]byte, plen) rlen, err := r.Read(preamble) if rlen == 0 { - return ct, ioutil.NopCloser(r), errors.New("empty response") + return ct, r, errors.New("empty response") } if err != nil && err != io.EOF { - return ct, ioutil.NopCloser(r), err + return ct, r, err } preambleR := bytes.NewReader(preamble[:rlen]) - bodyReader := ioutil.NopCloser(io.MultiReader(preambleR, r)) + bodyReader := io.MultiReader(preambleR, r) // Some web servers will use application/octet-stream as the default // content type for files without an extension (e.g. 'Dockerfile') // so if we receive this value we better check for text content diff --git a/builder/remotecontext/remote_test.go b/builder/remotecontext/remote_test.go index c38433b340..35b105f550 100644 --- a/builder/remotecontext/remote_test.go +++ b/builder/remotecontext/remote_test.go @@ -11,7 +11,7 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/internal/testutil" - "github.com/docker/docker/pkg/archive" + "github.com/gotestyourself/gotestyourself/fs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -174,11 +174,10 @@ func TestUnknownContentLength(t *testing.T) { } } -func TestMakeRemoteContext(t *testing.T) { - contextDir, cleanup := createTestTempDir(t, "", "builder-tarsum-test") - defer cleanup() - - createTestTempFile(t, contextDir, builder.DefaultDockerfileName, dockerfileContents, 0777) +func TestDownloadRemote(t *testing.T) { + contextDir := fs.NewDir(t, "test-builder-download-remote", + fs.WithFile(builder.DefaultDockerfileName, dockerfileContents)) + defer contextDir.Remove() mux := http.NewServeMux() server := httptest.NewServer(mux) @@ -187,39 +186,15 @@ func TestMakeRemoteContext(t *testing.T) { serverURL.Path = "/" + builder.DefaultDockerfileName remoteURL := serverURL.String() - mux.Handle("/", http.FileServer(http.Dir(contextDir))) + mux.Handle("/", http.FileServer(http.Dir(contextDir.Path()))) - remoteContext, err := MakeRemoteContext(remoteURL, map[string]func(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 - } + contentType, content, err := downloadRemote(remoteURL) + require.NoError(t, err) - r, err := archive.Generate(builder.DefaultDockerfileName, string(dockerfile)) - if err != nil { - return nil, err - } - return ioutil.NopCloser(r), nil - }, - }) - - if err != nil { - t.Fatalf("Error when executing DetectContextFromRemoteURL: %s", err) - } - - if remoteContext == nil { - t.Fatal("Remote context should not be nil") - } - - h, err := remoteContext.Hash(builder.DefaultDockerfileName) - if err != nil { - t.Fatalf("failed to compute hash %s", err) - } - - if expected, actual := "7b6b6b66bee9e2102fbdc2228be6c980a2a23adf371962a37286a49f7de0f7cc", h; expected != actual { - t.Fatalf("There should be file named %s %s in fileInfoSums", expected, actual) - } + assert.Equal(t, mimeTypes.TextPlain, contentType) + raw, err := ioutil.ReadAll(content) + require.NoError(t, err) + assert.Equal(t, dockerfileContents, string(raw)) } func TestGetWithStatusError(t *testing.T) {