From 44456ba065118231d02334f6b1faee7971a41d3a Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Tue, 8 Jul 2014 12:23:08 -0700 Subject: [PATCH 1/4] Revert "allow overwrite in untar" This reverts commit 5a3d774e5651da772a065282a1fb1a31e19e911c. Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: vieux) --- archive/archive.go | 15 ++++++--------- archive/archive_test.go | 3 --- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/archive/archive.go b/archive/archive.go index 8d8b7412cc..b9701b58af 100644 --- a/archive/archive.go +++ b/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/archive/archive_test.go b/archive/archive_test.go index 1b5e146965..e05cd72e49 100644 --- a/archive/archive_test.go +++ b/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 94c27842d8d129bdacde98d81e2013ff13143ee8 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Tue, 8 Jul 2014 12:26:59 -0700 Subject: [PATCH 2/4] 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) Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: vieux) --- archive/archive.go | 37 ++++--------------------------------- archive/archive_test.go | 38 -------------------------------------- 2 files changed, 4 insertions(+), 71 deletions(-) diff --git a/archive/archive.go b/archive/archive.go index b9701b58af..2ba62f5363 100644 --- a/archive/archive.go +++ b/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/archive/archive_test.go b/archive/archive_test.go index e05cd72e49..61ee0af8e7 100644 --- a/archive/archive_test.go +++ b/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 699556e6619ce6806030e87ee1f3ea57bcdefd81 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Tue, 8 Jul 2014 15:34:04 -0400 Subject: [PATCH 3/4] Tests for ADD tar Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: tiborvass) Conflicts: integration-cli/docker_cli_build_test.go Docker-DCO-1.1-Signed-off-by: Tibor Vass (github: vieux) --- .../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 | 199 ++++++++++++++++++ 5 files changed, 205 insertions(+) create mode 100644 integration-cli/build_tests/TestBuildAddTar/1/Dockerfile create mode 100644 integration-cli/build_tests/TestBuildAddTar/1/test.tar create mode 100644 integration-cli/build_tests/TestBuildAddTar/2/Dockerfile create mode 100644 integration-cli/build_tests/TestBuildAddTar/2/test.tar diff --git a/integration-cli/build_tests/TestBuildAddTar/1/Dockerfile b/integration-cli/build_tests/TestBuildAddTar/1/Dockerfile new file mode 100644 index 0000000000..2091b0e4d9 --- /dev/null +++ b/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/integration-cli/build_tests/TestBuildAddTar/1/test.tar b/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 /tmp/passwd' +RUN mkdir -p /var/run/sshd +RUN [ "$(cat /tmp/passwd)" = "root:testpass" ] +RUN [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ]`, + true) + if err != nil { + t.Fatal(err) + } + logDone("build - line break with \\") +} + +func TestBuildEOLInLine(t *testing.T) { + name := "testbuildeolinline" + defer deleteImages(name) + _, err := buildImage(name, + `FROM busybox +RUN sh -c 'echo root:testpass > /tmp/passwd' +RUN echo "foo \n bar"; echo "baz" +RUN mkdir -p /var/run/sshd +RUN [ "$(cat /tmp/passwd)" = "root:testpass" ] +RUN [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ]`, + true) + if err != nil { + t.Fatal(err) + } + logDone("build - end of line in dockerfile instruction") +} + +func TestBuildCommentsShebangs(t *testing.T) { + name := "testbuildcomments" + defer deleteImages(name) + _, err := buildImage(name, + `FROM busybox +# This is an ordinary comment. +RUN { echo '#!/bin/sh'; echo 'echo hello world'; } > /hello.sh +RUN [ ! -x /hello.sh ] +# comment with line break \ +RUN chmod +x /hello.sh +RUN [ -x /hello.sh ] +RUN [ "$(cat /hello.sh)" = $'#!/bin/sh\necho hello world' ] +RUN [ "$(/hello.sh)" = "hello world" ]`, + true) + if err != nil { + t.Fatal(err) + } + logDone("build - comments and shebangs") +} + +func TestBuildUsersAndGroups(t *testing.T) { + name := "testbuildusers" + defer deleteImages(name) + _, err := buildImage(name, + `FROM busybox + +# Make sure our defaults work +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)" = '0:0/root:root' ] + +# TODO decide if "args.user = strconv.Itoa(syscall.Getuid())" is acceptable behavior for changeUser in sysvinit instead of "return nil" when "USER" isn't specified (so that we get the proper group list even if that is the empty list, even in the default case of not supplying an explicit USER to run as, which implies USER 0) +USER root +RUN [ "$(id -G):$(id -Gn)" = '0 10:root wheel' ] + +# Setup dockerio user and group +RUN echo 'dockerio:x:1001:1001::/bin:/bin/false' >> /etc/passwd +RUN echo 'dockerio:x:1001:' >> /etc/group + +# Make sure we can switch to our user and all the information is exactly as we expect it to be +USER dockerio +RUN id -G +RUN id -Gn +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ] + +# Switch back to root and double check that worked exactly as we might expect it to +USER root +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '0:0/root:root/0 10:root wheel' ] + +# Add a "supplementary" group for our dockerio user +RUN echo 'supplementary:x:1002:dockerio' >> /etc/group + +# ... and then go verify that we get it like we expect +USER dockerio +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001 1002:dockerio supplementary' ] +USER 1001 +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001 1002:dockerio supplementary' ] + +# super test the new "user:group" syntax +USER dockerio:dockerio +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ] +USER 1001:dockerio +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ] +USER dockerio:1001 +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ] +USER 1001:1001 +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1001/dockerio:dockerio/1001:dockerio' ] +USER dockerio:supplementary +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ] +USER dockerio:1002 +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ] +USER 1001:supplementary +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ] +USER 1001:1002 +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1001:1002/dockerio:supplementary/1002:supplementary' ] + +# make sure unknown uid/gid still works properly +USER 1042:1043 +RUN [ "$(id -u):$(id -g)/$(id -un):$(id -gn)/$(id -G):$(id -Gn)" = '1042:1043/1042:1043/1043:1043' ]`, + true) + if err != nil { + t.Fatal(err) + } + logDone("build - users and groups") +} + +func TestBuildEnvUsage(t *testing.T) { + name := "testbuildenvusage" + defer deleteImages(name) + dockerfile := `FROM busybox +ENV FOO /foo/baz +ENV BAR /bar +ENV BAZ $BAR +ENV FOOPATH $PATH:$FOO +RUN [ "$BAR" = "$BAZ" ] +RUN [ "$FOOPATH" = "$PATH:/foo/baz" ] +ENV FROM hello/docker/world +ENV TO /docker/world/hello +ADD $FROM $TO +RUN [ "$(cat $TO)" = "hello" ]` + ctx, err := fakeContext(dockerfile, map[string]string{ + "hello/docker/world": "hello", + }) + if err != nil { + t.Fatal(err) + } + _, err = buildImageFromContext(name, ctx, true) + if err != nil { + t.Fatal(err) + } + logDone("build - environment variables usage") +} + +func TestBuildAddScript(t *testing.T) { + name := "testbuildaddscript" + defer deleteImages(name) + dockerfile := ` +FROM busybox +ADD test /test +RUN ["chmod","+x","/test"] +RUN ["/test"] +RUN [ "$(cat /testfile)" = 'test!' ]` + ctx, err := fakeContext(dockerfile, map[string]string{ + "test": "#!/bin/sh\necho 'test!' > /testfile", + }) + if err != nil { + t.Fatal(err) + } + _, err = buildImageFromContext(name, ctx, true) + if err != nil { + t.Fatal(err) + } + logDone("build - add and run script") +} + +func TestBuildAddTar(t *testing.T) { + + checkOutput := func(out string) { + n := -1 + x := "" + for i, line := range strings.Split(out, "\n") { + if strings.HasPrefix(line, "Step 2") { + n = i + 2 + x = line[strings.Index(line, "cat ")+4:] + } + if i == n { + if line != "Hi" { + t.Fatalf("Could not find contents of %s (expected 'Hi' got '%s'", x, line) + } + n = -2 + } + } + if n > -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) + } +} From dc62f3cdccfd744c7e1d345da7f167df98cf9686 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Tue, 8 Jul 2014 21:35:22 +0000 Subject: [PATCH 4/4] Bump version to v1.1.1 Docker-DCO-1.1-Signed-off-by: Victor Vieux (github: vieux) --- CHANGELOG.md | 5 +++++ VERSION | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e55cb0c759..327a8a7e69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 1.1.1 (2014-07-09) + +#### Builder +* Fix issue with ADD + ## 1.1.0 (2014-07-03) #### Notable features since 1.0.1 diff --git a/VERSION b/VERSION index 9084fa2f71..524cb55242 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.0 +1.1.1