From 06ea8bf159c78468171f4dbb85e436dd0d12ca3c Mon Sep 17 00:00:00 2001 From: David Sheets Date: Tue, 21 Feb 2017 12:07:45 -0800 Subject: [PATCH 1/3] build: accept -f - to read Dockerfile from stdin Heavily based on implementation by David Sheets Signed-off-by: David Sheets Signed-off-by: Tonis Tiigi Upstream-commit: 3f6dc81e10b8b813fffaa9b4167a60c5a507fa38 Component: engine --- components/engine/cli/command/image/build.go | 66 ++++++++++++-- .../engine/cli/command/image/build/context.go | 31 ++++--- .../integration-cli/docker_cli_build_test.go | 75 ++++++++++++++++ components/engine/pkg/archive/archive.go | 86 +++++++++++++++++++ components/engine/pkg/archive/archive_test.go | 56 ++++++++++++ 5 files changed, 297 insertions(+), 17 deletions(-) diff --git a/components/engine/cli/command/image/build.go b/components/engine/cli/command/image/build.go index b14b0356ca..f6984619c1 100644 --- a/components/engine/cli/command/image/build.go +++ b/components/engine/cli/command/image/build.go @@ -6,10 +6,12 @@ import ( "bytes" "fmt" "io" + "io/ioutil" "os" "path/filepath" "regexp" "runtime" + "time" "github.com/docker/distribution/reference" "github.com/docker/docker/api" @@ -25,6 +27,7 @@ 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" @@ -141,6 +144,7 @@ func (out *lastProgressOutput) WriteProgress(prog progress.Progress) error { func runBuild(dockerCli *command.DockerCli, options buildOptions) error { var ( buildCtx io.ReadCloser + dockerfileCtx io.ReadCloser err error contextDir string tempDir string @@ -157,6 +161,13 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { buildBuff = bytes.NewBuffer(nil) } + if options.dockerfileName == "-" { + if specifiedContext == "-" { + return errors.New("invalid argument: can't use stdin for both build context and dockerfile") + } + dockerfileCtx = dockerCli.In() + } + switch { case specifiedContext == "-": buildCtx, relDockerfile, err = build.GetContextFromReader(dockerCli.In(), options.dockerfileName) @@ -214,11 +225,11 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { // removed. The daemon will remove them for us, if needed, after it // parses the Dockerfile. Ignore errors here, as they will have been // caught by validateContextDirectory above. - var includes = []string{"."} - keepThem1, _ := fileutils.Matches(".dockerignore", excludes) - keepThem2, _ := fileutils.Matches(relDockerfile, excludes) - if keepThem1 || keepThem2 { - includes = append(includes, ".dockerignore", relDockerfile) + if keep, _ := fileutils.Matches(".dockerignore", excludes); keep { + excludes = append(excludes, "!.dockerignore") + } + if keep, _ := fileutils.Matches(relDockerfile, excludes); keep && dockerfileCtx == nil { + excludes = append(excludes, "!"+relDockerfile) } compression := archive.Uncompressed @@ -228,13 +239,56 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { buildCtx, err = archive.TarWithOptions(contextDir, &archive.TarOptions{ Compression: compression, ExcludePatterns: excludes, - IncludeFiles: includes, }) if err != nil { return err } } + // replace Dockerfile if added dynamically + if dockerfileCtx != nil { + file, err := ioutil.ReadAll(dockerfileCtx) + dockerfileCtx.Close() + if err != nil { + return 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{ + randomName: func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + return hdrTmpl, file, nil + }, + ".dockerignore": func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + if h == nil { + h = hdrTmpl + } + extraIgnore := randomName + "\n" + b := &bytes.Buffer{} + if content != nil { + _, err := b.ReadFrom(content) + if err != nil { + return nil, nil, err + } + } else { + extraIgnore += ".dockerignore\n" + } + b.Write([]byte("\n" + extraIgnore)) + return h, b.Bytes(), nil + }, + }) + relDockerfile = randomName + } + ctx := context.Background() var resolvedTags []*resolvedTag diff --git a/components/engine/cli/command/image/build/context.go b/components/engine/cli/command/image/build/context.go index 85d319e0b7..348c721931 100644 --- a/components/engine/cli/command/image/build/context.go +++ b/components/engine/cli/command/image/build/context.go @@ -89,6 +89,10 @@ func GetContextFromReader(r io.ReadCloser, dockerfileName string) (out io.ReadCl return ioutils.NewReadCloserWrapper(buf, func() error { return r.Close() }), dockerfileName, nil } + if dockerfileName == "-" { + return nil, "", errors.New("build context is not an archive") + } + // Input should be read as a Dockerfile. tmpDir, err := ioutil.TempDir("", "docker-build-context-") if err != nil { @@ -166,7 +170,7 @@ func GetContextFromLocalDir(localDir, dockerfileName string) (absContextDir, rel // When using a local context directory, when the Dockerfile is specified // with the `-f/--file` option then it is considered relative to the // current directory and not the context directory. - if dockerfileName != "" { + if dockerfileName != "" && dockerfileName != "-" { if dockerfileName, err = filepath.Abs(dockerfileName); err != nil { return "", "", errors.Errorf("unable to get absolute path to Dockerfile: %v", err) } @@ -220,6 +224,8 @@ func getDockerfileRelPath(givenContextDir, givenDockerfile string) (absContextDi absDockerfile = altPath } } + } else if absDockerfile == "-" { + absDockerfile = filepath.Join(absContextDir, DefaultDockerfileName) } // If not already an absolute path, the Dockerfile path should be joined to @@ -234,18 +240,21 @@ func getDockerfileRelPath(givenContextDir, givenDockerfile string) (absContextDi // an issue in golang. On Windows, EvalSymLinks does not work on UNC file // paths (those starting with \\). This hack means that when using links // on UNC paths, they will not be followed. - if !isUNC(absDockerfile) { - absDockerfile, err = filepath.EvalSymlinks(absDockerfile) - if err != nil { - return "", "", errors.Errorf("unable to evaluate symlinks in Dockerfile path: %v", err) - } - } + if givenDockerfile != "-" { + if !isUNC(absDockerfile) { + absDockerfile, err = filepath.EvalSymlinks(absDockerfile) + if err != nil { + return "", "", errors.Errorf("unable to evaluate symlinks in Dockerfile path: %v", err) - if _, err := os.Lstat(absDockerfile); err != nil { - if os.IsNotExist(err) { - return "", "", errors.Errorf("Cannot locate Dockerfile: %q", absDockerfile) + } + } + + if _, err := os.Lstat(absDockerfile); err != nil { + if os.IsNotExist(err) { + return "", "", errors.Errorf("Cannot locate Dockerfile: %q", absDockerfile) + } + return "", "", errors.Errorf("unable to stat Dockerfile: %v", err) } - return "", "", errors.Errorf("unable to stat Dockerfile: %v", err) } if relDockerfile, err = filepath.Rel(absContextDir, absDockerfile); err != nil { diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index 81d08e5419..a0930a2dcd 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -2024,6 +2024,81 @@ func (s *DockerSuite) TestBuildNoContext(c *check.C) { } } +func (s *DockerSuite) TestBuildDockerfileStdin(c *check.C) { + name := "stdindockerfile" + tmpDir, err := ioutil.TempDir("", "fake-context") + c.Assert(err, check.IsNil) + err = ioutil.WriteFile(filepath.Join(tmpDir, "foo"), []byte("bar"), 0600) + c.Assert(err, check.IsNil) + + icmd.RunCmd(icmd.Cmd{ + Command: []string{dockerBinary, "build", "-t", name, "-f", "-", tmpDir}, + Stdin: strings.NewReader( + `FROM busybox +ADD foo /foo +CMD ["cat", "/foo"]`), + }).Assert(c, icmd.Success) + + res := inspectField(c, name, "Config.Cmd") + c.Assert(strings.TrimSpace(string(res)), checker.Equals, `[cat /foo]`) +} + +func (s *DockerSuite) TestBuildDockerfileStdinConflict(c *check.C) { + name := "stdindockerfiletarcontext" + icmd.RunCmd(icmd.Cmd{ + Command: []string{dockerBinary, "build", "-t", name, "-f", "-", "-"}, + }).Assert(c, icmd.Expected{ + ExitCode: 1, + Err: "use stdin for both build context and dockerfile", + }) +} + +func (s *DockerSuite) TestBuildDockerfileStdinNoExtraFiles(c *check.C) { + s.testBuildDockerfileStdinNoExtraFiles(c, false, false) +} + +func (s *DockerSuite) TestBuildDockerfileStdinDockerignore(c *check.C) { + s.testBuildDockerfileStdinNoExtraFiles(c, true, false) +} + +func (s *DockerSuite) TestBuildDockerfileStdinDockerignoreIgnored(c *check.C) { + s.testBuildDockerfileStdinNoExtraFiles(c, true, true) +} + +func (s *DockerSuite) testBuildDockerfileStdinNoExtraFiles(c *check.C, hasDockerignore, ignoreDockerignore bool) { + name := "stdindockerfilenoextra" + tmpDir, err := ioutil.TempDir("", "fake-context") + c.Assert(err, check.IsNil) + err = ioutil.WriteFile(filepath.Join(tmpDir, "foo"), []byte("bar"), 0600) + c.Assert(err, check.IsNil) + if hasDockerignore { + // test that this file is removed + err = ioutil.WriteFile(filepath.Join(tmpDir, "Dockerfile"), []byte(""), 0600) + c.Assert(err, check.IsNil) + ignores := "Dockerfile\n" + if ignoreDockerignore { + ignores += ".dockerignore\n" + } + err = ioutil.WriteFile(filepath.Join(tmpDir, ".dockerignore"), []byte(ignores), 0600) + c.Assert(err, check.IsNil) + } + + icmd.RunCmd(icmd.Cmd{ + Command: []string{dockerBinary, "build", "-t", name, "-f", "-", tmpDir}, + Stdin: strings.NewReader( + `FROM busybox +COPY . /baz`), + }).Assert(c, icmd.Success) + + out, _ := dockerCmd(c, "run", "--rm", name, "ls", "-A", "/baz") + if hasDockerignore && !ignoreDockerignore { + c.Assert(strings.TrimSpace(string(out)), checker.Equals, ".dockerignore\nfoo") + } else { + c.Assert(strings.TrimSpace(string(out)), checker.Equals, "foo") + } + +} + func (s *DockerSuite) TestBuildWithVolumeOwnership(c *check.C) { testRequires(c, DaemonIsLinux) name := "testbuildimg" diff --git a/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index 194d76a8c7..53f35037d1 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/pkg/archive/archive.go @@ -14,6 +14,7 @@ import ( "os/exec" "path/filepath" "runtime" + "sort" "strings" "syscall" @@ -225,6 +226,91 @@ func CompressStream(dest io.Writer, compression Compression) (io.WriteCloser, er } } +// TarModifierFunc is a function that can be passed to ReplaceFileTarWrapper to +// define a modification step for a single path +type TarModifierFunc func(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) + +// ReplaceFileTarWrapper converts inputTarStream to a new tar stream +// while replacing a single file called header.Name with new contents. +// If the file with header.Name does not exist it is added to the tar stream. +// TODO: make this into a generic tar conversion function with walkFn argument +func ReplaceFileTarWrapper(inputTarStream io.ReadCloser, mods map[string]TarModifierFunc) io.ReadCloser { + pipeReader, pipeWriter := io.Pipe() + + modKeys := make([]string, 0, len(mods)) + for key := range mods { + modKeys = append(modKeys, key) + } + sort.Strings(modKeys) + + go func() { + tarReader := tar.NewReader(inputTarStream) + tarWriter := tar.NewWriter(pipeWriter) + + defer inputTarStream.Close() + + loop0: + for { + hdr, err := tarReader.Next() + for len(modKeys) > 0 && (err == io.EOF || err == nil && hdr.Name >= modKeys[0]) { + var h *tar.Header + var rdr io.Reader + if hdr != nil && hdr.Name == modKeys[0] { + h = hdr + rdr = tarReader + } + + h2, dt, err := mods[modKeys[0]](modKeys[0], h, rdr) + if err != nil { + pipeWriter.CloseWithError(err) + return + } + if h2 != nil { + h2.Name = modKeys[0] + h2.Size = int64(len(dt)) + if err := tarWriter.WriteHeader(h2); err != nil { + pipeWriter.CloseWithError(err) + return + } + if len(dt) != 0 { + if _, err := tarWriter.Write(dt); err != nil { + pipeWriter.CloseWithError(err) + return + } + } + } + modKeys = modKeys[1:] + if h != nil { + continue loop0 + } + } + + if err == io.EOF { + tarWriter.Close() + pipeWriter.Close() + return + } + + if err != nil { + pipeWriter.CloseWithError(err) + return + } + + if err := tarWriter.WriteHeader(hdr); err != nil { + pipeWriter.CloseWithError(err) + return + } + + if _, err := pools.Copy(tarWriter, tarReader); err != nil { + pipeWriter.CloseWithError(err) + return + } + + } + }() + return pipeReader +} + // Extension returns the extension of a file that uses the specified compression algorithm. func (compression *Compression) Extension() string { switch *compression { diff --git a/components/engine/pkg/archive/archive_test.go b/components/engine/pkg/archive/archive_test.go index 29295c05c2..f2d68f3857 100644 --- a/components/engine/pkg/archive/archive_test.go +++ b/components/engine/pkg/archive/archive_test.go @@ -1160,3 +1160,59 @@ func TestTempArchiveCloseMultipleTimes(t *testing.T) { } } } + +func testReplaceFileTarWrapper(t *testing.T, name string) { + srcDir, err := ioutil.TempDir("", "docker-test-srcDir") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(srcDir) + + destDir, err := ioutil.TempDir("", "docker-test-destDir") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(destDir) + + _, err = prepareUntarSourceDirectory(20, srcDir, false) + if err != nil { + t.Fatal(err) + } + + archive, err := TarWithOptions(srcDir, &TarOptions{}) + if err != nil { + t.Fatal(err) + } + defer archive.Close() + + archive2 := ReplaceFileTarWrapper(archive, map[string]TarModifierFunc{name: func(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + return &tar.Header{ + Mode: 0600, + Typeflag: tar.TypeReg, + }, []byte("foobar"), nil + }}) + + if err := Untar(archive2, destDir, nil); err != nil { + t.Fatal(err) + } + + dt, err := ioutil.ReadFile(filepath.Join(destDir, name)) + if err != nil { + t.Fatal(err) + } + if expected, actual := "foobar", string(dt); actual != expected { + t.Fatalf("file contents mismatch, expected: %q, got %q", expected, actual) + } +} + +func TestReplaceFileTarWrapperNewFile(t *testing.T) { + testReplaceFileTarWrapper(t, "abc") +} + +func TestReplaceFileTarWrapperReplaceFile(t *testing.T) { + testReplaceFileTarWrapper(t, "file-2") +} + +func TestReplaceFileTarWrapperLastFile(t *testing.T) { + testReplaceFileTarWrapper(t, "file-999") +} From 3712bab8c4b428bdbee90e52212359a499cd6cdb Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 5 Apr 2017 12:09:26 -0400 Subject: [PATCH 2/3] Factor out adding dockerfile from stdin. Signed-off-by: Daniel Nephin Upstream-commit: 56bf6de9f7f0d8e35a5170faf0c35ad07c89d04c Component: engine --- components/engine/cli/command/image/build.go | 83 +++++++++++--------- components/engine/pkg/archive/archive.go | 8 +- 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/components/engine/cli/command/image/build.go b/components/engine/cli/command/image/build.go index f6984619c1..965acb4b51 100644 --- a/components/engine/cli/command/image/build.go +++ b/components/engine/cli/command/image/build.go @@ -247,46 +247,10 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { // replace Dockerfile if added dynamically if dockerfileCtx != nil { - file, err := ioutil.ReadAll(dockerfileCtx) - dockerfileCtx.Close() + buildCtx, relDockerfile, err = addDockerfileToBuildContext(dockerfileCtx, buildCtx) if err != nil { return 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{ - randomName: func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { - return hdrTmpl, file, nil - }, - ".dockerignore": func(_ string, h *tar.Header, content io.Reader) (*tar.Header, []byte, error) { - if h == nil { - h = hdrTmpl - } - extraIgnore := randomName + "\n" - b := &bytes.Buffer{} - if content != nil { - _, err := b.ReadFrom(content) - if err != nil { - return nil, nil, err - } - } else { - extraIgnore += ".dockerignore\n" - } - b.Write([]byte("\n" + extraIgnore)) - return h, b.Bytes(), nil - }, - }) - relDockerfile = randomName } ctx := context.Background() @@ -392,6 +356,51 @@ 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 + } + extraIgnore := randomName + "\n" + b := &bytes.Buffer{} + if content != nil { + _, err := b.ReadFrom(content) + if err != nil { + return nil, nil, err + } + } else { + extraIgnore += ".dockerignore\n" + } + b.Write([]byte("\n" + extraIgnore)) + return h, b.Bytes(), nil + }, + }) + return buildCtx, randomName, nil +} + func isLocalDir(c string) bool { _, err := os.Stat(c) return err == nil diff --git a/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index 53f35037d1..3fb1ca4ca2 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/pkg/archive/archive.go @@ -230,10 +230,8 @@ func CompressStream(dest io.Writer, compression Compression) (io.WriteCloser, er // define a modification step for a single path type TarModifierFunc func(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) -// ReplaceFileTarWrapper converts inputTarStream to a new tar stream -// while replacing a single file called header.Name with new contents. -// If the file with header.Name does not exist it is added to the tar stream. -// TODO: make this into a generic tar conversion function with walkFn argument +// ReplaceFileTarWrapper converts inputTarStream to a new tar stream. Files in the +// tar stream are modified if they match any of the keys in mods. func ReplaceFileTarWrapper(inputTarStream io.ReadCloser, mods map[string]TarModifierFunc) io.ReadCloser { pipeReader, pipeWriter := io.Pipe() @@ -255,7 +253,7 @@ func ReplaceFileTarWrapper(inputTarStream io.ReadCloser, mods map[string]TarModi for len(modKeys) > 0 && (err == io.EOF || err == nil && hdr.Name >= modKeys[0]) { var h *tar.Header var rdr io.Reader - if hdr != nil && hdr.Name == modKeys[0] { + if err == nil && hdr != nil && hdr.Name == modKeys[0] { h = hdr rdr = tarReader } From 4adfcf130223f747573fd454793ea26423d14a99 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 5 Apr 2017 18:25:29 -0400 Subject: [PATCH 3/3] Upadte archive.ReplaceFileTarWrapper() to not expect a sorted archive Improve test coverage of ReplaceFileTarWrapper() Signed-off-by: Daniel Nephin Upstream-commit: 8cd6c30a489f7a0210526b0f94469c525ba8e0ee Component: engine --- components/engine/cli/command/image/build.go | 24 +-- .../integration-cli/docker_cli_build_test.go | 32 ++-- components/engine/pkg/archive/archive.go | 113 +++++++------- components/engine/pkg/archive/archive_test.go | 147 ++++++++++++------ .../engine/pkg/testutil/assert/assert.go | 19 ++- 5 files changed, 206 insertions(+), 129 deletions(-) diff --git a/components/engine/cli/command/image/build.go b/components/engine/cli/command/image/build.go index 965acb4b51..5268cbc254 100644 --- a/components/engine/cli/command/image/build.go +++ b/components/engine/cli/command/image/build.go @@ -218,13 +218,14 @@ func runBuild(dockerCli *command.DockerCli, options buildOptions) error { return errors.Errorf("Error checking context: '%s'.", err) } - // If .dockerignore mentions .dockerignore or the Dockerfile - // then make sure we send both files over to the daemon - // because Dockerfile is, obviously, needed no matter what, and - // .dockerignore is needed to know if either one needs to be - // removed. The daemon will remove them for us, if needed, after it - // parses the Dockerfile. Ignore errors here, as they will have been - // caught by validateContextDirectory above. + // If .dockerignore mentions .dockerignore or the Dockerfile then make + // sure we send both files over to the daemon because Dockerfile is, + // obviously, needed no matter what, and .dockerignore is needed to know + // if either one needs to be removed. The daemon will remove them + // if necessary, after it parses the Dockerfile. Ignore errors here, as + // they will have been caught by validateContextDirectory above. + // Excludes are used instead of includes to maintain the order of files + // in the archive. if keep, _ := fileutils.Matches(".dockerignore", excludes); keep { excludes = append(excludes, "!.dockerignore") } @@ -384,17 +385,16 @@ func addDockerfileToBuildContext(dockerfileCtx io.ReadCloser, buildCtx io.ReadCl if h == nil { h = hdrTmpl } - extraIgnore := randomName + "\n" + b := &bytes.Buffer{} if content != nil { - _, err := b.ReadFrom(content) - if err != nil { + if _, err := b.ReadFrom(content); err != nil { return nil, nil, err } } else { - extraIgnore += ".dockerignore\n" + b.WriteString(".dockerignore") } - b.Write([]byte("\n" + extraIgnore)) + b.WriteString("\n" + randomName + "\n") return h, b.Bytes(), nil }, }) diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index a0930a2dcd..014428b1bf 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -2069,34 +2069,40 @@ func (s *DockerSuite) testBuildDockerfileStdinNoExtraFiles(c *check.C, hasDocker name := "stdindockerfilenoextra" tmpDir, err := ioutil.TempDir("", "fake-context") c.Assert(err, check.IsNil) - err = ioutil.WriteFile(filepath.Join(tmpDir, "foo"), []byte("bar"), 0600) - c.Assert(err, check.IsNil) - if hasDockerignore { - // test that this file is removed - err = ioutil.WriteFile(filepath.Join(tmpDir, "Dockerfile"), []byte(""), 0600) + defer os.RemoveAll(tmpDir) + + writeFile := func(filename, content string) { + err = ioutil.WriteFile(filepath.Join(tmpDir, filename), []byte(content), 0600) c.Assert(err, check.IsNil) + } + + writeFile("foo", "bar") + + if hasDockerignore { + // Add an empty Dockerfile to verify that it is not added to the image + writeFile("Dockerfile", "") + ignores := "Dockerfile\n" if ignoreDockerignore { ignores += ".dockerignore\n" } - err = ioutil.WriteFile(filepath.Join(tmpDir, ".dockerignore"), []byte(ignores), 0600) - c.Assert(err, check.IsNil) + writeFile(".dockerignore", ignores) } - icmd.RunCmd(icmd.Cmd{ + result := icmd.RunCmd(icmd.Cmd{ Command: []string{dockerBinary, "build", "-t", name, "-f", "-", tmpDir}, Stdin: strings.NewReader( `FROM busybox COPY . /baz`), - }).Assert(c, icmd.Success) + }) + result.Assert(c, icmd.Success) - out, _ := dockerCmd(c, "run", "--rm", name, "ls", "-A", "/baz") + result = cli.DockerCmd(c, "run", "--rm", name, "ls", "-A", "/baz") if hasDockerignore && !ignoreDockerignore { - c.Assert(strings.TrimSpace(string(out)), checker.Equals, ".dockerignore\nfoo") + c.Assert(result.Stdout(), checker.Equals, ".dockerignore\nfoo\n") } else { - c.Assert(strings.TrimSpace(string(out)), checker.Equals, "foo") + c.Assert(result.Stdout(), checker.Equals, "foo\n") } - } func (s *DockerSuite) TestBuildWithVolumeOwnership(c *check.C) { diff --git a/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index 3fb1ca4ca2..30b3c5b36f 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/pkg/archive/archive.go @@ -14,7 +14,6 @@ import ( "os/exec" "path/filepath" "runtime" - "sort" "strings" "syscall" @@ -227,7 +226,10 @@ func CompressStream(dest io.Writer, compression Compression) (io.WriteCloser, er } // TarModifierFunc is a function that can be passed to ReplaceFileTarWrapper to -// define a modification step for a single path +// modify the contents or header of an entry in the archive. If the file already +// exists in the archive the TarModifierFunc will be called with the Header and +// a reader which will return the files content. If the file does not exist both +// header and content will be nil. type TarModifierFunc func(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) // ReplaceFileTarWrapper converts inputTarStream to a new tar stream. Files in the @@ -235,76 +237,77 @@ type TarModifierFunc func(path string, header *tar.Header, content io.Reader) (* func ReplaceFileTarWrapper(inputTarStream io.ReadCloser, mods map[string]TarModifierFunc) io.ReadCloser { pipeReader, pipeWriter := io.Pipe() - modKeys := make([]string, 0, len(mods)) - for key := range mods { - modKeys = append(modKeys, key) - } - sort.Strings(modKeys) - go func() { tarReader := tar.NewReader(inputTarStream) tarWriter := tar.NewWriter(pipeWriter) - defer inputTarStream.Close() + defer tarWriter.Close() - loop0: + modify := func(name string, original *tar.Header, modifier TarModifierFunc, tarReader io.Reader) error { + header, data, err := modifier(name, original, tarReader) + switch { + case err != nil: + return err + case header == nil: + return nil + } + + header.Name = name + header.Size = int64(len(data)) + if err := tarWriter.WriteHeader(header); err != nil { + return err + } + if len(data) != 0 { + if _, err := tarWriter.Write(data); err != nil { + return err + } + } + return nil + } + + var err error + var originalHeader *tar.Header for { - hdr, err := tarReader.Next() - for len(modKeys) > 0 && (err == io.EOF || err == nil && hdr.Name >= modKeys[0]) { - var h *tar.Header - var rdr io.Reader - if err == nil && hdr != nil && hdr.Name == modKeys[0] { - h = hdr - rdr = tarReader - } - - h2, dt, err := mods[modKeys[0]](modKeys[0], h, rdr) - if err != nil { - pipeWriter.CloseWithError(err) - return - } - if h2 != nil { - h2.Name = modKeys[0] - h2.Size = int64(len(dt)) - if err := tarWriter.WriteHeader(h2); err != nil { - pipeWriter.CloseWithError(err) - return - } - if len(dt) != 0 { - if _, err := tarWriter.Write(dt); err != nil { - pipeWriter.CloseWithError(err) - return - } - } - } - modKeys = modKeys[1:] - if h != nil { - continue loop0 - } - } - + originalHeader, err = tarReader.Next() if err == io.EOF { - tarWriter.Close() - pipeWriter.Close() - return + break } - if err != nil { pipeWriter.CloseWithError(err) return } - if err := tarWriter.WriteHeader(hdr); err != nil { + modifier, ok := mods[originalHeader.Name] + if !ok { + // No modifiers for this file, copy the header and data + if err := tarWriter.WriteHeader(originalHeader); err != nil { + pipeWriter.CloseWithError(err) + return + } + if _, err := pools.Copy(tarWriter, tarReader); err != nil { + pipeWriter.CloseWithError(err) + return + } + continue + } + delete(mods, originalHeader.Name) + + if err := modify(originalHeader.Name, originalHeader, modifier, tarReader); err != nil { pipeWriter.CloseWithError(err) return } - - if _, err := pools.Copy(tarWriter, tarReader); err != nil { - pipeWriter.CloseWithError(err) - return - } - } + + // Apply the modifiers that haven't matched any files in the archive + for name, modifier := range mods { + if err := modify(name, nil, modifier, nil); err != nil { + pipeWriter.CloseWithError(err) + return + } + } + + pipeWriter.Close() + }() return pipeReader } diff --git a/components/engine/pkg/archive/archive_test.go b/components/engine/pkg/archive/archive_test.go index f2d68f3857..b9f8c65f5d 100644 --- a/components/engine/pkg/archive/archive_test.go +++ b/components/engine/pkg/archive/archive_test.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "fmt" + "github.com/docker/docker/pkg/testutil/assert" "io" "io/ioutil" "os" @@ -1161,58 +1162,110 @@ func TestTempArchiveCloseMultipleTimes(t *testing.T) { } } -func testReplaceFileTarWrapper(t *testing.T, name string) { - srcDir, err := ioutil.TempDir("", "docker-test-srcDir") - if err != nil { - t.Fatal(err) +func TestReplaceFileTarWrapper(t *testing.T) { + filesInArchive := 20 + testcases := []struct { + doc string + filename string + modifier TarModifierFunc + expected string + fileCount int + }{ + { + doc: "Modifier creates a new file", + filename: "newfile", + modifier: createModifier(t), + expected: "the new content", + fileCount: filesInArchive + 1, + }, + { + doc: "Modifier replaces a file", + filename: "file-2", + modifier: createOrReplaceModifier, + expected: "the new content", + fileCount: filesInArchive, + }, + { + doc: "Modifier replaces the last file", + filename: fmt.Sprintf("file-%d", filesInArchive-1), + modifier: createOrReplaceModifier, + expected: "the new content", + fileCount: filesInArchive, + }, + { + doc: "Modifier appends to a file", + filename: "file-3", + modifier: appendModifier, + expected: "fooo\nnext line", + fileCount: filesInArchive, + }, } - defer os.RemoveAll(srcDir) - destDir, err := ioutil.TempDir("", "docker-test-destDir") - if err != nil { - t.Fatal(err) + for _, testcase := range testcases { + sourceArchive, cleanup := buildSourceArchive(t, filesInArchive) + defer cleanup() + + resultArchive := ReplaceFileTarWrapper( + sourceArchive, + map[string]TarModifierFunc{testcase.filename: testcase.modifier}) + + actual := readFileFromArchive(t, resultArchive, testcase.filename, testcase.fileCount, testcase.doc) + assert.Equal(t, actual, testcase.expected, testcase.doc) } +} + +func buildSourceArchive(t *testing.T, numberOfFiles int) (io.ReadCloser, func()) { + srcDir, err := ioutil.TempDir("", "docker-test-srcDir") + assert.NilError(t, err) + + _, err = prepareUntarSourceDirectory(numberOfFiles, srcDir, false) + assert.NilError(t, err) + + sourceArchive, err := TarWithOptions(srcDir, &TarOptions{}) + assert.NilError(t, err) + return sourceArchive, func() { + os.RemoveAll(srcDir) + sourceArchive.Close() + } +} + +func createOrReplaceModifier(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + return &tar.Header{ + Mode: 0600, + Typeflag: tar.TypeReg, + }, []byte("the new content"), nil +} + +func createModifier(t *testing.T) TarModifierFunc { + return func(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + assert.Nil(t, content) + return createOrReplaceModifier(path, header, content) + } +} + +func appendModifier(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) { + buffer := bytes.Buffer{} + if content != nil { + if _, err := buffer.ReadFrom(content); err != nil { + return nil, nil, err + } + } + buffer.WriteString("\nnext line") + return &tar.Header{Mode: 0600, Typeflag: tar.TypeReg}, buffer.Bytes(), nil +} + +func readFileFromArchive(t *testing.T, archive io.ReadCloser, name string, expectedCount int, doc string) string { + destDir, err := ioutil.TempDir("", "docker-test-destDir") + assert.NilError(t, err) defer os.RemoveAll(destDir) - _, err = prepareUntarSourceDirectory(20, srcDir, false) - if err != nil { - t.Fatal(err) - } + err = Untar(archive, destDir, nil) + assert.NilError(t, err) - archive, err := TarWithOptions(srcDir, &TarOptions{}) - if err != nil { - t.Fatal(err) - } - defer archive.Close() + files, _ := ioutil.ReadDir(destDir) + assert.Equal(t, len(files), expectedCount, doc) - archive2 := ReplaceFileTarWrapper(archive, map[string]TarModifierFunc{name: func(path string, header *tar.Header, content io.Reader) (*tar.Header, []byte, error) { - return &tar.Header{ - Mode: 0600, - Typeflag: tar.TypeReg, - }, []byte("foobar"), nil - }}) - - if err := Untar(archive2, destDir, nil); err != nil { - t.Fatal(err) - } - - dt, err := ioutil.ReadFile(filepath.Join(destDir, name)) - if err != nil { - t.Fatal(err) - } - if expected, actual := "foobar", string(dt); actual != expected { - t.Fatalf("file contents mismatch, expected: %q, got %q", expected, actual) - } -} - -func TestReplaceFileTarWrapperNewFile(t *testing.T) { - testReplaceFileTarWrapper(t, "abc") -} - -func TestReplaceFileTarWrapperReplaceFile(t *testing.T) { - testReplaceFileTarWrapper(t, "file-2") -} - -func TestReplaceFileTarWrapperLastFile(t *testing.T) { - testReplaceFileTarWrapper(t, "file-999") + content, err := ioutil.ReadFile(filepath.Join(destDir, name)) + assert.NilError(t, err) + return string(content) } diff --git a/components/engine/pkg/testutil/assert/assert.go b/components/engine/pkg/testutil/assert/assert.go index 86736d7c7d..fdc0fab5d8 100644 --- a/components/engine/pkg/testutil/assert/assert.go +++ b/components/engine/pkg/testutil/assert/assert.go @@ -20,9 +20,9 @@ type TestingT interface { // Equal compare the actual value to the expected value and fails the test if // they are not equal. -func Equal(t TestingT, actual, expected interface{}) { +func Equal(t TestingT, actual, expected interface{}, extra ...string) { if expected != actual { - fatal(t, "Expected '%v' (%T) got '%v' (%T)", expected, expected, actual, actual) + fatalWithExtra(t, extra, "Expected '%v' (%T) got '%v' (%T)", expected, expected, actual, actual) } } @@ -103,10 +103,25 @@ func NotNil(t TestingT, obj interface{}) { } } +// Nil fails the test if the object is not nil +func Nil(t TestingT, obj interface{}) { + if obj != nil { + fatal(t, "Expected nil value, got (%T) %s", obj, obj) + } +} + func fatal(t TestingT, format string, args ...interface{}) { t.Fatalf(errorSource()+format, args...) } +func fatalWithExtra(t TestingT, extra []string, format string, args ...interface{}) { + msg := fmt.Sprintf(errorSource()+format, args...) + if len(extra) > 0 { + msg += ": " + strings.Join(extra, ", ") + } + t.Fatalf(msg) +} + // See testing.decorate() func errorSource() string { _, filename, line, ok := runtime.Caller(3)