diff --git a/cli/command/image/build.go b/cli/command/image/build.go index dce178a71b..6a51e5ca9c 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -6,11 +6,9 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "os" "regexp" "runtime" - "time" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -24,7 +22,6 @@ import ( "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" - "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/urlutil" runconfigopts "github.com/docker/docker/runconfig/opts" units "github.com/docker/go-units" @@ -237,7 +234,7 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { // replace Dockerfile if added dynamically if dockerfileCtx != nil { - buildCtx, relDockerfile, err = addDockerfileToBuildContext(dockerfileCtx, buildCtx) + buildCtx, relDockerfile, err = build.AddDockerfileToBuildContext(dockerfileCtx, buildCtx) if err != nil { return err } @@ -347,50 +344,6 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { return nil } -func addDockerfileToBuildContext(dockerfileCtx io.ReadCloser, buildCtx io.ReadCloser) (io.ReadCloser, string, error) { - file, err := ioutil.ReadAll(dockerfileCtx) - dockerfileCtx.Close() - if err != nil { - return nil, "", err - } - now := time.Now() - hdrTmpl := &tar.Header{ - Mode: 0600, - Uid: 0, - Gid: 0, - ModTime: now, - Typeflag: tar.TypeReg, - AccessTime: now, - ChangeTime: now, - } - randomName := ".dockerfile." + stringid.GenerateRandomID()[:20] - - buildCtx = archive.ReplaceFileTarWrapper(buildCtx, map[string]archive.TarModifierFunc{ - // Add the dockerfile with a random filename - randomName: func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { - return hdrTmpl, file, nil - }, - // Update .dockerignore to include the random filename - ".dockerignore": func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { - if h == nil { - h = hdrTmpl - } - - b := &bytes.Buffer{} - if content != nil { - if _, err := b.ReadFrom(content); err != nil { - return nil, nil, err - } - } else { - b.WriteString(".dockerignore") - } - b.WriteString("\n" + randomName + "\n") - return h, b.Bytes(), nil - }, - }) - return buildCtx, randomName, nil -} - func isLocalDir(c string) bool { _, err := os.Stat(c) return err == nil diff --git a/cli/command/image/build/context.go b/cli/command/image/build/context.go index 83a90c9f15..c49b5f7fce 100644 --- a/cli/command/image/build/context.go +++ b/cli/command/image/build/context.go @@ -11,6 +11,8 @@ import ( "runtime" "strings" + "archive/tar" + "bytes" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/gitutils" @@ -18,7 +20,9 @@ import ( "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" + "github.com/docker/docker/pkg/stringid" "github.com/pkg/errors" + "time" ) const ( @@ -293,3 +297,49 @@ func getDockerfileRelPath(absContextDir, givenDockerfile string) (string, error) func isUNC(path string) bool { return runtime.GOOS == "windows" && strings.HasPrefix(path, `\\`) } + +// AddDockerfileToBuildContext from a ReadCloser, returns a new archive and +// the relative path to the dockerfile in the context. +func AddDockerfileToBuildContext(dockerfileCtx io.ReadCloser, buildCtx io.ReadCloser) (io.ReadCloser, string, error) { + file, err := ioutil.ReadAll(dockerfileCtx) + dockerfileCtx.Close() + if err != nil { + return nil, "", err + } + now := time.Now() + hdrTmpl := &tar.Header{ + Mode: 0600, + Uid: 0, + Gid: 0, + ModTime: now, + Typeflag: tar.TypeReg, + AccessTime: now, + ChangeTime: now, + } + randomName := ".dockerfile." + stringid.GenerateRandomID()[:20] + + buildCtx = archive.ReplaceFileTarWrapper(buildCtx, map[string]archive.TarModifierFunc{ + // Add the dockerfile with a random filename + randomName: func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + return hdrTmpl, file, nil + }, + // Update .dockerignore to include the random filename + ".dockerignore": func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + if h == nil { + h = hdrTmpl + } + + b := &bytes.Buffer{} + if content != nil { + if _, err := b.ReadFrom(content); err != nil { + return nil, nil, err + } + } else { + b.WriteString(".dockerignore") + } + b.WriteString("\n" + randomName + "\n") + return h, b.Bytes(), nil + }, + }) + return buildCtx, randomName, nil +} diff --git a/cli/command/image/build/context_test.go b/cli/command/image/build/context_test.go index 3cab228a77..cda0bfb3f1 100644 --- a/cli/command/image/build/context_test.go +++ b/cli/command/image/build/context_test.go @@ -12,7 +12,9 @@ import ( "testing" "github.com/docker/docker/pkg/archive" - "github.com/docker/docker/pkg/testutil/assert" + "github.com/docker/docker/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const dockerfileContents = "FROM busybox" @@ -36,10 +38,7 @@ func testValidateContextDirectory(t *testing.T, prepare func(t *testing.T) (stri defer cleanup() err := ValidateContextDirectory(contextDir, excludes) - - if err != nil { - t.Fatalf("Error should be nil, got: %s", err) - } + require.NoError(t, err) } func TestGetContextFromLocalDirNoDockerfile(t *testing.T) { @@ -47,7 +46,7 @@ func TestGetContextFromLocalDirNoDockerfile(t *testing.T) { defer cleanup() _, _, err := GetContextFromLocalDir(contextDir, "") - assert.Error(t, err, "Dockerfile") + testutil.ErrorContains(t, err, "Dockerfile") } func TestGetContextFromLocalDirNotExistingDir(t *testing.T) { @@ -56,19 +55,8 @@ func TestGetContextFromLocalDirNotExistingDir(t *testing.T) { fakePath := filepath.Join(contextDir, "fake") - absContextDir, relDockerfile, err := GetContextFromLocalDir(fakePath, "") - - if err == nil { - t.Fatalf("Error should not be nil") - } - - if absContextDir != "" { - t.Fatalf("Absolute directory path should be empty, got: %s", absContextDir) - } - - if relDockerfile != "" { - t.Fatalf("Relative path to Dockerfile should be empty, got: %s", relDockerfile) - } + _, _, err := GetContextFromLocalDir(fakePath, "") + testutil.ErrorContains(t, err, "fake") } func TestGetContextFromLocalDirNotExistingDockerfile(t *testing.T) { @@ -78,7 +66,7 @@ func TestGetContextFromLocalDirNotExistingDockerfile(t *testing.T) { fakePath := filepath.Join(contextDir, "fake") _, _, err := GetContextFromLocalDir(contextDir, fakePath) - assert.Error(t, err, "fake") + testutil.ErrorContains(t, err, "fake") } func TestGetContextFromLocalDirWithNoDirectory(t *testing.T) { @@ -91,18 +79,10 @@ func TestGetContextFromLocalDirWithNoDirectory(t *testing.T) { defer chdirCleanup() absContextDir, relDockerfile, err := GetContextFromLocalDir(contextDir, "") + require.NoError(t, err) - if err != nil { - t.Fatalf("Error when getting context from local dir: %s", err) - } - - if absContextDir != contextDir { - t.Fatalf("Absolute directory path should be equal to %s, got: %s", contextDir, absContextDir) - } - - if relDockerfile != DefaultDockerfileName { - t.Fatalf("Relative path to dockerfile should be equal to %s, got: %s", DefaultDockerfileName, relDockerfile) - } + assert.Equal(t, contextDir, absContextDir) + assert.Equal(t, DefaultDockerfileName, relDockerfile) } func TestGetContextFromLocalDirWithDockerfile(t *testing.T) { @@ -112,18 +92,10 @@ func TestGetContextFromLocalDirWithDockerfile(t *testing.T) { createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777) absContextDir, relDockerfile, err := GetContextFromLocalDir(contextDir, "") + require.NoError(t, err) - if err != nil { - t.Fatalf("Error when getting context from local dir: %s", err) - } - - if absContextDir != contextDir { - t.Fatalf("Absolute directory path should be equal to %s, got: %s", contextDir, absContextDir) - } - - if relDockerfile != DefaultDockerfileName { - t.Fatalf("Relative path to dockerfile should be equal to %s, got: %s", DefaultDockerfileName, relDockerfile) - } + assert.Equal(t, contextDir, absContextDir) + assert.Equal(t, DefaultDockerfileName, relDockerfile) } func TestGetContextFromLocalDirLocalFile(t *testing.T) { @@ -158,19 +130,10 @@ func TestGetContextFromLocalDirWithCustomDockerfile(t *testing.T) { createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777) absContextDir, relDockerfile, err := GetContextFromLocalDir(contextDir, DefaultDockerfileName) + require.NoError(t, err) - if err != nil { - t.Fatalf("Error when getting context from local dir: %s", err) - } - - if absContextDir != contextDir { - t.Fatalf("Absolute directory path should be equal to %s, got: %s", contextDir, absContextDir) - } - - if relDockerfile != DefaultDockerfileName { - t.Fatalf("Relative path to dockerfile should be equal to %s, got: %s", DefaultDockerfileName, relDockerfile) - } - + assert.Equal(t, contextDir, absContextDir) + assert.Equal(t, DefaultDockerfileName, relDockerfile) } func TestGetContextFromReaderString(t *testing.T) { @@ -198,9 +161,7 @@ func TestGetContextFromReaderString(t *testing.T) { t.Fatalf("Tar stream too long: %s", err) } - if err = tarArchive.Close(); err != nil { - t.Fatalf("Error when closing tar stream: %s", err) - } + require.NoError(t, tarArchive.Close()) if dockerfileContents != contents { t.Fatalf("Uncompressed tar archive does not equal: %s, got: %s", dockerfileContents, contents) @@ -218,24 +179,15 @@ func TestGetContextFromReaderTar(t *testing.T) { createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents, 0777) tarStream, err := archive.Tar(contextDir, archive.Uncompressed) - - if err != nil { - t.Fatalf("Error when creating tar: %s", err) - } + require.NoError(t, err) tarArchive, relDockerfile, err := GetContextFromReader(tarStream, DefaultDockerfileName) - - if err != nil { - t.Fatalf("Error when executing GetContextFromReader: %s", err) - } + require.NoError(t, err) tarReader := tar.NewReader(tarArchive) header, err := tarReader.Next() - - if err != nil { - t.Fatalf("Error when reading tar archive: %s", err) - } + require.NoError(t, err) if header.Name != DefaultDockerfileName { t.Fatalf("Dockerfile name should be: %s, got: %s", DefaultDockerfileName, header.Name) @@ -251,9 +203,7 @@ func TestGetContextFromReaderTar(t *testing.T) { t.Fatalf("Tar stream too long: %s", err) } - if err = tarArchive.Close(); err != nil { - t.Fatalf("Error when closing tar stream: %s", err) - } + require.NoError(t, tarArchive.Close()) if dockerfileContents != contents { t.Fatalf("Uncompressed tar archive does not equal: %s, got: %s", dockerfileContents, contents) @@ -293,11 +243,8 @@ func TestValidateContextDirectoryWithOneFileExcludes(t *testing.T) { // When an error occurs, it terminates the test. func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) { path, err := ioutil.TempDir(dir, prefix) - assert.NilError(t, err) - - return path, func() { - assert.NilError(t, os.RemoveAll(path)) - } + require.NoError(t, err) + return path, func() { require.NoError(t, os.RemoveAll(path)) } } // createTestTempFile creates a temporary file within dir with specific contents and permissions. @@ -305,11 +252,7 @@ func createTestTempDir(t *testing.T, dir, prefix string) (string, func()) { func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.FileMode) string { filePath := filepath.Join(dir, filename) err := ioutil.WriteFile(filePath, []byte(contents), perm) - - if err != nil { - t.Fatalf("Error when creating %s file: %s", filename, err) - } - + require.NoError(t, err) return filePath } @@ -319,22 +262,7 @@ func createTestTempFile(t *testing.T, dir, filename, contents string, perm os.Fi // When an error occurs, it terminates the test. func chdir(t *testing.T, dir string) func() { workingDirectory, err := os.Getwd() - - if err != nil { - t.Fatalf("Error when retrieving working directory: %s", err) - } - - err = os.Chdir(dir) - - if err != nil { - t.Fatalf("Error when changing directory to %s: %s", dir, err) - } - - return func() { - err = os.Chdir(workingDirectory) - - if err != nil { - t.Fatalf("Error when changing back to working directory (%s): %s", workingDirectory, err) - } - } + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + return func() { require.NoError(t, os.Chdir(workingDirectory)) } }