diff --git a/pkg/archive/archive_linux_test.go b/pkg/archive/archive_linux_test.go index efff0dddcf..640929298f 100644 --- a/pkg/archive/archive_linux_test.go +++ b/pkg/archive/archive_linux_test.go @@ -86,9 +86,8 @@ func checkFileMode(t *testing.T, path string, perm os.FileMode) { } func TestOverlayTarUntar(t *testing.T) { - oldmask, err := system.Umask(0) - assert.NilError(t, err) - defer system.Umask(oldmask) + restore := overrideUmask(0) + defer restore() src, err := os.MkdirTemp("", "docker-test-overlay-tar-src") assert.NilError(t, err) @@ -125,9 +124,8 @@ func TestOverlayTarUntar(t *testing.T) { } func TestOverlayTarAUFSUntar(t *testing.T) { - oldmask, err := system.Umask(0) - assert.NilError(t, err) - defer system.Umask(oldmask) + restore := overrideUmask(0) + defer restore() src, err := os.MkdirTemp("", "docker-test-overlay-tar-src") assert.NilError(t, err) diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go index 62409d827e..8eeccb608b 100644 --- a/pkg/archive/diff.go +++ b/pkg/archive/diff.go @@ -229,13 +229,8 @@ func applyLayerHandler(dest string, layer io.Reader, options *TarOptions, decomp dest = filepath.Clean(dest) // We need to be able to set any perms - if runtime.GOOS != "windows" { - oldmask, err := system.Umask(0) - if err != nil { - return 0, err - } - defer system.Umask(oldmask) - } + restore := overrideUmask(0) + defer restore() if decompress { decompLayer, err := DecompressStream(layer) diff --git a/pkg/archive/diff_unix.go b/pkg/archive/diff_unix.go new file mode 100644 index 0000000000..d7f806445e --- /dev/null +++ b/pkg/archive/diff_unix.go @@ -0,0 +1,22 @@ +//go:build !windows +// +build !windows + +package archive + +import "golang.org/x/sys/unix" + +// overrideUmask sets current process's file mode creation mask to newmask +// and returns a function to restore it. +// +// WARNING for readers stumbling upon this code. Changing umask in a multi- +// threaded environment isn't safe. Don't use this without understanding the +// risks, and don't export this function for others to use (we shouldn't even +// be using this ourself). +// +// FIXME(thaJeztah): we should get rid of these hacks if possible. +func overrideUmask(newMask int) func() { + oldMask := unix.Umask(newMask) + return func() { + unix.Umask(oldMask) + } +} diff --git a/pkg/archive/diff_windows.go b/pkg/archive/diff_windows.go new file mode 100644 index 0000000000..d28f5b2dfd --- /dev/null +++ b/pkg/archive/diff_windows.go @@ -0,0 +1,6 @@ +package archive + +// overrideUmask is a no-op on windows. +func overrideUmask(newmask int) func() { + return func() {} +} diff --git a/pkg/chrootarchive/diff_unix.go b/pkg/chrootarchive/diff_unix.go index e1bf74d1d5..fcc02f675e 100644 --- a/pkg/chrootarchive/diff_unix.go +++ b/pkg/chrootarchive/diff_unix.go @@ -16,7 +16,7 @@ import ( "github.com/containerd/containerd/pkg/userns" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/reexec" - "github.com/docker/docker/pkg/system" + "golang.org/x/sys/unix" ) type applyLayerResponse struct { @@ -42,11 +42,8 @@ func applyLayer() { } // We need to be able to set any perms - oldmask, err := system.Umask(0) - defer system.Umask(oldmask) - if err != nil { - fatal(err) - } + oldmask := unix.Umask(0) + defer unix.Umask(oldmask) if err := json.Unmarshal([]byte(os.Getenv("OPT")), &options); err != nil { fatal(err) diff --git a/pkg/directory/directory.go b/pkg/directory/directory.go index f0c45303cd..7b8d74a356 100644 --- a/pkg/directory/directory.go +++ b/pkg/directory/directory.go @@ -1,24 +1,8 @@ package directory // import "github.com/docker/docker/pkg/directory" -import ( - "os" - "path/filepath" -) +import "context" -// MoveToSubdir moves all contents of a directory to a subdirectory underneath the original path -func MoveToSubdir(oldpath, subdir string) error { - infos, err := os.ReadDir(oldpath) - if err != nil { - return err - } - for _, info := range infos { - if info.Name() != subdir { - oldName := filepath.Join(oldpath, info.Name()) - newName := filepath.Join(oldpath, subdir, info.Name()) - if err := os.Rename(oldName, newName); err != nil { - return err - } - } - } - return nil +// Size walks a directory tree and returns its total size in bytes. +func Size(ctx context.Context, dir string) (int64, error) { + return calcSize(ctx, dir) } diff --git a/pkg/directory/directory_test.go b/pkg/directory/directory_test.go index ec9c97e699..3bfa1e0fd7 100644 --- a/pkg/directory/directory_test.go +++ b/pkg/directory/directory_test.go @@ -3,9 +3,6 @@ package directory // import "github.com/docker/docker/pkg/directory" import ( "context" "os" - "path/filepath" - "reflect" - "sort" "testing" ) @@ -144,51 +141,6 @@ func TestSizeFileAndNestedDirectoryNonempty(t *testing.T) { } } -// Test migration of directory to a subdir underneath itself -func TestMoveToSubdir(t *testing.T) { - var outerDir, subDir string - var err error - - if outerDir, err = os.MkdirTemp(os.TempDir(), "TestMoveToSubdir"); err != nil { - t.Fatalf("failed to create directory: %v", err) - } - - if subDir, err = os.MkdirTemp(outerDir, "testSub"); err != nil { - t.Fatalf("failed to create subdirectory: %v", err) - } - - // write 4 temp files in the outer dir to get moved - filesList := []string{"a", "b", "c", "d"} - for _, fName := range filesList { - if file, err := os.Create(filepath.Join(outerDir, fName)); err != nil { - t.Fatalf("couldn't create temp file %q: %v", fName, err) - } else { - file.WriteString(fName) - file.Close() - } - } - - if err = MoveToSubdir(outerDir, filepath.Base(subDir)); err != nil { - t.Fatalf("Error during migration of content to subdirectory: %v", err) - } - // validate that the files were moved to the subdirectory - infos, err := os.ReadDir(subDir) - if err != nil { - t.Fatal(err) - } - if len(infos) != 4 { - t.Fatalf("Should be four files in the subdir after the migration: actual length: %d", len(infos)) - } - var results []string - for _, info := range infos { - results = append(results, info.Name()) - } - sort.Strings(results) - if !reflect.DeepEqual(filesList, results) { - t.Fatalf("Results after migration do not equal list of files: expected: %v, got: %v", filesList, results) - } -} - // Test a non-existing directory func TestSizeNonExistingDirectory(t *testing.T) { if _, err := Size(context.Background(), "/thisdirectoryshouldnotexist/TestSizeNonExistingDirectory"); err == nil { diff --git a/pkg/directory/directory_unix.go b/pkg/directory/directory_unix.go index eeedff18a4..4592205f63 100644 --- a/pkg/directory/directory_unix.go +++ b/pkg/directory/directory_unix.go @@ -10,14 +10,15 @@ import ( "syscall" ) -// Size walks a directory tree and returns its total size in bytes. -func Size(ctx context.Context, dir string) (size int64, err error) { +// calcSize walks a directory tree and returns its total size in bytes. +func calcSize(ctx context.Context, dir string) (int64, error) { + var size int64 data := make(map[uint64]struct{}) - err = filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { + err := filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { if err != nil { - // if dir does not exist, Size() returns the error. // if dir/x disappeared while walking, Size() ignores dir/x. - if os.IsNotExist(err) && d != dir { + // if dir does not exist, Size() returns the error. + if d != dir && os.IsNotExist(err) { return nil } return err @@ -40,16 +41,16 @@ func Size(ctx context.Context, dir string) (size int64, err error) { // Check inode to handle hard links correctly inode := fileInfo.Sys().(*syscall.Stat_t).Ino - // inode is not a uint64 on all platforms. Cast it to avoid issues. - if _, exists := data[inode]; exists { + //nolint:unconvert // inode is not an uint64 on all platforms. + if _, exists := data[uint64(inode)]; exists { return nil } - // inode is not a uint64 on all platforms. Cast it to avoid issues. - data[inode] = struct{}{} + + data[uint64(inode)] = struct{}{} //nolint:unconvert // inode is not an uint64 on all platforms. size += s return nil }) - return + return size, err } diff --git a/pkg/directory/directory_windows.go b/pkg/directory/directory_windows.go index f07f241880..fc72ff62e6 100644 --- a/pkg/directory/directory_windows.go +++ b/pkg/directory/directory_windows.go @@ -6,13 +6,14 @@ import ( "path/filepath" ) -// Size walks a directory tree and returns its total size in bytes. -func Size(ctx context.Context, dir string) (size int64, err error) { - err = filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { +// calcSize walks a directory tree and returns its total calcSize in bytes. +func calcSize(ctx context.Context, dir string) (int64, error) { + var size int64 + err := filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { if err != nil { - // if dir does not exist, Size() returns the error. // if dir/x disappeared while walking, Size() ignores dir/x. - if os.IsNotExist(err) && d != dir { + // if dir does not exist, Size() returns the error. + if d != dir && os.IsNotExist(err) { return nil } return err @@ -38,5 +39,5 @@ func Size(ctx context.Context, dir string) (size int64, err error) { return nil }) - return + return size, err } diff --git a/pkg/idtools/idtools_unix.go b/pkg/idtools/idtools_unix.go index aed1a41480..03846b0307 100644 --- a/pkg/idtools/idtools_unix.go +++ b/pkg/idtools/idtools_unix.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" "strconv" "sync" @@ -199,7 +200,7 @@ func callGetent(database, key string) (io.Reader, error) { } out, err := execCmd(getentCmd, database, key) if err != nil { - exitCode, errC := system.GetExitCode(err) + exitCode, errC := getExitCode(err) if errC != nil { return nil, err } @@ -217,6 +218,18 @@ func callGetent(database, key string) (io.Reader, error) { return bytes.NewReader(out), nil } +// getExitCode returns the ExitStatus of the specified error if its type is +// exec.ExitError, returns 0 and an error otherwise. +func getExitCode(err error) (int, error) { + exitCode := 0 + if exiterr, ok := err.(*exec.ExitError); ok { + if procExit, ok := exiterr.Sys().(syscall.WaitStatus); ok { + return procExit.ExitStatus(), nil + } + } + return exitCode, fmt.Errorf("failed to get exit code") +} + // setPermissions performs a chown/chmod only if the uid/gid don't match what's requested // Normally a Chown is a no-op if uid/gid match, but in some cases this can still cause an error, e.g. if the // dir is on an NFS share, so don't call chown unless we absolutely must. diff --git a/pkg/system/exitcode.go b/pkg/system/exitcode.go deleted file mode 100644 index 4ba8fe35bf..0000000000 --- a/pkg/system/exitcode.go +++ /dev/null @@ -1,19 +0,0 @@ -package system // import "github.com/docker/docker/pkg/system" - -import ( - "fmt" - "os/exec" - "syscall" -) - -// GetExitCode returns the ExitStatus of the specified error if its type is -// exec.ExitError, returns 0 and an error otherwise. -func GetExitCode(err error) (int, error) { - exitCode := 0 - if exiterr, ok := err.(*exec.ExitError); ok { - if procExit, ok := exiterr.Sys().(syscall.WaitStatus); ok { - return procExit.ExitStatus(), nil - } - } - return exitCode, fmt.Errorf("failed to get exit code") -} diff --git a/pkg/system/stat_windows.go b/pkg/system/stat_windows.go index b2456cb887..0ff3af2fa1 100644 --- a/pkg/system/stat_windows.go +++ b/pkg/system/stat_windows.go @@ -20,12 +20,12 @@ func (s StatT) Size() int64 { // Mode returns file's permission mode. func (s StatT) Mode() os.FileMode { - return os.FileMode(s.mode) + return s.mode } // Mtim returns file's last modification time. func (s StatT) Mtim() time.Time { - return time.Time(s.mtim) + return s.mtim } // Stat takes a path to a file and returns diff --git a/pkg/system/umask.go b/pkg/system/umask.go deleted file mode 100644 index d4a15cbedc..0000000000 --- a/pkg/system/umask.go +++ /dev/null @@ -1,14 +0,0 @@ -//go:build !windows -// +build !windows - -package system // import "github.com/docker/docker/pkg/system" - -import ( - "golang.org/x/sys/unix" -) - -// Umask sets current process's file mode creation mask to newmask -// and returns oldmask. -func Umask(newmask int) (oldmask int, err error) { - return unix.Umask(newmask), nil -} diff --git a/pkg/system/umask_windows.go b/pkg/system/umask_windows.go deleted file mode 100644 index fc62388c38..0000000000 --- a/pkg/system/umask_windows.go +++ /dev/null @@ -1,7 +0,0 @@ -package system // import "github.com/docker/docker/pkg/system" - -// Umask is not supported on the windows platform. -func Umask(newmask int) (oldmask int, err error) { - // should not be called on cli code path - return 0, ErrNotSupportedPlatform -}