diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index ad9b08dfef..4e1c7e06dc 100644 --- a/builder/dockerfile/copy.go +++ b/builder/dockerfile/copy.go @@ -64,6 +64,7 @@ type copyInstruction struct { dest string chownStr string allowLocalDecompression bool + preserveOwnership bool } // copier reads a raw COPY or ADD command, fetches remote sources using a downloader, @@ -466,7 +467,7 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b type copyFileOptions struct { decompress bool - identity idtools.Identity + identity *idtools.Identity archiver Archiver } @@ -532,7 +533,7 @@ func isArchivePath(driver containerfs.ContainerFS, path string) bool { return err == nil } -func copyDirectory(archiver Archiver, source, dest *copyEndpoint, identity idtools.Identity) error { +func copyDirectory(archiver Archiver, source, dest *copyEndpoint, identity *idtools.Identity) error { destExists, err := isExistingDirectory(dest) if err != nil { return errors.Wrapf(err, "failed to query destination path") @@ -541,28 +542,40 @@ func copyDirectory(archiver Archiver, source, dest *copyEndpoint, identity idtoo if err := archiver.CopyWithTar(source.path, dest.path); err != nil { return errors.Wrapf(err, "failed to copy directory") } - // TODO: @gupta-ak. Investigate how LCOW permission mappings will work. - return fixPermissions(source.path, dest.path, identity, !destExists) + if identity != nil { + // TODO: @gupta-ak. Investigate how LCOW permission mappings will work. + return fixPermissions(source.path, dest.path, *identity, !destExists) + } + return nil } -func copyFile(archiver Archiver, source, dest *copyEndpoint, identity idtools.Identity) error { +func copyFile(archiver Archiver, source, dest *copyEndpoint, identity *idtools.Identity) error { if runtime.GOOS == "windows" && dest.driver.OS() == "linux" { // LCOW if err := dest.driver.MkdirAll(dest.driver.Dir(dest.path), 0755); err != nil { return errors.Wrapf(err, "failed to create new directory") } } else { - if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, identity); err != nil { - // Normal containers - return errors.Wrapf(err, "failed to create new directory") + if identity == nil { + if err := os.MkdirAll(filepath.Dir(dest.path), 0755); err != nil { + return err + } + } else { + if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, *identity); err != nil { + // Normal containers + return errors.Wrapf(err, "failed to create new directory") + } } } if err := archiver.CopyFileWithTar(source.path, dest.path); err != nil { return errors.Wrapf(err, "failed to copy file") } - // TODO: @gupta-ak. Investigate how LCOW permission mappings will work. - return fixPermissions(source.path, dest.path, identity, false) + if identity != nil { + // TODO: @gupta-ak. Investigate how LCOW permission mappings will work. + return fixPermissions(source.path, dest.path, *identity, false) + } + return nil } func endsInSlash(driver containerfs.Driver, path string) bool { diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 07c434259b..4fd59a7831 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -126,7 +126,9 @@ func dispatchCopy(d dispatchRequest, c *instructions.CopyCommand) error { return err } copyInstruction.chownStr = c.Chown - + if c.From != "" && copyInstruction.chownStr == "" { + copyInstruction.preserveOwnership = true + } return d.builder.performCopy(d, copyInstruction) } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index fbe301cdae..fa35c4f3c0 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -204,7 +204,9 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error { opts := copyFileOptions{ decompress: inst.allowLocalDecompression, archiver: b.getArchiver(info.root, destInfo.root), - identity: identity, + } + if !inst.preserveOwnership { + opts.identity = &identity } if err := performCopyForInfo(destInfo, info, opts); err != nil { return errors.Wrapf(err, "failed to copy files") diff --git a/integration/build/build_test.go b/integration/build/build_test.go index 65856a9bb5..c09f746f35 100644 --- a/integration/build/build_test.go +++ b/integration/build/build_test.go @@ -522,6 +522,45 @@ func TestBuildWithEmptyDockerfile(t *testing.T) { } } +func TestBuildPreserveOwnership(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "FIXME") + skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "broken in earlier versions") + + ctx := context.Background() + + dockerfile, err := ioutil.ReadFile("testdata/Dockerfile.testBuildPreserveOwnership") + assert.NilError(t, err) + + source := fakecontext.New(t, "", fakecontext.WithDockerfile(string(dockerfile))) + defer source.Close() + + apiclient := testEnv.APIClient() + + for _, target := range []string{"copy_from", "copy_from_chowned"} { + t.Run(target, func(t *testing.T) { + resp, err := apiclient.ImageBuild( + ctx, + source.AsTarReader(t), + types.ImageBuildOptions{ + Remove: true, + ForceRemove: true, + Target: target, + }, + ) + assert.NilError(t, err) + + out := bytes.NewBuffer(nil) + assert.NilError(t, err) + _, err = io.Copy(out, resp.Body) + _ = resp.Body.Close() + if err != nil { + t.Log(out) + } + assert.NilError(t, err) + }) + } +} + func writeTarRecord(t *testing.T, w *tar.Writer, fn, contents string) { err := w.WriteHeader(&tar.Header{ Name: fn, diff --git a/integration/build/testdata/Dockerfile.testBuildPreserveOwnership b/integration/build/testdata/Dockerfile.testBuildPreserveOwnership new file mode 100644 index 0000000000..ba9d059942 --- /dev/null +++ b/integration/build/testdata/Dockerfile.testBuildPreserveOwnership @@ -0,0 +1,57 @@ +# Set up files and directories with known ownership +FROM busybox AS source +RUN touch /file && chown 100:200 /file \ + && mkdir -p /dir/subdir \ + && touch /dir/subdir/nestedfile \ + && chown 100:200 /dir \ + && chown 101:201 /dir/subdir \ + && chown 102:202 /dir/subdir/nestedfile + +FROM busybox AS test_base +RUN mkdir -p /existingdir/existingsubdir \ + && touch /existingdir/existingfile \ + && chown 500:600 /existingdir \ + && chown 501:601 /existingdir/existingsubdir \ + && chown 501:601 /existingdir/existingfile + + +# Copy files from the source stage +FROM test_base AS copy_from +COPY --from=source /file . +# Copy to a non-existing target directory creates the target directory (as root), then copies the _contents_ of the source directory into it +COPY --from=source /dir /dir +# Copying to an existing target directory will copy the _contents_ of the source directory into it +COPY --from=source /dir/. /existingdir + +RUN e="100:200"; p="/file" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="0:0"; p="/dir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="101:201"; p="/dir/subdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="102:202"; p="/dir/subdir/nestedfile" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ +# Existing files and directories ownership should not be modified + && e="500:600"; p="/existingdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="501:601"; p="/existingdir/existingsubdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="501:601"; p="/existingdir/existingfile" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ +# But new files and directories should maintain their ownership + && e="101:201"; p="/existingdir/subdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="102:202"; p="/existingdir/subdir/nestedfile"; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi + + +# Copy files from the source stage and chown them. +FROM test_base AS copy_from_chowned +COPY --from=source --chown=300:400 /file . +# Copy to a non-existing target directory creates the target directory (as root), then copies the _contents_ of the source directory into it +COPY --from=source --chown=300:400 /dir /dir +# Copying to an existing target directory copies the _contents_ of the source directory into it +COPY --from=source --chown=300:400 /dir/. /existingdir + +RUN e="300:400"; p="/file" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="300:400"; p="/dir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="300:400"; p="/dir/subdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="300:400"; p="/dir/subdir/nestedfile" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ +# Existing files and directories ownership should not be modified + && e="500:600"; p="/existingdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="501:601"; p="/existingdir/existingsubdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="501:601"; p="/existingdir/existingfile" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ +# But new files and directories should be chowned + && e="300:400"; p="/existingdir/subdir" ; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi \ + && e="300:400"; p="/existingdir/subdir/nestedfile"; a=`stat -c "%u:%g" "$p"`; if [ "$a" != "$e" ]; then echo "incorrect ownership on $p. expected $e, got $a"; exit 1; fi