From f3654041f4d197c97a2d0ce6c361dbdee3955359 Mon Sep 17 00:00:00 2001 From: Amit Barve Date: Thu, 4 Nov 2021 13:00:17 -0700 Subject: [PATCH 1/4] Handling of out-of-order whiteout files during tar expansion When extracting a container image layer tar, some files can show in an out of order fashion (i.e the file shows up first before its parent directory shows up). We already handle this by creating these parent directories if they don't already exist. However, that handling didn't apply to whiteout files. This commit fixes that. Signed-off-by: Amit Barve --- ext4/internal/compactext4/compact.go | 22 ++++++++++++++++++---- ext4/tar2ext4/tar2ext4.go | 15 ++++++++++----- ext4/tar2ext4/tar2ext4_test.go | 3 ++- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/ext4/internal/compactext4/compact.go b/ext4/internal/compactext4/compact.go index e35536b9fa..c11be83e35 100644 --- a/ext4/internal/compactext4/compact.go +++ b/ext4/internal/compactext4/compact.go @@ -414,6 +414,12 @@ func (w *Writer) makeInode(f *File, node *inode) (*inode, error) { node.Devmajor = f.Devmajor node.Devminor = f.Devminor node.Data = nil + + // copy over existing xattrs first, we need to merge existing xattrs and the passed xattrs. + existingXattrs := make(map[string][]byte) + if len(node.XattrInline) > 0 { + getXattrs(node.XattrInline[4:], existingXattrs, 0) + } node.XattrInline = nil var xstate xattrState @@ -452,6 +458,13 @@ func (w *Writer) makeInode(f *File, node *inode) (*inode, error) { return nil, fmt.Errorf("invalid mode %o", mode) } + // merge xattrs but prefer currently passed over existing + for name, data := range existingXattrs { + if _, ok := f.Xattrs[name]; !ok { + f.Xattrs[name] = data + } + } + // Accumulate the extended attributes. if len(f.Xattrs) != 0 { // Sort the xattrs to avoid non-determinism in map iteration. @@ -514,15 +527,16 @@ func (w *Writer) lookup(name string, mustExist bool) (*inode, *inode, string, er return dir, child, childname, nil } -// CreateWithParents adds a file to the file system creating the parent directories in the path if -// they don't exist (like `mkdir -p`). These non existing parent directories are created +// MakeParents ensures that all the parent directories in the path specified by `name` exists. If +// they don't exist it creates them (like `mkdir -p`). These non existing parent directories are created // with the same permissions as that of it's parent directory. It is expected that the a // call to make these parent directories will be made at a later point with the correct // permissions, at that time the permissions of these directories will be updated. -func (w *Writer) CreateWithParents(name string, f *File) error { +func (w *Writer) MakeParents(name string) error { if err := w.finishInode(); err != nil { return err } + // go through the directories in the path one by one and create the // parent directories if they don't exist. cleanname := path.Clean("/" + name)[1:] @@ -553,7 +567,7 @@ func (w *Writer) CreateWithParents(name string, f *File) error { } root = root.Children[dirname] } - return w.Create(name, f) + return nil } // Create adds a file to the file system. diff --git a/ext4/tar2ext4/tar2ext4.go b/ext4/tar2ext4/tar2ext4.go index 5fcc3ba78a..d9ba3131c7 100644 --- a/ext4/tar2ext4/tar2ext4.go +++ b/ext4/tar2ext4/tar2ext4.go @@ -5,7 +5,6 @@ import ( "bufio" "bytes" "encoding/binary" - "github.com/pkg/errors" "io" "io/ioutil" "os" @@ -13,6 +12,8 @@ import ( "strings" "unsafe" + "github.com/pkg/errors" + "github.com/Microsoft/hcsshim/ext4/dmverity" "github.com/Microsoft/hcsshim/ext4/internal/compactext4" "github.com/Microsoft/hcsshim/ext4/internal/format" @@ -86,6 +87,10 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { return err } + if err = fs.MakeParents(hdr.Name); err != nil { + return errors.Wrapf(err, "failed to ensure parent directories for %s", hdr.Name) + } + if p.convertWhiteout { dir, name := path.Split(hdr.Name) if strings.HasPrefix(name, whiteoutPrefix) { @@ -93,12 +98,12 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { // Update the directory with the appropriate xattr. f, err := fs.Stat(dir) if err != nil { - return err + return errors.Wrapf(err, "failed to stat parent directory of whiteout %s", hdr.Name) } f.Xattrs["trusted.overlay.opaque"] = []byte("y") err = fs.Create(dir, f) if err != nil { - return err + return errors.Wrapf(err, "failed to create opaque dir %s", hdr.Name) } } else { // Create an overlay-style whiteout. @@ -109,7 +114,7 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { } err = fs.Create(path.Join(dir, name[len(whiteoutPrefix):]), f) if err != nil { - return err + return errors.Wrapf(err, "failed to create whiteout file for %s", hdr.Name) } } @@ -161,7 +166,7 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { } f.Mode &= ^compactext4.TypeMask f.Mode |= typ - err = fs.CreateWithParents(hdr.Name, f) + err = fs.Create(hdr.Name, f) if err != nil { return err } diff --git a/ext4/tar2ext4/tar2ext4_test.go b/ext4/tar2ext4/tar2ext4_test.go index b5117b0419..bc6f327335 100644 --- a/ext4/tar2ext4/tar2ext4_test.go +++ b/ext4/tar2ext4/tar2ext4_test.go @@ -28,10 +28,11 @@ func Test_UnorderedTarExpansion(t *testing.T) { var files = []struct { path, body string }{ - {"foo/bar.txt", "inside bar.txt"}, + {"foo/.wh.bar.txt", "inside bar.txt"}, {"data/", ""}, {"root.txt", "inside root.txt"}, {"foo/", ""}, + {"A/.wh..wh..opq", ""}, {"A/B/b.txt", "inside b.txt"}, {"A/a.txt", "inside a.txt"}, {"A/", ""}, From a83bdd55bbdef922b121588d8ae02b90b60221cb Mon Sep 17 00:00:00 2001 From: Amit Barve Date: Thu, 4 Nov 2021 13:24:31 -0700 Subject: [PATCH 2/4] Run go mod vendor and go mod tidy Signed-off-by: Amit Barve --- .../ext4/internal/compactext4/compact.go | 22 +++++++++++++++---- .../hcsshim/ext4/tar2ext4/tar2ext4.go | 15 ++++++++----- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go b/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go index e35536b9fa..c11be83e35 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go +++ b/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go @@ -414,6 +414,12 @@ func (w *Writer) makeInode(f *File, node *inode) (*inode, error) { node.Devmajor = f.Devmajor node.Devminor = f.Devminor node.Data = nil + + // copy over existing xattrs first, we need to merge existing xattrs and the passed xattrs. + existingXattrs := make(map[string][]byte) + if len(node.XattrInline) > 0 { + getXattrs(node.XattrInline[4:], existingXattrs, 0) + } node.XattrInline = nil var xstate xattrState @@ -452,6 +458,13 @@ func (w *Writer) makeInode(f *File, node *inode) (*inode, error) { return nil, fmt.Errorf("invalid mode %o", mode) } + // merge xattrs but prefer currently passed over existing + for name, data := range existingXattrs { + if _, ok := f.Xattrs[name]; !ok { + f.Xattrs[name] = data + } + } + // Accumulate the extended attributes. if len(f.Xattrs) != 0 { // Sort the xattrs to avoid non-determinism in map iteration. @@ -514,15 +527,16 @@ func (w *Writer) lookup(name string, mustExist bool) (*inode, *inode, string, er return dir, child, childname, nil } -// CreateWithParents adds a file to the file system creating the parent directories in the path if -// they don't exist (like `mkdir -p`). These non existing parent directories are created +// MakeParents ensures that all the parent directories in the path specified by `name` exists. If +// they don't exist it creates them (like `mkdir -p`). These non existing parent directories are created // with the same permissions as that of it's parent directory. It is expected that the a // call to make these parent directories will be made at a later point with the correct // permissions, at that time the permissions of these directories will be updated. -func (w *Writer) CreateWithParents(name string, f *File) error { +func (w *Writer) MakeParents(name string) error { if err := w.finishInode(); err != nil { return err } + // go through the directories in the path one by one and create the // parent directories if they don't exist. cleanname := path.Clean("/" + name)[1:] @@ -553,7 +567,7 @@ func (w *Writer) CreateWithParents(name string, f *File) error { } root = root.Children[dirname] } - return w.Create(name, f) + return nil } // Create adds a file to the file system. diff --git a/test/vendor/github.com/Microsoft/hcsshim/ext4/tar2ext4/tar2ext4.go b/test/vendor/github.com/Microsoft/hcsshim/ext4/tar2ext4/tar2ext4.go index 5fcc3ba78a..d9ba3131c7 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/ext4/tar2ext4/tar2ext4.go +++ b/test/vendor/github.com/Microsoft/hcsshim/ext4/tar2ext4/tar2ext4.go @@ -5,7 +5,6 @@ import ( "bufio" "bytes" "encoding/binary" - "github.com/pkg/errors" "io" "io/ioutil" "os" @@ -13,6 +12,8 @@ import ( "strings" "unsafe" + "github.com/pkg/errors" + "github.com/Microsoft/hcsshim/ext4/dmverity" "github.com/Microsoft/hcsshim/ext4/internal/compactext4" "github.com/Microsoft/hcsshim/ext4/internal/format" @@ -86,6 +87,10 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { return err } + if err = fs.MakeParents(hdr.Name); err != nil { + return errors.Wrapf(err, "failed to ensure parent directories for %s", hdr.Name) + } + if p.convertWhiteout { dir, name := path.Split(hdr.Name) if strings.HasPrefix(name, whiteoutPrefix) { @@ -93,12 +98,12 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { // Update the directory with the appropriate xattr. f, err := fs.Stat(dir) if err != nil { - return err + return errors.Wrapf(err, "failed to stat parent directory of whiteout %s", hdr.Name) } f.Xattrs["trusted.overlay.opaque"] = []byte("y") err = fs.Create(dir, f) if err != nil { - return err + return errors.Wrapf(err, "failed to create opaque dir %s", hdr.Name) } } else { // Create an overlay-style whiteout. @@ -109,7 +114,7 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { } err = fs.Create(path.Join(dir, name[len(whiteoutPrefix):]), f) if err != nil { - return err + return errors.Wrapf(err, "failed to create whiteout file for %s", hdr.Name) } } @@ -161,7 +166,7 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { } f.Mode &= ^compactext4.TypeMask f.Mode |= typ - err = fs.CreateWithParents(hdr.Name, f) + err = fs.Create(hdr.Name, f) if err != nil { return err } From 3549d383339d1e134729adafa97dbf0db42f8975 Mon Sep 17 00:00:00 2001 From: Amit Barve Date: Thu, 4 Nov 2021 13:37:54 -0700 Subject: [PATCH 3/4] Init empty map. Signed-off-by: Amit Barve --- ext4/internal/compactext4/compact.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ext4/internal/compactext4/compact.go b/ext4/internal/compactext4/compact.go index c11be83e35..e64d86eba4 100644 --- a/ext4/internal/compactext4/compact.go +++ b/ext4/internal/compactext4/compact.go @@ -414,6 +414,9 @@ func (w *Writer) makeInode(f *File, node *inode) (*inode, error) { node.Devmajor = f.Devmajor node.Devminor = f.Devminor node.Data = nil + if f.Xattrs == nil { + f.Xattrs = make(map[string][]byte) + } // copy over existing xattrs first, we need to merge existing xattrs and the passed xattrs. existingXattrs := make(map[string][]byte) From cd51eb008abd68e94c0faa6f00b9c0804cff6321 Mon Sep 17 00:00:00 2001 From: Amit Barve Date: Thu, 4 Nov 2021 13:56:58 -0700 Subject: [PATCH 4/4] Go mod vendor Signed-off-by: Amit Barve --- .../Microsoft/hcsshim/ext4/internal/compactext4/compact.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go b/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go index c11be83e35..e64d86eba4 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go +++ b/test/vendor/github.com/Microsoft/hcsshim/ext4/internal/compactext4/compact.go @@ -414,6 +414,9 @@ func (w *Writer) makeInode(f *File, node *inode) (*inode, error) { node.Devmajor = f.Devmajor node.Devminor = f.Devminor node.Data = nil + if f.Xattrs == nil { + f.Xattrs = make(map[string][]byte) + } // copy over existing xattrs first, we need to merge existing xattrs and the passed xattrs. existingXattrs := make(map[string][]byte)