From eea97055fbc66e3bf57c536d074615cf3938360a Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Tue, 8 Jul 2014 12:23:08 -0700 Subject: [PATCH 1/3] Revert "allow overwrite in untar" This reverts commit 5a3d774e5651da772a065282a1fb1a31e19e911c. Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) Upstream-commit: 10d066c9cbc074a72ef3bb4f06a39b691a155082 Component: engine --- components/engine/archive/archive.go | 15 ++++++--------- components/engine/archive/archive_test.go | 3 --- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/components/engine/archive/archive.go b/components/engine/archive/archive.go index 8d8b7412cc..b9701b58af 100644 --- a/components/engine/archive/archive.go +++ b/components/engine/archive/archive.go @@ -383,8 +383,7 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) // identity (uncompressed), gzip, bzip2, xz. // If `dest` does not exist, it is created unless there are multiple entries in `archive`. // In the latter case, an error is returned. -// If `dest` is an existing file, it gets overwritten. -// If `dest` is an existing directory, its files get merged (with overwrite for conflicting files). +// An other error is returned if `dest` exists but is not a directory, to prevent overwriting. func Untar(archive io.Reader, dest string, options *TarOptions) error { if archive == nil { return fmt.Errorf("Empty archive") @@ -400,7 +399,7 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { var ( dirs []*tar.Header - create bool + destNotExist bool multipleEntries bool ) @@ -409,10 +408,9 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { return err } // destination does not exist, so it is assumed it has to be created. - create = true + destNotExist = true } else if !fi.IsDir() { - // destination exists and is not a directory, so it will be overwritten. - create = true + return fmt.Errorf("Trying to untar to `%s`: exists but not a directory", dest) } // Iterate through the files in the archive. @@ -427,7 +425,7 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { } // Return an error if destination needs to be created and there is more than 1 entry in the tar stream. - if create && multipleEntries { + if destNotExist && multipleEntries { return fmt.Errorf("Trying to untar an archive with multiple entries to an inexistant target `%s`: did you mean `%s` instead?", dest, filepath.Dir(dest)) } @@ -447,7 +445,7 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { } var path string - if create { + if destNotExist { path = dest // we are renaming hdr.Name to dest } else { path = filepath.Join(dest, hdr.Name) @@ -467,7 +465,6 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { } } } - if err := createTarFile(path, dest, hdr, tr, options == nil || !options.NoLchown); err != nil { return err } diff --git a/components/engine/archive/archive_test.go b/components/engine/archive/archive_test.go index 1b5e146965..e05cd72e49 100644 --- a/components/engine/archive/archive_test.go +++ b/components/engine/archive/archive_test.go @@ -176,9 +176,6 @@ func TestTarUntarFile(t *testing.T) { if err := ioutil.WriteFile(path.Join(origin, "before", "file"), []byte("hello world"), 0700); err != nil { t.Fatal(err) } - if err := ioutil.WriteFile(path.Join(origin, "after", "file2"), []byte("please overwrite me"), 0700); err != nil { - t.Fatal(err) - } tar, err := TarWithOptions(path.Join(origin, "before"), &TarOptions{Compression: Uncompressed, Includes: []string{"file"}}) if err != nil { From c916fa43d09cda60a6bcd6bafea71d94a35d5f1d Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Tue, 8 Jul 2014 12:26:59 -0700 Subject: [PATCH 2/3] Revert "improve untar when using files instead of directories. Specifies behavior on non-existant targets." This reverts commit 1c8d3106df0bd2aba304c7b3863949ca11c2b133. Conflicts: archive/archive_test.go Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) Upstream-commit: 4b1a464ac106b413fc773869adea9c89a08aee3a Component: engine --- components/engine/archive/archive.go | 37 +++------------------- components/engine/archive/archive_test.go | 38 ----------------------- 2 files changed, 4 insertions(+), 71 deletions(-) diff --git a/components/engine/archive/archive.go b/components/engine/archive/archive.go index b9701b58af..2ba62f5363 100644 --- a/components/engine/archive/archive.go +++ b/components/engine/archive/archive.go @@ -378,12 +378,10 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) } // Untar reads a stream of bytes from `archive`, parses it as a tar archive, -// and unpacks it into the directory at `dest`. +// and unpacks it into the directory at `path`. // The archive may be compressed with one of the following algorithms: // identity (uncompressed), gzip, bzip2, xz. -// If `dest` does not exist, it is created unless there are multiple entries in `archive`. -// In the latter case, an error is returned. -// An other error is returned if `dest` exists but is not a directory, to prevent overwriting. +// FIXME: specify behavior when target path exists vs. doesn't exist. func Untar(archive io.Reader, dest string, options *TarOptions) error { if archive == nil { return fmt.Errorf("Empty archive") @@ -397,21 +395,7 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { tr := tar.NewReader(decompressedArchive) - var ( - dirs []*tar.Header - destNotExist bool - multipleEntries bool - ) - - if fi, err := os.Lstat(dest); err != nil { - if !os.IsNotExist(err) { - return err - } - // destination does not exist, so it is assumed it has to be created. - destNotExist = true - } else if !fi.IsDir() { - return fmt.Errorf("Trying to untar to `%s`: exists but not a directory", dest) - } + var dirs []*tar.Header // Iterate through the files in the archive. for { @@ -424,11 +408,6 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { return err } - // Return an error if destination needs to be created and there is more than 1 entry in the tar stream. - if destNotExist && multipleEntries { - return fmt.Errorf("Trying to untar an archive with multiple entries to an inexistant target `%s`: did you mean `%s` instead?", dest, filepath.Dir(dest)) - } - // Normalize name, for safety and for a simple is-root check hdr.Name = filepath.Clean(hdr.Name) @@ -444,12 +423,7 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { } } - var path string - if destNotExist { - path = dest // we are renaming hdr.Name to dest - } else { - path = filepath.Join(dest, hdr.Name) - } + path := filepath.Join(dest, hdr.Name) // If path exits we almost always just want to remove and replace it // The only exception is when it is a directory *and* the file from @@ -469,9 +443,6 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error { return err } - // Successfully added an entry. Predicting multiple entries for next iteration (not current one). - multipleEntries = true - // Directory mtimes must be handled at the end to avoid further // file creation in them to modify the directory mtime if hdr.Typeflag == tar.TypeDir { diff --git a/components/engine/archive/archive_test.go b/components/engine/archive/archive_test.go index e05cd72e49..61ee0af8e7 100644 --- a/components/engine/archive/archive_test.go +++ b/components/engine/archive/archive_test.go @@ -160,44 +160,6 @@ func TestTarWithOptions(t *testing.T) { } } -func TestTarUntarFile(t *testing.T) { - origin, err := ioutil.TempDir("", "docker-test-untar-origin-file") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(origin) - - if err := os.MkdirAll(path.Join(origin, "before"), 0700); err != nil { - t.Fatal(err) - } - if err := os.MkdirAll(path.Join(origin, "after"), 0700); err != nil { - t.Fatal(err) - } - if err := ioutil.WriteFile(path.Join(origin, "before", "file"), []byte("hello world"), 0700); err != nil { - t.Fatal(err) - } - - tar, err := TarWithOptions(path.Join(origin, "before"), &TarOptions{Compression: Uncompressed, Includes: []string{"file"}}) - if err != nil { - t.Fatal(err) - } - - if err := Untar(tar, path.Join(origin, "after", "file2"), nil); err != nil { - t.Fatal(err) - } - - catCmd := exec.Command("cat", path.Join(origin, "after", "file2")) - out, err := CmdStream(catCmd, nil) - if err != nil { - t.Fatalf("Failed to start command: %s", err) - } - if output, err := ioutil.ReadAll(out); err != nil { - t.Error(err) - } else if string(output) != "hello world" { - t.Fatalf("Expected 'hello world', got '%s'", output) - } -} - // Some tar archives such as http://haproxy.1wt.eu/download/1.5/src/devel/haproxy-1.5-dev21.tar.gz // use PAX Global Extended Headers. // Failing prevents the archives from being uncompressed during ADD From ff37e700fcf17339b1b873f41aa54b3d2df1675c Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 8 Jul 2014 15:34:04 -0400 Subject: [PATCH 3/3] Tests for ADD tar Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: tiborvass) Upstream-commit: cfa4999d695d98f30d942460576d5f9c03a64d92 Component: engine --- .../build_tests/TestBuildAddTar/1/Dockerfile | 3 ++ .../build_tests/TestBuildAddTar/1/test.tar | Bin 0 -> 2560 bytes .../build_tests/TestBuildAddTar/2/Dockerfile | 3 ++ .../build_tests/TestBuildAddTar/2/test.tar | Bin 0 -> 2560 bytes .../integration-cli/docker_cli_build_test.go | 32 ++++++++++++++++++ 5 files changed, 38 insertions(+) create mode 100644 components/engine/integration-cli/build_tests/TestBuildAddTar/1/Dockerfile create mode 100644 components/engine/integration-cli/build_tests/TestBuildAddTar/1/test.tar create mode 100644 components/engine/integration-cli/build_tests/TestBuildAddTar/2/Dockerfile create mode 100644 components/engine/integration-cli/build_tests/TestBuildAddTar/2/test.tar diff --git a/components/engine/integration-cli/build_tests/TestBuildAddTar/1/Dockerfile b/components/engine/integration-cli/build_tests/TestBuildAddTar/1/Dockerfile new file mode 100644 index 0000000000..2091b0e4d9 --- /dev/null +++ b/components/engine/integration-cli/build_tests/TestBuildAddTar/1/Dockerfile @@ -0,0 +1,3 @@ +FROM busybox +ADD test.tar /test.tar +RUN cat /test.tar/test/foo diff --git a/components/engine/integration-cli/build_tests/TestBuildAddTar/1/test.tar b/components/engine/integration-cli/build_tests/TestBuildAddTar/1/test.tar new file mode 100644 index 0000000000000000000000000000000000000000..33639c647642cce352588cd4f13c021a0b222455 GIT binary patch literal 2560 zcmeH^K?=k$2t~7=Q+R`M8WXQD*XTe4T?Ja_{$rYUGtle;h3ZC(V!rRow93=<4MgM+ zz?B?p#(}n4pSFP4;6r3aW(L%P$U*2Ut8V|UGA=4j=1*Q4AL>|2jsAW|IZ^`}lb32q t@jvC|2jsAW|IZ^`}lb32q t@jvC -2 { + t.Fatalf("Could not find contents of %s in build output", x) + } + } + + for _, n := range []string{"1", "2"} { + buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestBuildAddTar", n) + buildCmd := exec.Command(dockerBinary, "build", "-t", "testbuildaddtar", ".") + buildCmd.Dir = buildDirectory + out, _, err := runCommandWithOutput(buildCmd) + errorOut(err, t, fmt.Sprintf("build failed to complete for TestBuildAddTar/%s: %v", n, err)) + checkOutput(out) + } +}