diff --git a/archive/tar.go b/archive/tar.go index 02fb0940b..90b0bdc65 100644 --- a/archive/tar.go +++ b/archive/tar.go @@ -88,8 +88,6 @@ const ( // See https://github.com/opencontainers/image-spec/blob/master/layer.md#applying-changesets func Apply(ctx context.Context, root string, r io.Reader) (int64, error) { root = filepath.Clean(root) - fn := prepareApply() - defer fn() var ( tr = tar.NewReader(r) @@ -445,13 +443,13 @@ func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header // Create directory unless it exists as a directory already. // In that case we just want to merge the two if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) { - if err := os.Mkdir(path, hdrInfo.Mode()); err != nil { + if err := mkdir(path, hdrInfo.Mode()); err != nil { return err } } case tar.TypeReg, tar.TypeRegA: - file, err := openFile(path, os.O_CREATE|os.O_WRONLY, hdrInfo.Mode()) + file, err := openFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, hdrInfo.Mode()) if err != nil { return err } diff --git a/archive/tar_unix.go b/archive/tar_unix.go index 7400d0058..44b106943 100644 --- a/archive/tar_unix.go +++ b/archive/tar_unix.go @@ -43,20 +43,28 @@ func open(p string) (*os.File, error) { } func openFile(name string, flag int, perm os.FileMode) (*os.File, error) { - return os.OpenFile(name, flag, perm) + f, err := os.OpenFile(name, flag, perm) + if err != nil { + return nil, err + } + // Call chmod to avoid permission mask + if err := os.Chmod(name, perm); err != nil { + return nil, err + } + return f, err } func mkdirAll(path string, perm os.FileMode) error { return os.MkdirAll(path, perm) } -func prepareApply() func() { - // Unset unmask before doing an apply operation, - // restore unmask when complete - oldmask := unix.Umask(0) - return func() { - unix.Umask(oldmask) +func mkdir(path string, perm os.FileMode) error { + if err := os.Mkdir(path, perm); err != nil { + return err } + // Only final created directory gets explicit permission + // call to avoid permission mask + return os.Chmod(path, perm) } func skipFile(*tar.Header) bool { diff --git a/archive/tar_windows.go b/archive/tar_windows.go index 4ccf605e0..cb3b6c59a 100644 --- a/archive/tar_windows.go +++ b/archive/tar_windows.go @@ -56,9 +56,8 @@ func mkdirAll(path string, perm os.FileMode) error { return sys.MkdirAll(path, perm) } -func prepareApply() func() { - // No umask or filesystem changes needed before apply - return func() {} +func mkdir(path string, perm os.FileMode) error { + return os.Mkdir(path, perm) } func skipFile(hdr *tar.Header) bool { diff --git a/fs/fstest/file.go b/fs/fstest/file.go index 48544c978..a5ec2f762 100644 --- a/fs/fstest/file.go +++ b/fs/fstest/file.go @@ -50,7 +50,10 @@ func RemoveAll(name string) Applier { func CreateDir(name string, perm os.FileMode) Applier { return applyFn(func(root string) error { fullPath := filepath.Join(root, name) - return os.MkdirAll(fullPath, perm) + if err := os.MkdirAll(fullPath, perm); err != nil { + return err + } + return os.Chmod(fullPath, perm) }) } @@ -75,6 +78,13 @@ func Chtime(name string, t time.Time) Applier { }) } +// Chmod returns a file applier which changes the file permission +func Chmod(name string, perm os.FileMode) Applier { + return applyFn(func(root string) error { + return os.Chmod(filepath.Join(root, name), perm) + }) +} + // Symlink returns a file applier which creates a symbolic link func Symlink(oldname, newname string) Applier { return applyFn(func(root string) error { diff --git a/fs/fstest/testsuite.go b/fs/fstest/testsuite.go index f0faa3448..844642a1d 100644 --- a/fs/fstest/testsuite.go +++ b/fs/fstest/testsuite.go @@ -17,8 +17,7 @@ type TestApplier interface { func FSSuite(t *testing.T, a TestApplier) { t.Run("Basic", makeTest(t, a, basicTest)) t.Run("Deletion", makeTest(t, a, deletionTest)) - // TODO: Add hard section, run if command line arg or function arg set to true - // Hard tests + t.Run("Update", makeTest(t, a, updateTest)) t.Run("HardlinkUnmodified", makeTest(t, a, hardlinkUnmodified)) t.Run("HardlinkBeforeUnmodified", makeTest(t, a, hardlinkBeforeUnmodified)) t.Run("HardlinkBeforeModified", makeTest(t, a, hardlinkBeforeModified)) @@ -125,6 +124,24 @@ var ( ), } + // updateTest covers file updates for content and permission + updateTest = []Applier{ + Apply( + CreateDir("/d1", 0755), + CreateDir("/d2", 0700), + CreateFile("/d1/f1", []byte("something..."), 0644), + CreateFile("/d1/f2", []byte("else..."), 0644), + CreateFile("/d1/f3", []byte("entirely..."), 0644), + ), + Apply( + CreateFile("/d1/f1", []byte("file content of a different length"), 0664), + Remove("/d1/f3"), + CreateFile("/d1/f3", []byte("updated content"), 0664), + Chmod("/d1/f2", 0766), + Chmod("/d2", 0777), + ), + } + hardlinkUnmodified = []Applier{ baseApplier, Apply(