Merge pull request moby#49162 from thaJeztah/pkg_system_volume_uuid

pkg/system: deprecate MkdirAll and remove custom volume GUID handling

builder/dockerfile/copy.go

@@ -502,11 +502,7 @@ func copyDirectory(archiver *archive.Archiver, source, dest string, identity *id
 
 func copyFile(archiver *archive.Archiver, source, dest string, identity *idtools.Identity) error {
 	if identity == nil {
-		// Use system.MkdirAll here, which is a custom version of os.MkdirAll
-		// modified for use on Windows to handle volume GUID paths. These paths
-		// are of the form \\?\Volume{<GUID>}\<path>. An example would be:
-		// \\?\Volume{dae8d3ac-b9a1-11e9-88eb-e8554b2ba1db}\bin\busybox.exe
-		if err := system.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
+		if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
 			return err
 		}
 	} else {

cmd/dockerd/daemon.go

@@ -54,7 +54,6 @@ import (
 	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/rootless"
 	"github.com/docker/docker/pkg/sysinfo"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/plugin"
 	"github.com/docker/docker/runconfig"
 	"github.com/docker/go-connections/tlsconfig"
@@ -143,14 +142,14 @@ func (cli *daemonCLI) start(ctx context.Context) (err error) {
 		return err
 	}
 
-	if err := system.MkdirAll(cli.Config.ExecRoot, 0o700); err != nil {
+	if err := os.MkdirAll(cli.Config.ExecRoot, 0o700); err != nil {
 		return err
 	}
 
 	potentiallyUnderRuntimeDir := []string{cli.Config.ExecRoot}
 
 	if cli.Pidfile != "" {
-		if err = system.MkdirAll(filepath.Dir(cli.Pidfile), 0o755); err != nil {
+		if err = os.MkdirAll(filepath.Dir(cli.Pidfile), 0o755); err != nil {
 			return errors.Wrap(err, "failed to create pidfile directory")
 		}
 		if err = pidfile.Write(cli.Pidfile, os.Getpid()); err != nil {

container/container_windows.go

@@ -9,7 +9,6 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/events"
 	swarmtypes "github.com/docker/docker/api/types/swarm"
-	"github.com/docker/docker/pkg/system"
 )
 
 const (
@@ -46,7 +45,7 @@ func (container *Container) CreateSecretSymlinks() error {
 		if err != nil {
 			return err
 		}
-		if err := system.MkdirAll(filepath.Dir(resolvedPath), 0); err != nil {
+		if err := os.MkdirAll(filepath.Dir(resolvedPath), 0); err != nil {
 			return err
 		}
 		if err := os.Symlink(filepath.Join(containerInternalSecretMountPath, r.SecretID), resolvedPath); err != nil {
@@ -96,7 +95,7 @@ func (container *Container) CreateConfigSymlinks() error {
 		if err != nil {
 			return err
 		}
-		if err := system.MkdirAll(filepath.Dir(resolvedPath), 0); err != nil {
+		if err := os.MkdirAll(filepath.Dir(resolvedPath), 0); err != nil {
 			return err
 		}
 		if err := os.Symlink(filepath.Join(containerInternalConfigsDirPath, configRef.ConfigID), resolvedPath); err != nil {

daemon/daemon.go

@@ -66,7 +66,6 @@ import (
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/sysinfo"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/plugin"
 	pluginexec "github.com/docker/docker/plugin/executor/containerd"
 	refstore "github.com/docker/docker/reference"
@@ -810,7 +809,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
 		return nil, fmt.Errorf("Unable to get the full path to the TempDir (%s): %s", tmp, err)
 	}
 	if isWindows {
-		if err := system.MkdirAll(realTmp, 0); err != nil {
+		if err := os.MkdirAll(realTmp, 0); err != nil {
 			return nil, fmt.Errorf("Unable to create the TempDir (%s): %s", realTmp, err)
 		}
 		os.Setenv("TEMP", realTmp)

daemon/graphdriver/windows/windows.go

@@ -31,7 +31,6 @@ import (
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/longpath"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/go-units"
 	"github.com/moby/sys/reexec"
 	"github.com/pkg/errors"
@@ -103,7 +102,7 @@ func InitFilter(home string, options []string, _ idtools.IdentityMapping) (graph
 
 	// Setting file-mode is a no-op on Windows, so passing "0" to make it more
 	// transparent that the filemode passed has no effect.
-	if err = system.MkdirAll(home, 0); err != nil {
+	if err = os.MkdirAll(home, 0); err != nil {
 		return nil, errors.Wrapf(err, "windowsfilter failed to create '%s'", home)
 	}
 

daemon/runtime_unix.go

@@ -22,7 +22,6 @@ import (
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/libcontainerd/shimopts"
 	"github.com/docker/docker/pkg/ioutils"
-	"github.com/docker/docker/pkg/system"
 	"github.com/opencontainers/runtime-spec/specs-go/features"
 	"github.com/pkg/errors"
 )
@@ -94,7 +93,7 @@ func initRuntimesDir(cfg *config.Config) error {
 	if err := os.RemoveAll(runtimeDir); err != nil {
 		return err
 	}
-	return system.MkdirAll(runtimeDir, 0o700)
+	return os.MkdirAll(runtimeDir, 0o700)
 }
 
 func setupRuntimes(cfg *config.Config) (runtimes, error) {

libcontainerd/supervisor/remote_daemon.go

@@ -16,7 +16,6 @@ import (
 	"github.com/containerd/log"
 	"github.com/docker/docker/pkg/pidfile"
 	"github.com/docker/docker/pkg/process"
-	"github.com/docker/docker/pkg/system"
 	"github.com/moby/buildkit/util/grpcerrors"
 	"github.com/pelletier/go-toml"
 	"github.com/pkg/errors"
@@ -93,7 +92,7 @@ func Start(ctx context.Context, rootDir, stateDir string, opts ...DaemonOpt) (Da
 		}
 	}
 
-	if err := system.MkdirAll(stateDir, 0o700); err != nil {
+	if err := os.MkdirAll(stateDir, 0o700); err != nil {
 		return nil, err
 	}
 

pkg/idtools/idtools_windows.go

@@ -2,8 +2,6 @@ package idtools // import "github.com/docker/docker/pkg/idtools"
 
 import (
 	"os"
-
-	"github.com/docker/docker/pkg/system"
 )
 
 const (
@@ -15,10 +13,10 @@ const (
 	ContainerUserSidString          = "S-1-5-93-2-2"
 )
 
-// This is currently a wrapper around MkdirAll, however, since currently
+// This is currently a wrapper around [os.MkdirAll] since currently
 // permissions aren't set through this path, the identity isn't utilized.
 // Ownership is handled elsewhere, but in the future could be support here
 // too.
 func mkdirAs(path string, _ os.FileMode, _ Identity, _, _ bool) error {
-	return system.MkdirAll(path, 0)
+	return os.MkdirAll(path, 0)
 }

pkg/system/filesys.go

@@ -17,3 +17,11 @@ import (
 func IsAbs(path string) bool {
 	return filepath.IsAbs(path) || strings.HasPrefix(path, string(os.PathSeparator))
 }
+
+// MkdirAll creates a directory named path along with any necessary parents,
+// with permission specified by attribute perm for all dir created.
+//
+// Deprecated: [os.MkdirAll] now natively supports Windows GUID volume paths, and should be used instead. This alias will be removed in the next release.
+func MkdirAll(path string, perm os.FileMode) error {
+	return os.MkdirAll(path, perm)
+}

pkg/system/filesys_unix.go

@@ -5,12 +5,6 @@ package system // import "github.com/docker/docker/pkg/system"
 import "os"
 
 // MkdirAllWithACL is a wrapper for os.MkdirAll on unix systems.
-func MkdirAllWithACL(path string, perm os.FileMode, sddl string) error {
-	return os.MkdirAll(path, perm)
-}
-
-// MkdirAll creates a directory named path along with any necessary parents,
-// with permission specified by attribute perm for all dir created.
-func MkdirAll(path string, perm os.FileMode) error {
+func MkdirAllWithACL(path string, perm os.FileMode, _ string) error {
 	return os.MkdirAll(path, perm)
 }

pkg/system/filesys_windows.go

@@ -2,7 +2,7 @@ package system // import "github.com/docker/docker/pkg/system"
 
 import (
 	"os"
-	"regexp"
+	"path/filepath"
 	"syscall"
 	"unsafe"
 
@@ -12,11 +12,6 @@ import (
 // SddlAdministratorsLocalSystem is local administrators plus NT AUTHORITY\System.
 const SddlAdministratorsLocalSystem = "D:P(A;OICI;GA;;;BA)(A;OICI;GA;;;SY)"
 
-// volumePath is a regular expression to check if a path is a Windows
-// volume path (e.g., "\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}"
-// or "\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}\").
-var volumePath = regexp.MustCompile(`^\\\\\?\\Volume{[a-z0-9-]+}\\?$`)
-
 // MkdirAllWithACL is a custom version of os.MkdirAll modified for use on Windows
 // so that it is both volume path aware, and can create a directory with
 // an appropriate SDDL defined ACL.
@@ -25,26 +20,23 @@ func MkdirAllWithACL(path string, _ os.FileMode, sddl string) error {
 	if err != nil {
 		return &os.PathError{Op: "mkdirall", Path: path, Err: err}
 	}
-	return mkdirall(path, sa)
-}
-
-// MkdirAll is a custom version of os.MkdirAll that is volume path aware for
-// Windows. It can be used as a drop-in replacement for os.MkdirAll.
-func MkdirAll(path string, _ os.FileMode) error {
-	return mkdirall(path, nil)
+	return mkdirAllWithACL(path, sa)
 }
 
-// mkdirall is a custom version of os.MkdirAll modified for use on Windows
-// so that it is both volume path aware, and can create a directory with
-// a DACL.
-func mkdirall(path string, perm *windows.SecurityAttributes) error {
-	if volumePath.MatchString(path) {
-		return nil
+// mkdirAllWithACL is a custom version of os.MkdirAll with DACL support on Windows.
+// It is fully identical to [os.MkdirAll] if no DACL is provided.
+//
+// Code in this function is based on the implementation in [go1.23.4].
+//
+// Copyright 2009 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+//
+// [go1.23.4]: https://github.com/golang/go/blob/go1.23.4/src/os/path.go#L12-L66
+func mkdirAllWithACL(path string, perm *windows.SecurityAttributes) error {
+	if perm == nil {
+		return os.MkdirAll(path, 0)
 	}
-
-	// The rest of this method is largely copied from os.MkdirAll and should be kept
-	// as-is to ensure compatibility.
-
 	// Fast path: if we can tell whether path is a directory or file, stop with success or error.
 	dir, err := os.Stat(path)
 	if err == nil {
@@ -55,19 +47,25 @@ func mkdirall(path string, perm *windows.SecurityAttributes) error {
 	}
 
 	// Slow path: make sure parent exists and then call Mkdir for path.
-	i := len(path)
-	for i > 0 && os.IsPathSeparator(path[i-1]) { // Skip trailing path separator.
+
+	// Extract the parent folder from path by first removing any trailing
+	// path separator and then scanning backward until finding a path
+	// separator or reaching the beginning of the string.
+	i := len(path) - 1
+	for i >= 0 && os.IsPathSeparator(path[i]) {
 		i--
 	}
-
-	j := i
-	for j > 0 && !os.IsPathSeparator(path[j-1]) { // Scan backward over element.
-		j--
+	for i >= 0 && !os.IsPathSeparator(path[i]) {
+		i--
+	}
+	if i < 0 {
+		i = 0
 	}
 
-	if j > 1 {
-		// Create parent.
-		err = mkdirall(fixRootDirectory(path[:j-1]), perm)
+	// If there is a parent directory, and it is not the volume name,
+	// recurse to ensure parent directory exists.
+	if parent := path[:i]; len(parent) > len(filepath.VolumeName(path)) {
+		err = mkdirAllWithACL(parent, perm)
 		if err != nil {
 			return err
 		}
@@ -111,17 +109,6 @@ func mkdirWithACL(name string, sa *windows.SecurityAttributes) error {
 	return nil
 }
 
-// fixRootDirectory fixes a reference to a drive's root directory to
-// have the required trailing slash.
-func fixRootDirectory(p string) string {
-	if len(p) == len(`\\?\c:`) {
-		if os.IsPathSeparator(p[0]) && os.IsPathSeparator(p[1]) && p[2] == '?' && os.IsPathSeparator(p[3]) && p[5] == ':' {
-			return p + `\`
-		}
-	}
-	return p
-}
-
 func makeSecurityAttributes(sddl string) (*windows.SecurityAttributes, error) {
 	var sa windows.SecurityAttributes
 	sa.Length = uint32(unsafe.Sizeof(sa))