Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Dec 23, 2024
6 changes: 1 addition & 5 deletions builder/dockerfile/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions cmd/dockerd/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions container/container_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions daemon/graphdriver/windows/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand Down
3 changes: 1 addition & 2 deletions daemon/runtime_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions libcontainerd/supervisor/remote_daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 2 additions & 4 deletions pkg/idtools/idtools_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package idtools // import "github.com/docker/docker/pkg/idtools"

import (
"os"

"github.com/docker/docker/pkg/system"
)

const (
Expand All @@ -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)
}
8 changes: 8 additions & 0 deletions pkg/system/filesys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 1 addition & 7 deletions pkg/system/filesys_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
73 changes: 30 additions & 43 deletions pkg/system/filesys_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package system // import "github.com/docker/docker/pkg/system"

import (
"os"
"regexp"
"path/filepath"
"syscall"
"unsafe"

Expand All @@ -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.
Expand All @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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))
Expand Down
Loading