From b638bc6f17f7cef6534b20dd44845e5b066ae2cc Mon Sep 17 00:00:00 2001 From: Mauricio Garavaglia Date: Mon, 23 Nov 2015 02:22:22 -0300 Subject: [PATCH] Fix files ownership when ADD is used Signed-off-by: Mauricio Garavaglia --- daemon/daemonbuilder/builder.go | 4 ++- integration-cli/docker_cli_build_test.go | 40 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/daemon/daemonbuilder/builder.go b/daemon/daemonbuilder/builder.go index 0b5528f16f..4b50487ac6 100644 --- a/daemon/daemonbuilder/builder.go +++ b/daemon/daemonbuilder/builder.go @@ -127,6 +127,7 @@ func (d Docker) Release(sessionID string, activeImages []string) { func (d Docker) Copy(c *daemon.Container, destPath string, src builder.FileInfo, decompress bool) error { srcPath := src.Path() destExists := true + destDir := false rootUID, rootGID := d.Daemon.GetRemappedUIDGID() // Work in daemon-local OS specific file paths @@ -140,6 +141,7 @@ func (d Docker) Copy(c *daemon.Container, destPath string, src builder.FileInfo, // Preserve the trailing slash // TODO: why are we appending another path separator if there was already one? if strings.HasSuffix(destPath, string(os.PathSeparator)) || destPath == "." { + destDir = true dest += string(os.PathSeparator) } @@ -182,7 +184,7 @@ func (d Docker) Copy(c *daemon.Container, destPath string, src builder.FileInfo, } // only needed for fixPermissions, but might as well put it before CopyFileWithTar - if destExists && destStat.IsDir() { + if destDir || (destExists && destStat.IsDir()) { destPath = filepath.Join(destPath, src.Name()) } diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index faeec5465d..d3e9af0ac4 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -3252,6 +3252,46 @@ func (s *DockerSuite) TestBuildAddFileNotFound(c *check.C) { } } +func (s *DockerSuite) TestBuildAddChangeOwnership(c *check.C) { + testRequires(c, DaemonIsLinux) + name := "testbuildaddown" + + ctx := func() *FakeContext { + dockerfile := ` + FROM busybox + ADD foo /bar/ + RUN [ $(stat -c %U:%G "/bar") = 'root:root' ] + RUN [ $(stat -c %U:%G "/bar/foo") = 'root:root' ] + ` + tmpDir, err := ioutil.TempDir("", "fake-context") + c.Assert(err, check.IsNil) + testFile, err := os.Create(filepath.Join(tmpDir, "foo")) + if err != nil { + c.Fatalf("failed to create foo file: %v", err) + } + defer testFile.Close() + + chownCmd := exec.Command("chown", "daemon:daemon", "foo") + chownCmd.Dir = tmpDir + out, _, err := runCommandWithOutput(chownCmd) + if err != nil { + c.Fatal(err, out) + } + + if err := ioutil.WriteFile(filepath.Join(tmpDir, "Dockerfile"), []byte(dockerfile), 0644); err != nil { + c.Fatalf("failed to open destination dockerfile: %v", err) + } + return fakeContextFromDir(tmpDir) + }() + + defer ctx.Close() + + if _, err := buildImageFromContext(name, ctx, true); err != nil { + c.Fatalf("build failed to complete for TestBuildAddChangeOwnership: %v", err) + } + +} + func (s *DockerSuite) TestBuildInheritance(c *check.C) { testRequires(c, DaemonIsLinux) name := "testbuildinheritance"