Merge pull request #38599 from thaJeztah/builder_fix_copy_permissions
builder: fix `COPY --from` should preserve ownership
This commit is contained in:
commit
29de017df7
|
@ -64,6 +64,7 @@ type copyInstruction struct {
|
||||||
dest string
|
dest string
|
||||||
chownStr string
|
chownStr string
|
||||||
allowLocalDecompression bool
|
allowLocalDecompression bool
|
||||||
|
preserveOwnership bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// copier reads a raw COPY or ADD command, fetches remote sources using a downloader,
|
// 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 {
|
type copyFileOptions struct {
|
||||||
decompress bool
|
decompress bool
|
||||||
identity idtools.Identity
|
identity *idtools.Identity
|
||||||
archiver Archiver
|
archiver Archiver
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -532,7 +533,7 @@ func isArchivePath(driver containerfs.ContainerFS, path string) bool {
|
||||||
return err == nil
|
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)
|
destExists, err := isExistingDirectory(dest)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return errors.Wrapf(err, "failed to query destination path")
|
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 {
|
if err := archiver.CopyWithTar(source.path, dest.path); err != nil {
|
||||||
return errors.Wrapf(err, "failed to copy directory")
|
return errors.Wrapf(err, "failed to copy directory")
|
||||||
}
|
}
|
||||||
|
if identity != nil {
|
||||||
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
|
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
|
||||||
return fixPermissions(source.path, dest.path, identity, !destExists)
|
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" {
|
if runtime.GOOS == "windows" && dest.driver.OS() == "linux" {
|
||||||
// LCOW
|
// LCOW
|
||||||
if err := dest.driver.MkdirAll(dest.driver.Dir(dest.path), 0755); err != nil {
|
if err := dest.driver.MkdirAll(dest.driver.Dir(dest.path), 0755); err != nil {
|
||||||
return errors.Wrapf(err, "failed to create new directory")
|
return errors.Wrapf(err, "failed to create new directory")
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, identity); err != nil {
|
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
|
// Normal containers
|
||||||
return errors.Wrapf(err, "failed to create new directory")
|
return errors.Wrapf(err, "failed to create new directory")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if err := archiver.CopyFileWithTar(source.path, dest.path); err != nil {
|
if err := archiver.CopyFileWithTar(source.path, dest.path); err != nil {
|
||||||
return errors.Wrapf(err, "failed to copy file")
|
return errors.Wrapf(err, "failed to copy file")
|
||||||
}
|
}
|
||||||
|
if identity != nil {
|
||||||
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
|
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
|
||||||
return fixPermissions(source.path, dest.path, identity, false)
|
return fixPermissions(source.path, dest.path, *identity, false)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func endsInSlash(driver containerfs.Driver, path string) bool {
|
func endsInSlash(driver containerfs.Driver, path string) bool {
|
||||||
|
|
|
@ -126,7 +126,9 @@ func dispatchCopy(d dispatchRequest, c *instructions.CopyCommand) error {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
copyInstruction.chownStr = c.Chown
|
copyInstruction.chownStr = c.Chown
|
||||||
|
if c.From != "" && copyInstruction.chownStr == "" {
|
||||||
|
copyInstruction.preserveOwnership = true
|
||||||
|
}
|
||||||
return d.builder.performCopy(d, copyInstruction)
|
return d.builder.performCopy(d, copyInstruction)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -204,7 +204,9 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error {
|
||||||
opts := copyFileOptions{
|
opts := copyFileOptions{
|
||||||
decompress: inst.allowLocalDecompression,
|
decompress: inst.allowLocalDecompression,
|
||||||
archiver: b.getArchiver(info.root, destInfo.root),
|
archiver: b.getArchiver(info.root, destInfo.root),
|
||||||
identity: identity,
|
}
|
||||||
|
if !inst.preserveOwnership {
|
||||||
|
opts.identity = &identity
|
||||||
}
|
}
|
||||||
if err := performCopyForInfo(destInfo, info, opts); err != nil {
|
if err := performCopyForInfo(destInfo, info, opts); err != nil {
|
||||||
return errors.Wrapf(err, "failed to copy files")
|
return errors.Wrapf(err, "failed to copy files")
|
||||||
|
|
|
@ -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) {
|
func writeTarRecord(t *testing.T, w *tar.Writer, fn, contents string) {
|
||||||
err := w.WriteHeader(&tar.Header{
|
err := w.WriteHeader(&tar.Header{
|
||||||
Name: fn,
|
Name: fn,
|
||||||
|
|
|
@ -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
|
Loading…
Reference in New Issue