From bcc4a146e4c13e5525a0fcc1ad32ebda383a832b Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 30 Jul 2019 15:45:14 -0700 Subject: [PATCH] Support applying with parent directories Signed-off-by: Derek McGowan --- archive/tar.go | 159 +++++++++++++++++++--------- archive/tar_linux_test.go | 183 +++++++++++++++++++++++++++++++++ archive/tar_opts.go | 27 ++++- archive/tar_opts_linux.go | 6 -- archive/tar_opts_other.go | 25 ----- archive/tar_opts_windows.go | 19 +--- archive/tar_unix.go | 77 ++++++++++++-- archive/tar_windows.go | 31 +++--- diff/apply/apply_linux.go | 90 +++++++++++----- diff/apply/apply_linux_test.go | 24 +++-- diff/windows/windows.go | 2 +- 11 files changed, 483 insertions(+), 160 deletions(-) create mode 100644 archive/tar_linux_test.go delete mode 100644 archive/tar_opts_other.go diff --git a/archive/tar.go b/archive/tar.go index 7b1732c38..3a1e77a7f 100644 --- a/archive/tar.go +++ b/archive/tar.go @@ -110,11 +110,15 @@ func Apply(ctx context.Context, root string, r io.Reader, opts ...ApplyOpt) (int if options.Filter == nil { options.Filter = all } + if options.applyFunc == nil { + options.applyFunc = applyNaive + } - return apply(ctx, root, tar.NewReader(r), options) + return options.applyFunc(ctx, root, tar.NewReader(r), options) } -// applyNaive applies a tar stream of an OCI style diff tar. +// applyNaive applies a tar stream of an OCI style diff tar to a directory +// applying each file as either a whole file or whiteout. // See https://github.com/opencontainers/image-spec/blob/master/layer.md#applying-changesets func applyNaive(ctx context.Context, root string, tr *tar.Reader, options ApplyOptions) (size int64, err error) { var ( @@ -123,8 +127,50 @@ func applyNaive(ctx context.Context, root string, tr *tar.Reader, options ApplyO // Used for handling opaque directory markers which // may occur out of order unpackedPaths = make(map[string]struct{}) + + convertWhiteout = options.ConvertWhiteout ) + if convertWhiteout == nil { + // handle whiteouts by removing the target files + convertWhiteout = func(hdr *tar.Header, path string) (bool, error) { + base := filepath.Base(path) + dir := filepath.Dir(path) + if base == whiteoutOpaqueDir { + _, err := os.Lstat(dir) + if err != nil { + return false, err + } + err = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if err != nil { + if os.IsNotExist(err) { + err = nil // parent was deleted + } + return err + } + if path == dir { + return nil + } + if _, exists := unpackedPaths[path]; !exists { + err := os.RemoveAll(path) + return err + } + return nil + }) + return false, err + } + + if strings.HasPrefix(base, whiteoutPrefix) { + originalBase := base[len(whiteoutPrefix):] + originalPath := filepath.Join(dir, originalBase) + + return false, os.RemoveAll(originalPath) + } + + return true, nil + } + } + // Iterate through the files in the archive. for { select { @@ -182,55 +228,13 @@ func applyNaive(ctx context.Context, root string, tr *tar.Reader, options ApplyO if base == "" { parentPath = filepath.Dir(path) } - if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) { - err = mkdirAll(parentPath, 0755) - if err != nil { - return 0, err - } + if err := mkparent(ctx, parentPath, root, options.Parents); err != nil { + return 0, err } } // Naive whiteout convert function which handles whiteout files by // removing the target files. - convertWhiteout := func(hdr *tar.Header, path string) (bool, error) { - base := filepath.Base(path) - dir := filepath.Dir(path) - if base == whiteoutOpaqueDir { - _, err := os.Lstat(dir) - if err != nil { - return false, err - } - err = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { - if err != nil { - if os.IsNotExist(err) { - err = nil // parent was deleted - } - return err - } - if path == dir { - return nil - } - if _, exists := unpackedPaths[path]; !exists { - err := os.RemoveAll(path) - return err - } - return nil - }) - return false, err - } - - if strings.HasPrefix(base, whiteoutPrefix) { - originalBase := base[len(whiteoutPrefix):] - originalPath := filepath.Join(dir, originalBase) - - return false, os.RemoveAll(originalPath) - } - - return true, nil - } - if options.ConvertWhiteout != nil { - convertWhiteout = options.ConvertWhiteout - } if err := validateWhiteout(path); err != nil { return 0, err } @@ -375,6 +379,66 @@ func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header return chtimes(path, boundTime(latestTime(hdr.AccessTime, hdr.ModTime)), boundTime(hdr.ModTime)) } +func mkparent(ctx context.Context, path, root string, parents []string) error { + if dir, err := os.Lstat(path); err == nil { + if dir.IsDir() { + return nil + } + return &os.PathError{ + Op: "mkparent", + Path: path, + Err: syscall.ENOTDIR, + } + } else if !os.IsNotExist(err) { + return err + } + + i := len(path) + for i > len(root) && !os.IsPathSeparator(path[i-1]) { + i-- + } + + if i > len(root)+1 { + if err := mkparent(ctx, path[:i-1], root, parents); err != nil { + return err + } + } + + if err := mkdir(path, 0755); err != nil { + // Check that still doesn't exist + dir, err1 := os.Lstat(path) + if err1 == nil && dir.IsDir() { + return nil + } + return err + } + + for _, p := range parents { + ppath, err := fs.RootPath(p, path[len(root):]) + if err != nil { + return err + } + + dir, err := os.Lstat(ppath) + if err == nil { + if !dir.IsDir() { + // Replaced, do not copy attributes + break + } + if err := copyDirInfo(dir, path); err != nil { + return err + } + return copyUpXAttrs(path, ppath) + } else if !os.IsNotExist(err) { + return err + } + } + + log.G(ctx).Debugf("parent directory %q not found: default permissions(0755) used", path) + + return nil +} + type changeWriter struct { tw *tar.Writer source string @@ -545,6 +609,9 @@ func (cw *changeWriter) Close() error { } func (cw *changeWriter) includeParents(hdr *tar.Header) error { + if cw.addedDirs == nil { + return nil + } name := strings.TrimRight(hdr.Name, "/") fname := filepath.Join(cw.source, name) parent := filepath.Dir(name) diff --git a/archive/tar_linux_test.go b/archive/tar_linux_test.go new file mode 100644 index 000000000..2a7070155 --- /dev/null +++ b/archive/tar_linux_test.go @@ -0,0 +1,183 @@ +// +build linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package archive + +import ( + "bytes" + "context" + "fmt" + "io" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/containerd/containerd/log/logtest" + "github.com/containerd/containerd/mount" + "github.com/containerd/containerd/pkg/testutil" + "github.com/containerd/containerd/snapshots/overlay" + "github.com/containerd/continuity/fs" + "github.com/containerd/continuity/fs/fstest" + "github.com/pkg/errors" +) + +func TestOverlayApply(t *testing.T) { + testutil.RequiresRoot(t) + + base, err := ioutil.TempDir("", "test-ovl-diff-apply-") + if err != nil { + t.Fatalf("unable to create temp dir: %+v", err) + } + defer os.RemoveAll(base) + + if err := overlay.Supported(base); err != nil { + t.Skipf("skipping because overlay is not supported %v", err) + } + fstest.FSSuite(t, overlayDiffApplier{ + tmp: base, + diff: WriteDiff, + t: t, + }) +} + +func TestOverlayApplyNoParents(t *testing.T) { + testutil.RequiresRoot(t) + + base, err := ioutil.TempDir("", "test-ovl-diff-apply-") + if err != nil { + t.Fatalf("unable to create temp dir: %+v", err) + } + defer os.RemoveAll(base) + + if err := overlay.Supported(base); err != nil { + t.Skipf("skipping because overlay is not supported %v", err) + } + fstest.FSSuite(t, overlayDiffApplier{ + tmp: base, + diff: func(ctx context.Context, w io.Writer, a, b string) error { + cw := newChangeWriter(w, b) + cw.addedDirs = nil + err := fs.Changes(ctx, a, b, cw.HandleChange) + if err != nil { + return errors.Wrap(err, "failed to create diff tar stream") + } + return cw.Close() + }, + t: t, + }) +} + +type overlayDiffApplier struct { + tmp string + diff func(context.Context, io.Writer, string, string) error + t *testing.T +} + +type overlayContext struct { + merged string + lowers []string + mounted bool +} + +type contextKey struct{} + +func (d overlayDiffApplier) TestContext(ctx context.Context) (context.Context, func(), error) { + merged, err := ioutil.TempDir(d.tmp, "merged") + if err != nil { + return ctx, nil, errors.Wrap(err, "failed to make merged dir") + } + + oc := &overlayContext{ + merged: merged, + } + + ctx = logtest.WithT(ctx, d.t) + + return context.WithValue(ctx, contextKey{}, oc), func() { + if oc.mounted { + mount.Unmount(oc.merged, 0) + } + }, nil +} + +func (d overlayDiffApplier) Apply(ctx context.Context, a fstest.Applier) (string, func(), error) { + oc := ctx.Value(contextKey{}).(*overlayContext) + + applyCopy, err := ioutil.TempDir(d.tmp, "apply-copy-") + if err != nil { + return "", nil, errors.Wrap(err, "failed to create temp dir") + } + defer os.RemoveAll(applyCopy) + + base := oc.merged + if len(oc.lowers) == 1 { + base = oc.lowers[0] + } + + if err = fs.CopyDir(applyCopy, base); err != nil { + return "", nil, errors.Wrap(err, "failed to copy base") + } + + if err := a.Apply(applyCopy); err != nil { + return "", nil, errors.Wrap(err, "failed to apply changes to copy of base") + } + + buf := bytes.NewBuffer(nil) + + if err := d.diff(ctx, buf, base, applyCopy); err != nil { + return "", nil, errors.Wrap(err, "failed to create diff") + } + + if oc.mounted { + if err := mount.Unmount(oc.merged, 0); err != nil { + return "", nil, errors.Wrap(err, "failed to unmount") + } + oc.mounted = false + } + + next, err := ioutil.TempDir(d.tmp, "lower-") + if err != nil { + return "", nil, errors.Wrap(err, "failed to create temp dir") + } + + if _, err = Apply(ctx, next, buf, WithConvertWhiteout(OverlayConvertWhiteout), WithParents(oc.lowers)); err != nil { + return "", nil, errors.Wrap(err, "failed to apply tar stream") + } + + oc.lowers = append([]string{next}, oc.lowers...) + + if len(oc.lowers) == 1 { + return oc.lowers[0], nil, nil + } + + m := mount.Mount{ + Type: "overlay", + Source: "overlay", + Options: []string{ + fmt.Sprintf("lowerdir=%s", strings.Join(oc.lowers, ":")), + }, + } + + if err := m.Mount(oc.merged); err != nil { + return "", nil, errors.Wrapf(err, "failed to mount: %v", m) + } + oc.mounted = true + + return oc.merged, nil, nil +} diff --git a/archive/tar_opts.go b/archive/tar_opts.go index 970c0f658..ca419e112 100644 --- a/archive/tar_opts.go +++ b/archive/tar_opts.go @@ -16,7 +16,19 @@ package archive -import "archive/tar" +import ( + "archive/tar" + "context" +) + +// ApplyOptions provides additional options for an Apply operation +type ApplyOptions struct { + Filter Filter // Filter tar headers + ConvertWhiteout ConvertWhiteout // Convert whiteout files + Parents []string // Parent directories to handle inherited attributes without CoW + + applyFunc func(context.Context, string, *tar.Reader, ApplyOptions) (int64, error) +} // ApplyOpt allows setting mutable archive apply properties on creation type ApplyOpt func(options *ApplyOptions) error @@ -47,3 +59,16 @@ func WithConvertWhiteout(c ConvertWhiteout) ApplyOpt { return nil } } + +// WithParents provides parent directories for resolving inherited attributes +// directory from the filesystem. +// Inherited attributes are searched from first to last, making the first +// element in the list the most immediate parent directory. +// NOTE: When applying to a filesystem which supports CoW, file attributes +// should be inherited by the filesystem. +func WithParents(p []string) ApplyOpt { + return func(options *ApplyOptions) error { + options.Parents = p + return nil + } +} diff --git a/archive/tar_opts_linux.go b/archive/tar_opts_linux.go index 07ef64655..38ef9e9bc 100644 --- a/archive/tar_opts_linux.go +++ b/archive/tar_opts_linux.go @@ -27,12 +27,6 @@ import ( "golang.org/x/sys/unix" ) -// ApplyOptions provides additional options for an Apply operation -type ApplyOptions struct { - Filter Filter // Filter tar headers - ConvertWhiteout ConvertWhiteout // Convert whiteout files -} - // AufsConvertWhiteout converts whiteout files for aufs. func AufsConvertWhiteout(_ *tar.Header, _ string) (bool, error) { return true, nil diff --git a/archive/tar_opts_other.go b/archive/tar_opts_other.go deleted file mode 100644 index 03c24cf8b..000000000 --- a/archive/tar_opts_other.go +++ /dev/null @@ -1,25 +0,0 @@ -// +build !linux,!windows - -/* - Copyright The containerd Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package archive - -// ApplyOptions provides additional options for an Apply operation -type ApplyOptions struct { - Filter Filter // Filter tar headers - ConvertWhiteout ConvertWhiteout // Convert whiteout files -} diff --git a/archive/tar_opts_windows.go b/archive/tar_opts_windows.go index 10d1453a2..f472013bc 100644 --- a/archive/tar_opts_windows.go +++ b/archive/tar_opts_windows.go @@ -18,29 +18,12 @@ package archive -// ApplyOptions provides additional options for an Apply operation -type ApplyOptions struct { - ParentLayerPaths []string // Parent layer paths used for Windows layer apply - IsWindowsContainerLayer bool // True if the tar stream to be applied is a Windows Container Layer - Filter Filter // Filter tar headers - ConvertWhiteout ConvertWhiteout // Convert whiteout files -} - -// WithParentLayers adds parent layers to the apply process this is required -// for all Windows layers except the base layer. -func WithParentLayers(parentPaths []string) ApplyOpt { - return func(options *ApplyOptions) error { - options.ParentLayerPaths = parentPaths - return nil - } -} - // AsWindowsContainerLayer indicates that the tar stream to apply is that of // a Windows Container Layer. The caller must be holding SeBackupPrivilege and // SeRestorePrivilege. func AsWindowsContainerLayer() ApplyOpt { return func(options *ApplyOptions) error { - options.IsWindowsContainerLayer = true + options.applyFunc = applyWindowsLayer return nil } } diff --git a/archive/tar_unix.go b/archive/tar_unix.go index 022dd6d4f..e87218753 100644 --- a/archive/tar_unix.go +++ b/archive/tar_unix.go @@ -20,11 +20,12 @@ package archive import ( "archive/tar" - "context" "os" + "strings" "sync" "syscall" + "github.com/containerd/continuity/fs" "github.com/containerd/continuity/sysx" "github.com/opencontainers/runc/libcontainer/system" "github.com/pkg/errors" @@ -74,10 +75,6 @@ func openFile(name string, flag int, perm os.FileMode) (*os.File, error) { return f, err } -func mkdirAll(path string, perm os.FileMode) error { - return os.MkdirAll(path, perm) -} - func mkdir(path string, perm os.FileMode) error { if err := os.Mkdir(path, perm); err != nil { return err @@ -149,11 +146,71 @@ func getxattr(path, attr string) ([]byte, error) { } func setxattr(path, key, value string) error { - return sysx.LSetxattr(path, key, []byte(value), 0) + // Do not set trusted attributes + if strings.HasPrefix(key, "trusted.") { + return errors.Wrap(unix.ENOTSUP, "admin attributes from archive not supported") + } + return unix.Lsetxattr(path, key, []byte(value), 0) } -// apply applies a tar stream of an OCI style diff tar. -// See https://github.com/opencontainers/image-spec/blob/master/layer.md#applying-changesets -func apply(ctx context.Context, root string, tr *tar.Reader, options ApplyOptions) (size int64, err error) { - return applyNaive(ctx, root, tr, options) +func copyDirInfo(fi os.FileInfo, path string) error { + st := fi.Sys().(*syscall.Stat_t) + if err := os.Lchown(path, int(st.Uid), int(st.Gid)); err != nil { + if os.IsPermission(err) { + // Normally if uid/gid are the same this would be a no-op, but some + // filesystems may still return EPERM... for instance NFS does this. + // In such a case, this is not an error. + if dstStat, err2 := os.Lstat(path); err2 == nil { + st2 := dstStat.Sys().(*syscall.Stat_t) + if st.Uid == st2.Uid && st.Gid == st2.Gid { + err = nil + } + } + } + if err != nil { + return errors.Wrapf(err, "failed to chown %s", path) + } + } + + if err := os.Chmod(path, fi.Mode()); err != nil { + return errors.Wrapf(err, "failed to chmod %s", path) + } + + timespec := []unix.Timespec{unix.Timespec(fs.StatAtime(st)), unix.Timespec(fs.StatMtime(st))} + if err := unix.UtimesNanoAt(unix.AT_FDCWD, path, timespec, unix.AT_SYMLINK_NOFOLLOW); err != nil { + return errors.Wrapf(err, "failed to utime %s", path) + } + + return nil +} + +func copyUpXAttrs(dst, src string) error { + xattrKeys, err := sysx.LListxattr(src) + if err != nil { + if err == unix.ENOTSUP || err == sysx.ENODATA { + return nil + } + return errors.Wrapf(err, "failed to list xattrs on %s", src) + } + for _, xattr := range xattrKeys { + // Do not copy up trusted attributes + if strings.HasPrefix(xattr, "trusted.") { + continue + } + data, err := sysx.LGetxattr(src, xattr) + if err != nil { + if err == unix.ENOTSUP || err == sysx.ENODATA { + continue + } + return errors.Wrapf(err, "failed to get xattr %q on %s", xattr, src) + } + if err := unix.Lsetxattr(dst, xattr, data, unix.XATTR_CREATE); err != nil { + if err == unix.ENOTSUP || err == unix.ENODATA || err == unix.EEXIST { + continue + } + return errors.Wrapf(err, "failed to set xattr %q on %s", xattr, dst) + } + } + + return nil } diff --git a/archive/tar_windows.go b/archive/tar_windows.go index b97631fcc..a5c6da694 100644 --- a/archive/tar_windows.go +++ b/archive/tar_windows.go @@ -23,7 +23,6 @@ import ( "bufio" "context" "encoding/base64" - "errors" "fmt" "io" "os" @@ -36,6 +35,7 @@ import ( "github.com/Microsoft/go-winio" "github.com/Microsoft/hcsshim" "github.com/containerd/containerd/sys" + "github.com/pkg/errors" ) const ( @@ -107,10 +107,6 @@ func openFile(name string, flag int, perm os.FileMode) (*os.File, error) { return sys.OpenFileSequential(name, flag, perm) } -func mkdirAll(path string, perm os.FileMode) error { - return sys.MkdirAll(path, perm) -} - func mkdir(path string, perm os.FileMode) error { return os.Mkdir(path, perm) } @@ -153,16 +149,8 @@ func setxattr(path, key, value string) error { return errors.New("xattrs not supported on Windows") } -// apply applies a tar stream of an OCI style diff tar of a Windows layer. -// See https://github.com/opencontainers/image-spec/blob/master/layer.md#applying-changesets -func apply(ctx context.Context, root string, tr *tar.Reader, options ApplyOptions) (size int64, err error) { - if options.IsWindowsContainerLayer { - return applyWindowsLayer(ctx, root, tr, options) - } - return applyNaive(ctx, root, tr, options) -} - -// applyWindowsLayer applies a tar stream of an OCI style diff tar of a Windows layer. +// applyWindowsLayer applies a tar stream of an OCI style diff tar of a Windows +// layer using the hcsshim layer writer and backup streams. // See https://github.com/opencontainers/image-spec/blob/master/layer.md#applying-changesets func applyWindowsLayer(ctx context.Context, root string, tr *tar.Reader, options ApplyOptions) (size int64, err error) { home, id := filepath.Split(root) @@ -170,7 +158,7 @@ func applyWindowsLayer(ctx context.Context, root string, tr *tar.Reader, options HomeDir: home, } - w, err := hcsshim.NewLayerWriter(info, id, options.ParentLayerPaths) + w, err := hcsshim.NewLayerWriter(info, id, options.Parents) if err != nil { return 0, err } @@ -443,3 +431,14 @@ func writeBackupStreamFromTarFile(w io.Writer, t *tar.Reader, hdr *tar.Header) ( } } } + +func copyDirInfo(fi os.FileInfo, path string) error { + if err := os.Chmod(path, fi.Mode()); err != nil { + return errors.Wrapf(err, "failed to chmod %s", path) + } + return nil +} + +func copyUpXAttrs(dst, src string) error { + return nil +} diff --git a/diff/apply/apply_linux.go b/diff/apply/apply_linux.go index d055298b3..c36b6090c 100644 --- a/diff/apply/apply_linux.go +++ b/diff/apply/apply_linux.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/containerd/containerd/archive" + "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/mount" "github.com/pkg/errors" ) @@ -31,60 +32,97 @@ import ( func apply(ctx context.Context, mounts []mount.Mount, r io.Reader) error { switch { case len(mounts) == 1 && mounts[0].Type == "overlay": - path, err := getOverlayPath(mounts[0].Options) + path, parents, err := getOverlayPath(mounts[0].Options) if err != nil { + if errdefs.IsInvalidArgument(err) { + break + } return err } - _, err = archive.Apply(ctx, path, r, - archive.WithConvertWhiteout(archive.OverlayConvertWhiteout)) + opts := []archive.ApplyOpt{ + archive.WithConvertWhiteout(archive.OverlayConvertWhiteout), + } + if len(parents) > 0 { + opts = append(opts, archive.WithParents(parents)) + } + _, err = archive.Apply(ctx, path, r, opts...) return err case len(mounts) == 1 && mounts[0].Type == "aufs": - path, err := getAufsPath(mounts[0].Options) + path, parents, err := getAufsPath(mounts[0].Options) if err != nil { + if errdefs.IsInvalidArgument(err) { + break + } return err } - _, err = archive.Apply(ctx, path, r, - archive.WithConvertWhiteout(archive.AufsConvertWhiteout)) + opts := []archive.ApplyOpt{ + archive.WithConvertWhiteout(archive.AufsConvertWhiteout), + } + if len(parents) > 0 { + opts = append(opts, archive.WithParents(parents)) + } + _, err = archive.Apply(ctx, path, r, opts...) return err - default: - return mount.WithTempMount(ctx, mounts, func(root string) error { - _, err := archive.Apply(ctx, root, r) - return err - }) } + return mount.WithTempMount(ctx, mounts, func(root string) error { + _, err := archive.Apply(ctx, root, r) + return err + }) } -func getOverlayPath(options []string) (string, error) { +func getOverlayPath(options []string) (upper string, lower []string, err error) { const upperdirPrefix = "upperdir=" + const lowerdirPrefix = "lowerdir=" + for _, o := range options { if strings.HasPrefix(o, upperdirPrefix) { - return strings.TrimPrefix(o, upperdirPrefix), nil + upper = strings.TrimPrefix(o, upperdirPrefix) + } else if strings.HasPrefix(o, lowerdirPrefix) { + lower = strings.Split(strings.TrimPrefix(o, lowerdirPrefix), ":") } } - return "", errors.New("upperdir not found") + if upper == "" { + return "", nil, errors.Wrap(errdefs.ErrInvalidArgument, "upperdir not found") + } + + return } -func getAufsPath(options []string) (string, error) { +// getAufsPath handles options as given by the containerd aufs package only, +// formatted as "br:=rw[:=ro+wh]*" +func getAufsPath(options []string) (upper string, lower []string, err error) { const ( - sep = ":" - brPrefix1 = "br:" - brPrefix2 = "br=" - rwSuffix = "=rw" + sep = ":" + brPrefix = "br:" + rwSuffix = "=rw" + roSuffix = "=ro+wh" ) for _, o := range options { - if strings.HasPrefix(o, brPrefix1) { - o = strings.TrimPrefix(o, brPrefix1) - } else if strings.HasPrefix(o, brPrefix2) { - o = strings.TrimPrefix(o, brPrefix2) + if strings.HasPrefix(o, brPrefix) { + o = strings.TrimPrefix(o, brPrefix) } else { continue } + for _, b := range strings.Split(o, sep) { if strings.HasSuffix(b, rwSuffix) { - return strings.TrimSuffix(b, rwSuffix), nil + if upper != "" { + return "", nil, errors.Wrap(errdefs.ErrInvalidArgument, "multiple rw branch found") + } + upper = strings.TrimSuffix(b, rwSuffix) + } else if strings.HasSuffix(b, roSuffix) { + if upper == "" { + return "", nil, errors.Wrap(errdefs.ErrInvalidArgument, "rw branch be first") + } + lower = append(lower, strings.TrimSuffix(b, roSuffix)) + } else { + return "", nil, errors.Wrap(errdefs.ErrInvalidArgument, "unhandled aufs suffix") } + } - break } - return "", errors.New("rw branch not found") + if upper == "" { + return "", nil, errors.Wrap(errdefs.ErrInvalidArgument, "rw branch not found") + } + return } diff --git a/diff/apply/apply_linux_test.go b/diff/apply/apply_linux_test.go index ee7c7a5fd..999d506c8 100644 --- a/diff/apply/apply_linux_test.go +++ b/diff/apply/apply_linux_test.go @@ -23,34 +23,32 @@ import ( ) func TestGetOverlayPath(t *testing.T) { - good := []string{"upperdir=/test/upper", "lowerdir=/test/lower", "workdir=/test/work"} - path, err := getOverlayPath(good) + good := []string{"upperdir=/test/upper", "lowerdir=/test/lower1:/test/lower2", "workdir=/test/work"} + path, parents, err := getOverlayPath(good) if err != nil { t.Fatalf("Get overlay path failed: %v", err) } if path != "/test/upper" { t.Fatalf("Unexpected upperdir: %q", path) } + if len(parents) != 2 || parents[0] != "/test/lower1" || parents[1] != "/test/lower2" { + t.Fatalf("Unexpected parents: %v", parents) + } bad := []string{"lowerdir=/test/lower"} - _, err = getOverlayPath(bad) + _, _, err = getOverlayPath(bad) if err == nil { t.Fatalf("An error is expected") } } func TestGetAufsPath(t *testing.T) { - rwDir := "/test/rw" for _, test := range []struct { options []string expectErr bool }{ { - options: []string{"random:option", "br:" + rwDir + "=rw:/test/ro=ro+wh"}, - expectErr: false, - }, - { - options: []string{"random:option", "br=" + rwDir + "=rw:/test/ro=ro+wh"}, + options: []string{"random:option", "br:/test/rw=rw:/test/ro=ro+wh"}, expectErr: false, }, { @@ -62,7 +60,7 @@ func TestGetAufsPath(t *testing.T) { expectErr: true, }, } { - path, err := getAufsPath(test.options) + path, parents, err := getAufsPath(test.options) if test.expectErr { if err == nil { t.Fatalf("An error is expected") @@ -72,8 +70,12 @@ func TestGetAufsPath(t *testing.T) { if err != nil { t.Fatalf("Get aufs path failed: %v", err) } - if path != rwDir { + if path != "/test/rw" { t.Fatalf("Unexpected rw dir: %q", path) } + if len(parents) != 1 || parents[0] != "/test/ro" { + t.Fatalf("Unexpected parents: %v", parents) + } + } } diff --git a/diff/windows/windows.go b/diff/windows/windows.go index ce584dc27..af193516b 100644 --- a/diff/windows/windows.go +++ b/diff/windows/windows.go @@ -139,7 +139,7 @@ func (s windowsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts return emptyDesc, err } - if _, err := archive.Apply(ctx, layer, rc, archive.WithParentLayers(parentLayerPaths), archive.AsWindowsContainerLayer()); err != nil { + if _, err := archive.Apply(ctx, layer, rc, archive.WithParents(parentLayerPaths), archive.AsWindowsContainerLayer()); err != nil { return emptyDesc, err }