Merge pull request #42520 from thaJeztah/remove_lcow_step5_alternative

Remove LCOW (step 5): volumes/mounts: remove LCOW code (alternative)

container/container.go

@@ -474,11 +474,7 @@ func (container *Container) ShouldRestart() bool {
 
 // AddMountPointWithVolume adds a new mount point configured with a volume to the container.
 func (container *Container) AddMountPointWithVolume(destination string, vol volume.Volume, rw bool) {
-	operatingSystem := container.OS
+	volumeParser := volumemounts.NewParser()
-	if operatingSystem == "" {
-		operatingSystem = runtime.GOOS
-	}
-	volumeParser := volumemounts.NewParser(operatingSystem)
 	container.MountPoints[destination] = &volumemounts.MountPoint{
 		Type:        mounttypes.TypeVolume,
 		Name:        vol.Name(),

container/container_unix.go

@@ -64,7 +64,7 @@ func (container *Container) BuildHostnameFile() error {
 func (container *Container) NetworkMounts() []Mount {
 	var mounts []Mount
 	shared := container.HostConfig.NetworkMode.IsContainer()
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 	if container.ResolvConfPath != "" {
 		if _, err := os.Stat(container.ResolvConfPath); err != nil {
 			logrus.Warnf("ResolvConfPath set to %q, but can't stat this filename (err = %v); skipping", container.ResolvConfPath, err)
@@ -199,7 +199,7 @@ func (container *Container) UnmountIpcMount() error {
 // IpcMounts returns the list of IPC mounts
 func (container *Container) IpcMounts() []Mount {
 	var mounts []Mount
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 
 	if container.HasMountFor("/dev/shm") {
 		return mounts
@@ -419,7 +419,6 @@ func copyExistingContents(source, destination string) error {
 
 // TmpfsMounts returns the list of tmpfs mounts
 func (container *Container) TmpfsMounts() ([]Mount, error) {
-	parser := volumemounts.NewParser(container.OS)
 	var mounts []Mount
 	for dest, data := range container.HostConfig.Tmpfs {
 		mounts = append(mounts, Mount{
@@ -428,6 +427,7 @@ func (container *Container) TmpfsMounts() ([]Mount, error) {
 			Data:        data,
 		})
 	}
+	parser := volumemounts.NewParser()
 	for dest, mnt := range container.MountPoints {
 		if mnt.Type == mounttypes.TypeTmpfs {
 			data, err := parser.ConvertTmpfsOptions(mnt.Spec.TmpfsOptions, mnt.Spec.ReadOnly)

daemon/archive_unix.go

@@ -12,7 +12,7 @@ import (
 // cannot be configured with a read-only rootfs.
 func checkIfPathIsInAVolume(container *container.Container, absPath string) (bool, error) {
 	var toVolume bool
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 	for _, mnt := range container.MountPoints {
 		if toVolume = parser.HasResource(mnt, absPath); toVolume {
 			if mnt.RW {

daemon/container.go

@@ -3,10 +3,8 @@ package daemon // import "github.com/docker/docker/daemon"
 import (
 	"fmt"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
-	"strings"
 	"time"
 
 	containertypes "github.com/docker/docker/api/types/container"
@@ -231,12 +229,12 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
 
 // verifyContainerSettings performs validation of the hostconfig and config
 // structures.
-func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
+func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
 	// First perform verification of settings common across all platforms.
-	if err = validateContainerConfig(config, platform); err != nil {
+	if err = validateContainerConfig(config); err != nil {
 		return warnings, err
 	}
-	if err := validateHostConfig(hostConfig, platform); err != nil {
+	if err := validateHostConfig(hostConfig); err != nil {
 		return warnings, err
 	}
 
@@ -248,11 +246,11 @@ func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *conta
 	return warnings, err
 }
 
-func validateContainerConfig(config *containertypes.Config, platform string) error {
+func validateContainerConfig(config *containertypes.Config) error {
 	if config == nil {
 		return nil
 	}
-	if err := translateWorkingDir(config, platform); err != nil {
+	if err := translateWorkingDir(config); err != nil {
 		return err
 	}
 	if len(config.StopSignal) > 0 {
@@ -269,7 +267,7 @@ func validateContainerConfig(config *containertypes.Config, platform string) err
 	return validateHealthCheck(config.Healthcheck)
 }
 
-func validateHostConfig(hostConfig *containertypes.HostConfig, platform string) error {
+func validateHostConfig(hostConfig *containertypes.HostConfig) error {
 	if hostConfig == nil {
 		return nil
 	}
@@ -278,7 +276,7 @@ func validateHostConfig(hostConfig *containertypes.HostConfig, platform string)
 		return errors.Errorf("can't create 'AutoRemove' container with restart policy")
 	}
 	// Validate mounts; check if host directories still exist
-	parser := volumemounts.NewParser(platform)
+	parser := volumemounts.NewParser()
 	for _, c := range hostConfig.Mounts {
 		cfg := c
 		if err := parser.ValidateMountConfig(&cfg); err != nil {
@@ -373,24 +371,14 @@ func validateRestartPolicy(policy containertypes.RestartPolicy) error {
 
 // translateWorkingDir translates the working-dir for the target platform,
 // and returns an error if the given path is not an absolute path.
-func translateWorkingDir(config *containertypes.Config, platform string) error {
+func translateWorkingDir(config *containertypes.Config) error {
 	if config.WorkingDir == "" {
 		return nil
 	}
-	wd := config.WorkingDir
+	wd := filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
-	switch {
-	case runtime.GOOS != platform:
-		// LCOW. Force Unix semantics
-		wd = strings.Replace(wd, string(os.PathSeparator), "/", -1)
-		if !path.IsAbs(wd) {
-			return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
-		}
-	default:
-		wd = filepath.FromSlash(wd) // Ensure in platform semantics
 	if !system.IsAbs(wd) {
 		return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
 	}
-	}
 	config.WorkingDir = wd
 	return nil
 }

daemon/container_unix_test.go

@@ -38,7 +38,7 @@ func TestContainerWarningHostAndPublishPorts(t *testing.T) {
 			},
 		}
 		d := &Daemon{configStore: cs}
-		wrns, err := d.verifyContainerSettings("", hostConfig, &containertypes.Config{}, false)
+		wrns, err := d.verifyContainerSettings(hostConfig, &containertypes.Config{}, false)
 		assert.NilError(t, err)
 		assert.DeepEqual(t, tc.warnings, wrns)
 	}

daemon/create.go

@@ -16,7 +16,6 @@ import (
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/idtools"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/runconfig"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/selinux/go-selinux"
@@ -61,22 +60,13 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
 		return containertypes.ContainerCreateCreatedBody{}, errdefs.InvalidParameter(errors.New("Config cannot be empty in order to create a container"))
 	}
 
-	os := runtime.GOOS
+	warnings, err := daemon.verifyContainerSettings(opts.params.HostConfig, opts.params.Config, false)
-	var img *image.Image
-	if opts.params.Config.Image != "" {
-		var err error
-		img, err = daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform)
-		if err == nil {
-			os = img.OS
-		}
-	}
-
-	warnings, err := daemon.verifyContainerSettings(os, opts.params.HostConfig, opts.params.Config, false)
 	if err != nil {
 		return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, errdefs.InvalidParameter(err)
 	}
 
-	if img != nil && opts.params.Platform == nil {
+	if opts.params.Platform == nil && opts.params.Config.Image != "" {
+		if img, _ := daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform); img != nil {
 			p := platforms.DefaultSpec()
 			imgPlat := v1.Platform{
 				OS:           img.OS,
@@ -88,6 +78,7 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
 				warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.Format(imgPlat), platforms.Format(p)))
 			}
 		}
+	}
 
 	err = verifyNetworkingConfig(opts.params.NetworkingConfig)
 	if err != nil {
@@ -132,22 +123,14 @@ func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr
 		}
 		if img.OS != "" {
 			os = img.OS
-		} else {
-			// default to the host OS except on Windows with LCOW
-			if isWindows && system.LCOWSupported() {
-				os = "linux"
-			}
 		}
 		imgID = img.ID()
-
+		if isWindows && img.OS == "linux" {
-		if isWindows && img.OS == "linux" && !system.LCOWSupported() {
 			return nil, errors.New("operating system on which parent image was created is not Windows")
 		}
-	} else {
+	} else if isWindows {
-		if isWindows {
 		os = "linux" // 'scratch' case.
 	}
-	}
 
 	// On WCOW, if are not being invoked by the builder to create this container (where
 	// ignoreImagesArgEscaped will be true) - if the image already has its arguments escaped,

daemon/create_windows.go

@@ -28,7 +28,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
 		}
 		hostConfig.Isolation = "hyperv"
 	}
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 	for spec := range config.Volumes {
 
 		mp, err := parser.ParseMountRaw(spec, hostConfig.VolumeDriver)

daemon/daemon_linux_test.go

@@ -116,7 +116,7 @@ func TestNotCleanupMounts(t *testing.T) {
 func TestValidateContainerIsolationLinux(t *testing.T) {
 	d := Daemon{}
 
-	_, err := d.verifyContainerSettings("linux", &containertypes.HostConfig{Isolation: containertypes.IsolationHyperV}, nil, false)
+	_, err := d.verifyContainerSettings(&containertypes.HostConfig{Isolation: containertypes.IsolationHyperV}, nil, false)
 	assert.Check(t, is.Error(err, "invalid isolation 'hyperv' on linux"))
 }
 

daemon/daemon_test.go

@@ -305,7 +305,7 @@ func TestMerge(t *testing.T) {
 func TestValidateContainerIsolation(t *testing.T) {
 	d := Daemon{}
 
-	_, err := d.verifyContainerSettings(runtime.GOOS, &containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false)
+	_, err := d.verifyContainerSettings(&containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false)
 	assert.Check(t, is.Error(err, "invalid isolation 'invalid' on "+runtime.GOOS))
 }
 

daemon/daemon_unix.go

@@ -719,7 +719,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.
 		return warnings, fmt.Errorf("Unknown runtime specified %s", hostConfig.Runtime)
 	}
 
-	parser := volumemounts.NewParser(runtime.GOOS)
+	parser := volumemounts.NewParser()
 	for dest := range hostConfig.Tmpfs {
 		if err := parser.ValidateTmpfsMountDestination(dest); err != nil {
 			return warnings, err

daemon/oci_linux.go

@@ -570,7 +570,7 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
 		for _, m := range mounts {
 			if m.Source == "tmpfs" {
 				data := m.Data
-				parser := volumemounts.NewParser("linux")
+				parser := volumemounts.NewParser()
 				options := []string{"noexec", "nosuid", "nodev", string(parser.DefaultPropagationMode())}
 				if data != "" {
 					options = append(options, strings.Split(data, ",")...)

daemon/start.go

@@ -81,7 +81,7 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos
 
 	// check if hostConfig is in line with the current system settings.
 	// It may happen cgroups are umounted or the like.
-	if _, err = daemon.verifyContainerSettings(ctr.OS, ctr.HostConfig, nil, false); err != nil {
+	if _, err = daemon.verifyContainerSettings(ctr.HostConfig, nil, false); err != nil {
 		return errdefs.InvalidParameter(err)
 	}
 	// Adapt for old containers in case we have updates in this function and

daemon/update.go

@@ -13,12 +13,7 @@ import (
 func (daemon *Daemon) ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) {
 	var warnings []string
 
-	c, err := daemon.GetContainer(name)
+	warnings, err := daemon.verifyContainerSettings(hostConfig, nil, true)
-	if err != nil {
-		return container.ContainerUpdateOKBody{Warnings: warnings}, err
-	}
-
-	warnings, err = daemon.verifyContainerSettings(c.OS, hostConfig, nil, true)
 	if err != nil {
 		return container.ContainerUpdateOKBody{Warnings: warnings}, errdefs.InvalidParameter(err)
 	}

daemon/volumes.go

@@ -62,7 +62,7 @@ func (m mounts) parts(i int) int {
 func (daemon *Daemon) registerMountPoints(container *container.Container, hostConfig *containertypes.HostConfig) (retErr error) {
 	binds := map[string]bool{}
 	mountPoints := map[string]*volumemounts.MountPoint{}
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 
 	ctx := context.TODO()
 	defer func() {
@@ -265,8 +265,6 @@ func (daemon *Daemon) backportMountSpec(container *container.Container) {
 	container.Lock()
 	defer container.Unlock()
 
-	parser := volumemounts.NewParser(container.OS)
-
 	maybeUpdate := make(map[string]bool)
 	for _, mp := range container.MountPoints {
 		if mp.Spec.Source != "" && mp.Type != "" {
@@ -283,6 +281,7 @@ func (daemon *Daemon) backportMountSpec(container *container.Container) {
 		mountSpecs[m.Target] = true
 	}
 
+	parser := volumemounts.NewParser()
 	binds := make(map[string]*volumemounts.MountPoint, len(container.HostConfig.Binds))
 	for _, rawSpec := range container.HostConfig.Binds {
 		mp, err := parser.ParseMountRaw(rawSpec, container.HostConfig.VolumeDriver)

daemon/volumes_unit_test.go

@@ -1,7 +1,6 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
-	"runtime"
 	"testing"
 
 	volumemounts "github.com/docker/docker/volume/mounts"
@@ -21,7 +20,7 @@ func TestParseVolumesFrom(t *testing.T) {
 		{"foobar:baz", "", "", true},
 	}
 
-	parser := volumemounts.NewParser(runtime.GOOS)
+	parser := volumemounts.NewParser()
 
 	for _, c := range cases {
 		id, mode, err := parser.ParseVolumesFrom(c.spec)

volume/mounts/lcow_parser.go

@@ -7,6 +7,23 @@ import (
 	"github.com/docker/docker/api/types/mount"
 )
 
+// NewLCOWParser creates a parser with Linux Containers on Windows semantics.
+func NewLCOWParser() Parser {
+	return &lcowParser{
+		windowsParser{
+			fi: defaultFileInfoProvider{},
+		},
+	}
+}
+
+// rxLCOWDestination is the regex expression for the mount destination for LCOW
+//
+// Destination (aka container path):
+//    -  Variation on hostdir but can be a drive followed by colon as well
+//    -  If a path, must be absolute. Can include spaces
+//    -  Drive cannot be c: (explicitly checked in code, not RegEx)
+const rxLCOWDestination = `(?P<destination>/(?:[^\\/:*?"<>\r\n]+[/]?)*)`
+
 var lcowSpecificValidators mountValidator = func(m *mount.Mount) error {
 	if path.Clean(m.Target) == "/" {
 		return ErrVolumeTargetIsRoot

volume/mounts/lcow_parser_test.go

@@ -0,0 +1,152 @@
+package mounts // import "github.com/docker/docker/volume/mounts"
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/docker/docker/api/types/mount"
+	"gotest.tools/v3/assert"
+)
+
+func TestLCOWParseMountRaw(t *testing.T) {
+	valid := []string{
+		`/foo`,
+		`/foo/`,
+		`/foo bar`,
+		`c:\:/foo`,
+		`c:\windows\:/foo`,
+		`c:\windows:/s p a c e`,
+		`c:\windows:/s p a c e:RW`,
+		`c:\program files:/s p a c e i n h o s t d i r`,
+		`0123456789name:/foo`,
+		`MiXeDcAsEnAmE:/foo`,
+		`name:/foo`,
+		`name:/foo:rW`,
+		`name:/foo:RW`,
+		`name:/foo:RO`,
+		`c:/:/forward/slashes/are/good/too`,
+		`c:/:/including with/spaces:ro`,
+		`/Program Files (x86)`, // With capitals and brackets
+	}
+
+	invalid := map[string]string{
+		``:                                   "invalid volume specification: ",
+		`.`:                                  "invalid volume specification: ",
+		`c:`:                                 "invalid volume specification: ",
+		`c:\`:                                "invalid volume specification: ",
+		`../`:                                "invalid volume specification: ",
+		`c:\:../`:                            "invalid volume specification: ",
+		`c:\:/foo:xyzzy`:                     "invalid volume specification: ",
+		`/`:                                  "destination can't be '/'",
+		`/..`:                                "destination can't be '/'",
+		`c:\notexist:/foo`:                   `source path does not exist: c:\notexist`,
+		`c:\windows\system32\ntdll.dll:/foo`: `source path must be a directory`,
+		`name<:/foo`:                         `invalid volume specification`,
+		`name>:/foo`:                         `invalid volume specification`,
+		`name::/foo`:                         `invalid volume specification`,
+		`name":/foo`:                         `invalid volume specification`,
+		`name\:/foo`:                         `invalid volume specification`,
+		`name*:/foo`:                         `invalid volume specification`,
+		`name|:/foo`:                         `invalid volume specification`,
+		`name?:/foo`:                         `invalid volume specification`,
+		`name/:/foo`:                         `invalid volume specification`,
+		`/foo:rw`:                            `invalid volume specification`,
+		`/foo:ro`:                            `invalid volume specification`,
+		`con:/foo`:                           `cannot be a reserved word for Windows filenames`,
+		`PRN:/foo`:                           `cannot be a reserved word for Windows filenames`,
+		`aUx:/foo`:                           `cannot be a reserved word for Windows filenames`,
+		`nul:/foo`:                           `cannot be a reserved word for Windows filenames`,
+		`com1:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com2:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com3:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com4:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com5:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com6:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com7:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com8:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`com9:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt1:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt2:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt3:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt4:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt5:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt6:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt7:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt8:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt9:/foo`:                          `cannot be a reserved word for Windows filenames`,
+		`\\.\pipe\foo:/foo`:                  `Linux containers on Windows do not support named pipe mounts`,
+	}
+
+	parser := NewLCOWParser()
+	if p, ok := parser.(*lcowParser); ok {
+		p.fi = mockFiProvider{}
+	}
+
+	for _, path := range valid {
+		if _, err := parser.ParseMountRaw(path, "local"); err != nil {
+			t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
+		}
+	}
+
+	for path, expectedError := range invalid {
+		if mp, err := parser.ParseMountRaw(path, "local"); err == nil {
+			t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
+		} else {
+			if !strings.Contains(err.Error(), expectedError) {
+				t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+			}
+		}
+	}
+}
+
+func TestLCOWParseMountRawSplit(t *testing.T) {
+	cases := []struct {
+		bind      string
+		driver    string
+		expType   mount.Type
+		expDest   string
+		expSource string
+		expName   string
+		expDriver string
+		expRW     bool
+		fail      bool
+	}{
+		{`c:\:/foo`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", true, false},
+		{`c:\:/foo:ro`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", false, false},
+		{`c:\:/foo:rw`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", true, false},
+		{`c:\:/foo:foo`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", false, true},
+		{`name:/foo:rw`, "local", mount.TypeVolume, `/foo`, ``, `name`, "local", true, false},
+		{`name:/foo`, "local", mount.TypeVolume, `/foo`, ``, `name`, "local", true, false},
+		{`name:/foo:ro`, "local", mount.TypeVolume, `/foo`, ``, `name`, "local", false, false},
+		{`name:/`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
+		{`driver/name:/`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
+		{`\\.\pipe\foo:\\.\pipe\bar`, "local", mount.TypeNamedPipe, `\\.\pipe\bar`, `\\.\pipe\foo`, "", "", true, true},
+		{`\\.\pipe\foo:/data`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
+		{`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
+	}
+
+	parser := NewLCOWParser()
+	if p, ok := parser.(*lcowParser); ok {
+		p.fi = mockFiProvider{}
+	}
+
+	for i, c := range cases {
+		c := c
+		t.Run(fmt.Sprintf("%d_%s", i, c.bind), func(t *testing.T) {
+			m, err := parser.ParseMountRaw(c.bind, c.driver)
+			if c.fail {
+				assert.ErrorContains(t, err, "", "expected an error")
+				return
+			}
+
+			assert.NilError(t, err)
+			assert.Equal(t, m.Destination, c.expDest)
+			assert.Equal(t, m.Source, c.expSource)
+			assert.Equal(t, m.Name, c.expName)
+			assert.Equal(t, m.Driver, c.expDriver)
+			assert.Equal(t, m.RW, c.expRW)
+			assert.Equal(t, m.Type, c.expType)
+		})
+	}
+}

volume/mounts/linux_parser.go

@@ -12,7 +12,15 @@ import (
 	"github.com/docker/docker/volume"
 )
 
+// NewLinuxParser creates a parser with Linux semantics.
+func NewLinuxParser() Parser {
+	return &linuxParser{
+		fi: defaultFileInfoProvider{},
+	}
+}
+
 type linuxParser struct {
+	fi fileInfoProvider
 }
 
 func linuxSplitRawSpec(raw string) ([]string, error) {
@@ -82,7 +90,7 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour
 		}
 
 		if validateBindSourceExists {
-			exists, _, err := currentFileInfoProvider.fileInfo(mnt.Source)
+			exists, _, err := p.fi.fileInfo(mnt.Source)
 			if err != nil {
 				return &errMountConfig{mnt, err}
 			}
@@ -115,12 +123,6 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour
 	return nil
 }
 
-// read-write modes
-var rwModes = map[string]bool{
-	"rw": true,
-	"ro": true,
-}
-
 // label modes
 var linuxLabelModes = map[string]bool{
 	"Z": true,

volume/mounts/linux_parser_test.go

@@ -0,0 +1,221 @@
+package mounts // import "github.com/docker/docker/volume/mounts"
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/docker/docker/api/types/mount"
+	"gotest.tools/v3/assert"
+)
+
+func TestLinuxParseMountRaw(t *testing.T) {
+	valid := []string{
+		"/home",
+		"/home:/home",
+		"/home:/something/else",
+		"/with space",
+		"/home:/with space",
+		"relative:/absolute-path",
+		"hostPath:/containerPath:ro",
+		"/hostPath:/containerPath:rw",
+		"/rw:/ro",
+		"/hostPath:/containerPath:shared",
+		"/hostPath:/containerPath:rshared",
+		"/hostPath:/containerPath:slave",
+		"/hostPath:/containerPath:rslave",
+		"/hostPath:/containerPath:private",
+		"/hostPath:/containerPath:rprivate",
+		"/hostPath:/containerPath:ro,shared",
+		"/hostPath:/containerPath:ro,slave",
+		"/hostPath:/containerPath:ro,private",
+		"/hostPath:/containerPath:ro,z,shared",
+		"/hostPath:/containerPath:ro,Z,slave",
+		"/hostPath:/containerPath:Z,ro,slave",
+		"/hostPath:/containerPath:slave,Z,ro",
+		"/hostPath:/containerPath:Z,slave,ro",
+		"/hostPath:/containerPath:slave,ro,Z",
+		"/hostPath:/containerPath:rslave,ro,Z",
+		"/hostPath:/containerPath:ro,rshared,Z",
+		"/hostPath:/containerPath:ro,Z,rprivate",
+	}
+
+	invalid := map[string]string{
+		"":                                "invalid volume specification",
+		"./":                              "mount path must be absolute",
+		"../":                             "mount path must be absolute",
+		"/:../":                           "mount path must be absolute",
+		"/:path":                          "mount path must be absolute",
+		":":                               "invalid volume specification",
+		"/tmp:":                           "invalid volume specification",
+		":test":                           "invalid volume specification",
+		":/test":                          "invalid volume specification",
+		"tmp:":                            "invalid volume specification",
+		":test:":                          "invalid volume specification",
+		"::":                              "invalid volume specification",
+		":::":                             "invalid volume specification",
+		"/tmp:::":                         "invalid volume specification",
+		":/tmp::":                         "invalid volume specification",
+		"/path:rw":                        "invalid volume specification",
+		"/path:ro":                        "invalid volume specification",
+		"/rw:rw":                          "invalid volume specification",
+		"path:ro":                         "invalid volume specification",
+		"/path:/path:sw":                  `invalid mode`,
+		"/path:/path:rwz":                 `invalid mode`,
+		"/path:/path:ro,rshared,rslave":   `invalid mode`,
+		"/path:/path:ro,z,rshared,rslave": `invalid mode`,
+		"/path:shared":                    "invalid volume specification",
+		"/path:slave":                     "invalid volume specification",
+		"/path:private":                   "invalid volume specification",
+		"name:/absolute-path:shared":      "invalid volume specification",
+		"name:/absolute-path:rshared":     "invalid volume specification",
+		"name:/absolute-path:slave":       "invalid volume specification",
+		"name:/absolute-path:rslave":      "invalid volume specification",
+		"name:/absolute-path:private":     "invalid volume specification",
+		"name:/absolute-path:rprivate":    "invalid volume specification",
+	}
+
+	parser := NewLinuxParser()
+	if p, ok := parser.(*linuxParser); ok {
+		p.fi = mockFiProvider{}
+	}
+
+	for _, path := range valid {
+		if _, err := parser.ParseMountRaw(path, "local"); err != nil {
+			t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
+		}
+	}
+
+	for path, expectedError := range invalid {
+		if mp, err := parser.ParseMountRaw(path, "local"); err == nil {
+			t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
+		} else {
+			if !strings.Contains(err.Error(), expectedError) {
+				t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+			}
+		}
+	}
+}
+
+func TestLinuxParseMountRawSplit(t *testing.T) {
+	cases := []struct {
+		bind      string
+		driver    string
+		expType   mount.Type
+		expDest   string
+		expSource string
+		expName   string
+		expDriver string
+		expRW     bool
+		fail      bool
+	}{
+		{"/tmp:/tmp1", "", mount.TypeBind, "/tmp1", "/tmp", "", "", true, false},
+		{"/tmp:/tmp2:ro", "", mount.TypeBind, "/tmp2", "/tmp", "", "", false, false},
+		{"/tmp:/tmp3:rw", "", mount.TypeBind, "/tmp3", "/tmp", "", "", true, false},
+		{"/tmp:/tmp4:foo", "", mount.TypeBind, "", "", "", "", false, true},
+		{"name:/named1", "", mount.TypeVolume, "/named1", "", "name", "", true, false},
+		{"name:/named2", "external", mount.TypeVolume, "/named2", "", "name", "external", true, false},
+		{"name:/named3:ro", "local", mount.TypeVolume, "/named3", "", "name", "local", false, false},
+		{"local/name:/tmp:rw", "", mount.TypeVolume, "/tmp", "", "local/name", "", true, false},
+		{"/tmp:tmp", "", mount.TypeBind, "", "", "", "", true, true},
+	}
+
+	parser := NewLinuxParser()
+	if p, ok := parser.(*linuxParser); ok {
+		p.fi = mockFiProvider{}
+	}
+
+	for i, c := range cases {
+		c := c
+		t.Run(fmt.Sprintf("%d_%s", i, c.bind), func(t *testing.T) {
+			m, err := parser.ParseMountRaw(c.bind, c.driver)
+			if c.fail {
+				assert.ErrorContains(t, err, "", "expected an error")
+				return
+			}
+
+			assert.NilError(t, err)
+			assert.Equal(t, m.Destination, c.expDest)
+			assert.Equal(t, m.Source, c.expSource)
+			assert.Equal(t, m.Name, c.expName)
+			assert.Equal(t, m.Driver, c.expDriver)
+			assert.Equal(t, m.RW, c.expRW)
+			assert.Equal(t, m.Type, c.expType)
+		})
+	}
+}
+
+// TestLinuxParseMountSpecBindWithFileinfoError makes sure that the parser returns
+// the error produced by the fileinfo provider.
+//
+// Some extra context for the future in case of changes and possible wtf are we
+// testing this for:
+//
+// Currently this "fileInfoProvider" returns (bool, bool, error)
+// The 1st bool is "does this path exist"
+// The 2nd bool is "is this path a dir"
+// Then of course the error is an error.
+//
+// The issue is the parser was ignoring the error and only looking at the
+// "does this path exist" boolean, which is always false if there is an error.
+// Then the error returned to the caller was a (slightly, maybe) friendlier
+// error string than what comes from `os.Stat`
+// So ...the caller was always getting an error saying the path doesn't exist
+// even if it does exist but got some other error (like a permission error).
+// This is confusing to users.
+func TestLinuxParseMountSpecBindWithFileinfoError(t *testing.T) {
+	parser := NewLinuxParser()
+	testErr := fmt.Errorf("some crazy error")
+	if pr, ok := parser.(*linuxParser); ok {
+		pr.fi = &mockFiProviderWithError{err: testErr}
+	}
+
+	_, err := parser.ParseMountSpec(mount.Mount{
+		Type:   mount.TypeBind,
+		Source: `/bananas`,
+		Target: `/bananas`,
+	})
+	assert.ErrorContains(t, err, testErr.Error())
+}
+
+func TestConvertTmpfsOptions(t *testing.T) {
+	type testCase struct {
+		opt                  mount.TmpfsOptions
+		readOnly             bool
+		expectedSubstrings   []string
+		unexpectedSubstrings []string
+	}
+	cases := []testCase{
+		{
+			opt:                  mount.TmpfsOptions{SizeBytes: 1024 * 1024, Mode: 0700},
+			readOnly:             false,
+			expectedSubstrings:   []string{"size=1m", "mode=700"},
+			unexpectedSubstrings: []string{"ro"},
+		},
+		{
+			opt:                  mount.TmpfsOptions{},
+			readOnly:             true,
+			expectedSubstrings:   []string{"ro"},
+			unexpectedSubstrings: []string{},
+		},
+	}
+	p := NewLinuxParser()
+	for _, c := range cases {
+		data, err := p.ConvertTmpfsOptions(&c.opt, c.readOnly)
+		if err != nil {
+			t.Fatalf("could not convert %+v (readOnly: %v) to string: %v",
+				c.opt, c.readOnly, err)
+		}
+		t.Logf("data=%q", data)
+		for _, s := range c.expectedSubstrings {
+			if !strings.Contains(data, s) {
+				t.Fatalf("expected substring: %s, got %v (case=%+v)", s, data, c)
+			}
+		}
+		for _, s := range c.unexpectedSubstrings {
+			if strings.Contains(data, s) {
+				t.Fatalf("unexpected substring: %s, got %v (case=%+v)", s, data, c)
+			}
+		}
+	}
+}

volume/mounts/parser.go

@@ -7,17 +7,16 @@ import (
 	"github.com/docker/docker/api/types/mount"
 )
 
-const (
-	// OSLinux is the same as runtime.GOOS on linux
-	OSLinux = "linux"
-	// OSWindows is the same as runtime.GOOS on windows
-	OSWindows = "windows"
-)
-
 // ErrVolumeTargetIsRoot is returned when the target destination is root.
 // It's used by both LCOW and Linux parsers.
 var ErrVolumeTargetIsRoot = errors.New("invalid specification: destination can't be '/'")
 
+// read-write modes
+var rwModes = map[string]bool{
+	"rw": true,
+	"ro": true,
+}
+
 // Parser represents a platform specific parser for mount expressions
 type Parser interface {
 	ParseMountRaw(raw, volumeDriver string) (*MountPoint, error)
@@ -34,14 +33,10 @@ type Parser interface {
 	ValidateMountConfig(mt *mount.Mount) error
 }
 
-// NewParser creates a parser for a given container OS, depending on the current host OS (linux on a windows host will resolve to an lcowParser)
+// NewParser creates a parser for the current host OS
-func NewParser(containerOS string) Parser {
+func NewParser() Parser {
-	switch containerOS {
+	if runtime.GOOS == "windows" {
-	case OSWindows:
+		return NewWindowsParser()
-		return &windowsParser{}
 	}
-	if runtime.GOOS == OSWindows {
+	return NewLinuxParser()
-		return &lcowParser{}
-	}
-	return &linuxParser{}
 }

volume/mounts/parser_test.go

@@ -1,65 +1,13 @@
 package mounts // import "github.com/docker/docker/volume/mounts"
 
 import (
-	"errors"
 	"io/ioutil"
 	"os"
-	"runtime"
-	"strings"
 	"testing"
 
 	"github.com/docker/docker/api/types/mount"
-	"gotest.tools/v3/assert"
-	"gotest.tools/v3/assert/cmp"
 )
 
-type parseMountRawTestSet struct {
-	valid   []string
-	invalid map[string]string
-}
-
-func TestConvertTmpfsOptions(t *testing.T) {
-	type testCase struct {
-		opt                  mount.TmpfsOptions
-		readOnly             bool
-		expectedSubstrings   []string
-		unexpectedSubstrings []string
-	}
-	cases := []testCase{
-		{
-			opt:                  mount.TmpfsOptions{SizeBytes: 1024 * 1024, Mode: 0700},
-			readOnly:             false,
-			expectedSubstrings:   []string{"size=1m", "mode=700"},
-			unexpectedSubstrings: []string{"ro"},
-		},
-		{
-			opt:                  mount.TmpfsOptions{},
-			readOnly:             true,
-			expectedSubstrings:   []string{"ro"},
-			unexpectedSubstrings: []string{},
-		},
-	}
-	p := &linuxParser{}
-	for _, c := range cases {
-		data, err := p.ConvertTmpfsOptions(&c.opt, c.readOnly)
-		if err != nil {
-			t.Fatalf("could not convert %+v (readOnly: %v) to string: %v",
-				c.opt, c.readOnly, err)
-		}
-		t.Logf("data=%q", data)
-		for _, s := range c.expectedSubstrings {
-			if !strings.Contains(data, s) {
-				t.Fatalf("expected substring: %s, got %v (case=%+v)", s, data, c)
-			}
-		}
-		for _, s := range c.unexpectedSubstrings {
-			if strings.Contains(data, s) {
-				t.Fatalf("unexpected substring: %s, got %v (case=%+v)", s, data, c)
-			}
-		}
-	}
-}
-
 type mockFiProvider struct{}
 
 func (mockFiProvider) fileInfo(path string) (exists, isDir bool, err error) {
@@ -84,365 +32,25 @@ func (mockFiProvider) fileInfo(path string) (exists, isDir bool, err error) {
 	return false, false, nil
 }
 
-func TestParseMountRaw(t *testing.T) {
+// always returns the configured error
+// this is used to test error handling
+type mockFiProviderWithError struct{ err error }
 
-	previousProvider := currentFileInfoProvider
+func (m mockFiProviderWithError) fileInfo(path string) (bool, bool, error) {
-	defer func() { currentFileInfoProvider = previousProvider }()
+	return false, false, m.err
-	currentFileInfoProvider = mockFiProvider{}
-	windowsSet := parseMountRawTestSet{
-		valid: []string{
-			`d:\`,
-			`d:`,
-			`d:\path`,
-			`d:\path with space`,
-			`c:\:d:\`,
-			`c:\windows\:d:`,
-			`c:\windows:d:\s p a c e`,
-			`c:\windows:d:\s p a c e:RW`,
-			`c:\program files:d:\s p a c e i n h o s t d i r`,
-			`0123456789name:d:`,
-			`MiXeDcAsEnAmE:d:`,
-			`name:D:`,
-			`name:D::rW`,
-			`name:D::RW`,
-			`name:D::RO`,
-			`c:/:d:/forward/slashes/are/good/too`,
-			`c:/:d:/including with/spaces:ro`,
-			`c:\Windows`,                // With capital
-			`c:\Program Files (x86)`,    // With capitals and brackets
-			`\\?\c:\windows\:d:`,        // Long path handling (source)
-			`c:\windows\:\\?\d:\`,       // Long path handling (target)
-			`\\.\pipe\foo:\\.\pipe\foo`, // named pipe
-			`//./pipe/foo://./pipe/foo`, // named pipe forward slashes
-		},
-		invalid: map[string]string{
-			``:                                 "invalid volume specification: ",
-			`.`:                                "invalid volume specification: ",
-			`..\`:                              "invalid volume specification: ",
-			`c:\:..\`:                          "invalid volume specification: ",
-			`c:\:d:\:xyzzy`:                    "invalid volume specification: ",
-			`c:`:                               "cannot be `c:`",
-			`c:\`:                              "cannot be `c:`",
-			`c:\notexist:d:`:                   `source path does not exist: c:\notexist`,
-			`c:\windows\system32\ntdll.dll:d:`: `source path must be a directory`,
-			`name<:d:`:                         `invalid volume specification`,
-			`name>:d:`:                         `invalid volume specification`,
-			`name::d:`:                         `invalid volume specification`,
-			`name":d:`:                         `invalid volume specification`,
-			`name\:d:`:                         `invalid volume specification`,
-			`name*:d:`:                         `invalid volume specification`,
-			`name|:d:`:                         `invalid volume specification`,
-			`name?:d:`:                         `invalid volume specification`,
-			`name/:d:`:                         `invalid volume specification`,
-			`d:\pathandmode:rw`:                `invalid volume specification`,
-			`d:\pathandmode:ro`:                `invalid volume specification`,
-			`con:d:`:                           `cannot be a reserved word for Windows filenames`,
-			`PRN:d:`:                           `cannot be a reserved word for Windows filenames`,
-			`aUx:d:`:                           `cannot be a reserved word for Windows filenames`,
-			`nul:d:`:                           `cannot be a reserved word for Windows filenames`,
-			`com1:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com2:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com3:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com4:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com5:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com6:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com7:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com8:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`com9:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt1:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt2:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt3:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt4:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt5:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt6:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt7:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt8:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt9:d:`:                          `cannot be a reserved word for Windows filenames`,
-			`c:\windows\system32\ntdll.dll`:    `Only directories can be mapped on this platform`,
-			`\\.\pipe\foo:c:\pipe`:             `'c:\pipe' is not a valid pipe path`,
-		},
-	}
-	lcowSet := parseMountRawTestSet{
-		valid: []string{
-			`/foo`,
-			`/foo/`,
-			`/foo bar`,
-			`c:\:/foo`,
-			`c:\windows\:/foo`,
-			`c:\windows:/s p a c e`,
-			`c:\windows:/s p a c e:RW`,
-			`c:\program files:/s p a c e i n h o s t d i r`,
-			`0123456789name:/foo`,
-			`MiXeDcAsEnAmE:/foo`,
-			`name:/foo`,
-			`name:/foo:rW`,
-			`name:/foo:RW`,
-			`name:/foo:RO`,
-			`c:/:/forward/slashes/are/good/too`,
-			`c:/:/including with/spaces:ro`,
-			`/Program Files (x86)`, // With capitals and brackets
-		},
-		invalid: map[string]string{
-			``:                                   "invalid volume specification: ",
-			`.`:                                  "invalid volume specification: ",
-			`c:`:                                 "invalid volume specification: ",
-			`c:\`:                                "invalid volume specification: ",
-			`../`:                                "invalid volume specification: ",
-			`c:\:../`:                            "invalid volume specification: ",
-			`c:\:/foo:xyzzy`:                     "invalid volume specification: ",
-			`/`:                                  "destination can't be '/'",
-			`/..`:                                "destination can't be '/'",
-			`c:\notexist:/foo`:                   `source path does not exist: c:\notexist`,
-			`c:\windows\system32\ntdll.dll:/foo`: `source path must be a directory`,
-			`name<:/foo`:                         `invalid volume specification`,
-			`name>:/foo`:                         `invalid volume specification`,
-			`name::/foo`:                         `invalid volume specification`,
-			`name":/foo`:                         `invalid volume specification`,
-			`name\:/foo`:                         `invalid volume specification`,
-			`name*:/foo`:                         `invalid volume specification`,
-			`name|:/foo`:                         `invalid volume specification`,
-			`name?:/foo`:                         `invalid volume specification`,
-			`name/:/foo`:                         `invalid volume specification`,
-			`/foo:rw`:                            `invalid volume specification`,
-			`/foo:ro`:                            `invalid volume specification`,
-			`con:/foo`:                           `cannot be a reserved word for Windows filenames`,
-			`PRN:/foo`:                           `cannot be a reserved word for Windows filenames`,
-			`aUx:/foo`:                           `cannot be a reserved word for Windows filenames`,
-			`nul:/foo`:                           `cannot be a reserved word for Windows filenames`,
-			`com1:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com2:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com3:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com4:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com5:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com6:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com7:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com8:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`com9:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt1:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt2:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt3:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt4:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt5:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt6:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt7:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt8:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`lpt9:/foo`:                          `cannot be a reserved word for Windows filenames`,
-			`\\.\pipe\foo:/foo`:                  `Linux containers on Windows do not support named pipe mounts`,
-		},
-	}
-	linuxSet := parseMountRawTestSet{
-		valid: []string{
-			"/home",
-			"/home:/home",
-			"/home:/something/else",
-			"/with space",
-			"/home:/with space",
-			"relative:/absolute-path",
-			"hostPath:/containerPath:ro",
-			"/hostPath:/containerPath:rw",
-			"/rw:/ro",
-			"/hostPath:/containerPath:shared",
-			"/hostPath:/containerPath:rshared",
-			"/hostPath:/containerPath:slave",
-			"/hostPath:/containerPath:rslave",
-			"/hostPath:/containerPath:private",
-			"/hostPath:/containerPath:rprivate",
-			"/hostPath:/containerPath:ro,shared",
-			"/hostPath:/containerPath:ro,slave",
-			"/hostPath:/containerPath:ro,private",
-			"/hostPath:/containerPath:ro,z,shared",
-			"/hostPath:/containerPath:ro,Z,slave",
-			"/hostPath:/containerPath:Z,ro,slave",
-			"/hostPath:/containerPath:slave,Z,ro",
-			"/hostPath:/containerPath:Z,slave,ro",
-			"/hostPath:/containerPath:slave,ro,Z",
-			"/hostPath:/containerPath:rslave,ro,Z",
-			"/hostPath:/containerPath:ro,rshared,Z",
-			"/hostPath:/containerPath:ro,Z,rprivate",
-		},
-		invalid: map[string]string{
-			"":                                "invalid volume specification",
-			"./":                              "mount path must be absolute",
-			"../":                             "mount path must be absolute",
-			"/:../":                           "mount path must be absolute",
-			"/:path":                          "mount path must be absolute",
-			":":                               "invalid volume specification",
-			"/tmp:":                           "invalid volume specification",
-			":test":                           "invalid volume specification",
-			":/test":                          "invalid volume specification",
-			"tmp:":                            "invalid volume specification",
-			":test:":                          "invalid volume specification",
-			"::":                              "invalid volume specification",
-			":::":                             "invalid volume specification",
-			"/tmp:::":                         "invalid volume specification",
-			":/tmp::":                         "invalid volume specification",
-			"/path:rw":                        "invalid volume specification",
-			"/path:ro":                        "invalid volume specification",
-			"/rw:rw":                          "invalid volume specification",
-			"path:ro":                         "invalid volume specification",
-			"/path:/path:sw":                  `invalid mode`,
-			"/path:/path:rwz":                 `invalid mode`,
-			"/path:/path:ro,rshared,rslave":   `invalid mode`,
-			"/path:/path:ro,z,rshared,rslave": `invalid mode`,
-			"/path:shared":                    "invalid volume specification",
-			"/path:slave":                     "invalid volume specification",
-			"/path:private":                   "invalid volume specification",
-			"name:/absolute-path:shared":      "invalid volume specification",
-			"name:/absolute-path:rshared":     "invalid volume specification",
-			"name:/absolute-path:slave":       "invalid volume specification",
-			"name:/absolute-path:rslave":      "invalid volume specification",
-			"name:/absolute-path:private":     "invalid volume specification",
-			"name:/absolute-path:rprivate":    "invalid volume specification",
-		},
-	}
-
-	linParser := &linuxParser{}
-	winParser := &windowsParser{}
-	lcowParser := &lcowParser{}
-	tester := func(parser Parser, set parseMountRawTestSet) {
-
-		for _, path := range set.valid {
-
-			if _, err := parser.ParseMountRaw(path, "local"); err != nil {
-				t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
-			}
-		}
-
-		for path, expectedError := range set.invalid {
-			if mp, err := parser.ParseMountRaw(path, "local"); err == nil {
-				t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
-			} else {
-				if !strings.Contains(err.Error(), expectedError) {
-					t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
-				}
-			}
-		}
-	}
-	tester(linParser, linuxSet)
-	tester(winParser, windowsSet)
-	tester(lcowParser, lcowSet)
-
-}
-
-// testParseMountRaw is a structure used by TestParseMountRawSplit for
-// specifying test cases for the ParseMountRaw() function.
-type testParseMountRaw struct {
-	bind      string
-	driver    string
-	expType   mount.Type
-	expDest   string
-	expSource string
-	expName   string
-	expDriver string
-	expRW     bool
-	fail      bool
-}
-
-func TestParseMountRawSplit(t *testing.T) {
-	previousProvider := currentFileInfoProvider
-	defer func() { currentFileInfoProvider = previousProvider }()
-	currentFileInfoProvider = mockFiProvider{}
-	windowsCases := []testParseMountRaw{
-		{`c:\:d:`, "local", mount.TypeBind, `d:`, `c:\`, ``, "", true, false},
-		{`c:\:d:\`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", true, false},
-		{`c:\:d:\:ro`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", false, false},
-		{`c:\:d:\:rw`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", true, false},
-		{`c:\:d:\:foo`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", false, true},
-		{`name:d::rw`, "local", mount.TypeVolume, `d:`, ``, `name`, "local", true, false},
-		{`name:d:`, "local", mount.TypeVolume, `d:`, ``, `name`, "local", true, false},
-		{`name:d::ro`, "local", mount.TypeVolume, `d:`, ``, `name`, "local", false, false},
-		{`name:c:`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
-		{`driver/name:c:`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
-		{`\\.\pipe\foo:\\.\pipe\bar`, "local", mount.TypeNamedPipe, `\\.\pipe\bar`, `\\.\pipe\foo`, "", "", true, false},
-		{`\\.\pipe\foo:c:\foo\bar`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
-		{`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
-	}
-	lcowCases := []testParseMountRaw{
-		{`c:\:/foo`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", true, false},
-		{`c:\:/foo:ro`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", false, false},
-		{`c:\:/foo:rw`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", true, false},
-		{`c:\:/foo:foo`, "local", mount.TypeBind, `/foo`, `c:\`, ``, "", false, true},
-		{`name:/foo:rw`, "local", mount.TypeVolume, `/foo`, ``, `name`, "local", true, false},
-		{`name:/foo`, "local", mount.TypeVolume, `/foo`, ``, `name`, "local", true, false},
-		{`name:/foo:ro`, "local", mount.TypeVolume, `/foo`, ``, `name`, "local", false, false},
-		{`name:/`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
-		{`driver/name:/`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
-		{`\\.\pipe\foo:\\.\pipe\bar`, "local", mount.TypeNamedPipe, `\\.\pipe\bar`, `\\.\pipe\foo`, "", "", true, true},
-		{`\\.\pipe\foo:/data`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
-		{`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
-	}
-	linuxCases := []testParseMountRaw{
-		{"/tmp:/tmp1", "", mount.TypeBind, "/tmp1", "/tmp", "", "", true, false},
-		{"/tmp:/tmp2:ro", "", mount.TypeBind, "/tmp2", "/tmp", "", "", false, false},
-		{"/tmp:/tmp3:rw", "", mount.TypeBind, "/tmp3", "/tmp", "", "", true, false},
-		{"/tmp:/tmp4:foo", "", mount.TypeBind, "", "", "", "", false, true},
-		{"name:/named1", "", mount.TypeVolume, "/named1", "", "name", "", true, false},
-		{"name:/named2", "external", mount.TypeVolume, "/named2", "", "name", "external", true, false},
-		{"name:/named3:ro", "local", mount.TypeVolume, "/named3", "", "name", "local", false, false},
-		{"local/name:/tmp:rw", "", mount.TypeVolume, "/tmp", "", "local/name", "", true, false},
-		{"/tmp:tmp", "", mount.TypeBind, "", "", "", "", true, true},
-	}
-	linParser := &linuxParser{}
-	winParser := &windowsParser{}
-	lcowParser := &lcowParser{}
-	tester := func(parser Parser, cases []testParseMountRaw) {
-		for i, c := range cases {
-			t.Logf("case %d", i)
-			m, err := parser.ParseMountRaw(c.bind, c.driver)
-			if c.fail {
-				if err == nil {
-					t.Errorf("Expected error, was nil, for spec %s\n", c.bind)
-				}
-				continue
-			}
-
-			if m == nil || err != nil {
-				t.Errorf("ParseMountRaw failed for spec '%s', driver '%s', error '%v'", c.bind, c.driver, err.Error())
-				continue
-			}
-
-			if m.Destination != c.expDest {
-				t.Errorf("Expected destination '%s, was %s', for spec '%s'", c.expDest, m.Destination, c.bind)
-			}
-
-			if m.Source != c.expSource {
-				t.Errorf("Expected source '%s', was '%s', for spec '%s'", c.expSource, m.Source, c.bind)
-			}
-
-			if m.Name != c.expName {
-				t.Errorf("Expected name '%s', was '%s' for spec '%s'", c.expName, m.Name, c.bind)
-			}
-
-			if m.Driver != c.expDriver {
-				t.Errorf("Expected driver '%s', was '%s', for spec '%s'", c.expDriver, m.Driver, c.bind)
-			}
-
-			if m.RW != c.expRW {
-				t.Errorf("Expected RW '%v', was '%v' for spec '%s'", c.expRW, m.RW, c.bind)
-			}
-			if m.Type != c.expType {
-				t.Fatalf("Expected type '%s', was '%s', for spec '%s'", c.expType, m.Type, c.bind)
-			}
-		}
-	}
-
-	tester(linParser, linuxCases)
-	tester(winParser, windowsCases)
-	tester(lcowParser, lcowCases)
 }
 
 func TestParseMountSpec(t *testing.T) {
-	type c struct {
-		input    mount.Mount
-		expected MountPoint
-	}
 	testDir, err := ioutil.TempDir("", "test-mount-config")
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(testDir)
-	parser := NewParser(runtime.GOOS)
+	parser := NewParser()
-	cases := []c{
+	cases := []struct {
+		input    mount.Mount
+		expected MountPoint
+	}{
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, Propagation: parser.DefaultPropagationMode()}},
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, RW: true, Propagation: parser.DefaultPropagationMode()}},
 		{mount.Mount{Type: mount.TypeBind, Source: testDir + string(os.PathSeparator), Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, Propagation: parser.DefaultPropagationMode()}},
@@ -482,49 +90,3 @@ func TestParseMountSpec(t *testing.T) {
 	}
 
 }
-
-// always returns the configured error
-// this is used to test error handling
-type mockFiProviderWithError struct{ err error }
-
-func (m mockFiProviderWithError) fileInfo(path string) (bool, bool, error) {
-	return false, false, m.err
-}
-
-// TestParseMountSpecBindWithFileinfoError makes sure that the parser returns
-// the error produced by the fileinfo provider.
-//
-// Some extra context for the future in case of changes and possible wtf are we
-// testing this for:
-//
-// Currently this "fileInfoProvider" returns (bool, bool, error)
-// The 1st bool is "does this path exist"
-// The 2nd bool is "is this path a dir"
-// Then of course the error is an error.
-//
-// The issue is the parser was ignoring the error and only looking at the
-// "does this path exist" boolean, which is always false if there is an error.
-// Then the error returned to the caller was a (slightly, maybe) friendlier
-// error string than what comes from `os.Stat`
-// So ...the caller was always getting an error saying the path doesn't exist
-// even if it does exist but got some other error (like a permission error).
-// This is confusing to users.
-func TestParseMountSpecBindWithFileinfoError(t *testing.T) {
-	previousProvider := currentFileInfoProvider
-	defer func() { currentFileInfoProvider = previousProvider }()
-
-	testErr := errors.New("some crazy error")
-	currentFileInfoProvider = &mockFiProviderWithError{err: testErr}
-
-	p := "/bananas"
-	if runtime.GOOS == "windows" {
-		p = `c:\bananas`
-	}
-	m := mount.Mount{Type: mount.TypeBind, Source: p, Target: p}
-
-	parser := NewParser(runtime.GOOS)
-
-	_, err := parser.ParseMountSpec(m)
-	assert.Assert(t, err != nil)
-	assert.Assert(t, cmp.Contains(err.Error(), "some crazy error"))
-}

volume/mounts/validate_test.go

@@ -48,7 +48,7 @@ func TestValidateMount(t *testing.T) {
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: "/foo"}, nil},
 		{mount.Mount{Type: "invalid", Target: "/foo"}, errors.New("mount type unknown")},
 	}
-	parser := NewParser(runtime.GOOS)
+	parser := NewParser()
 	for i, x := range cases {
 		err := parser.ValidateMountConfig(&x.input)
 		if err == nil && x.expected == nil {
@@ -59,7 +59,7 @@ func TestValidateMount(t *testing.T) {
 		}
 	}
 	if runtime.GOOS == "windows" {
-		parser = &lcowParser{}
+		parser = NewLCOWParser()
 		for i, x := range lcowCases {
 			err := parser.ValidateMountConfig(&x.input)
 			if err == nil && x.expected == nil {

volume/mounts/volume_unix.go

@@ -12,7 +12,3 @@ func (p *linuxParser) HasResource(m *MountPoint, absolutePath string) bool {
 	relPath, err := filepath.Rel(m.Destination, absolutePath)
 	return err == nil && relPath != ".." && !strings.HasPrefix(relPath, fmt.Sprintf("..%c", filepath.Separator))
 }
-
-func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool {
-	return false
-}

volume/mounts/volume_windows.go

@@ -1,8 +1,5 @@
 package mounts // import "github.com/docker/docker/volume/mounts"
 
-func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool {
-	return false
-}
 func (p *linuxParser) HasResource(m *MountPoint, absolutePath string) bool {
 	return false
 }

volume/mounts/windows_parser.go

@@ -12,7 +12,15 @@ import (
 	"github.com/docker/docker/pkg/stringid"
 )
 
+// NewWindowsParser creates a parser with Windows semantics.
+func NewWindowsParser() Parser {
+	return &windowsParser{
+		fi: defaultFileInfoProvider{},
+	}
+}
+
 type windowsParser struct {
+	fi fileInfoProvider
 }
 
 const (
@@ -63,12 +71,6 @@ const (
 	// rxDestination is the regex expression for the mount destination
 	rxDestination = `(?P<destination>((?:\\\\\?\\)?([a-z]):((?:[\\/][^\\/:*?"<>\r\n]+)*[\\/]?))|(` + rxPipe + `))`
 
-	rxLCOWDestination = `(?P<destination>/(?:[^\\/:*?"<>\r\n]+[/]?)*)`
-	// Destination (aka container path):
-	//    -  Variation on hostdir but can be a drive followed by colon as well
-	//    -  If a path, must be absolute. Can include spaces
-	//    -  Drive cannot be c: (explicitly checked in code, not RegEx)
-
 	// rxMode is the regex expression for the mode of the mount
 	// Mode (optional):
 	//    -  Hopefully self explanatory in comparison to above regex's.
@@ -78,7 +80,7 @@ const (
 
 type mountValidator func(mnt *mount.Mount) error
 
-func windowsSplitRawSpec(raw, destRegex string) ([]string, error) {
+func (p *windowsParser) windowsSplitRawSpec(raw, destRegex string) ([]string, error) {
 	specExp := regexp.MustCompile(`^` + rxSource + destRegex + rxMode + `$`)
 	match := specExp.FindStringSubmatch(strings.ToLower(raw))
 
@@ -121,8 +123,7 @@ func windowsSplitRawSpec(raw, destRegex string) ([]string, error) {
 				return nil, fmt.Errorf("volume name %q cannot be a reserved word for Windows filenames", matchgroups["destination"])
 			}
 		} else {
-
+			exists, isDir, _ := p.fi.fileInfo(matchgroups["destination"])
-			exists, isDir, _ := currentFileInfoProvider.fileInfo(matchgroups["destination"])
 			if exists && !isDir {
 				return nil, fmt.Errorf("file '%s' cannot be mapped. Only directories can be mapped on this platform", matchgroups["destination"])
 
@@ -136,8 +137,10 @@ func windowsValidMountMode(mode string) bool {
 	if mode == "" {
 		return true
 	}
+	// TODO should windows mounts produce an error if any mode was provided (they're a no-op on windows)
 	return rwModes[strings.ToLower(mode)]
 }
+
 func windowsValidateNotRoot(p string) error {
 	p = strings.ToLower(strings.Replace(p, `/`, `\`, -1))
 	if p == "c:" || p == `c:\` {
@@ -177,7 +180,7 @@ func (p *windowsParser) ReadWrite(mode string) bool {
 	return strings.ToLower(mode) != "ro"
 }
 
-// IsVolumeNameValid checks a volume name in a platform specific manner.
+// ValidateVolumeName checks a volume name in a platform specific manner.
 func (p *windowsParser) ValidateVolumeName(name string) error {
 	nameExp := regexp.MustCompile(`^` + rxName + `$`)
 	if !nameExp.MatchString(name) {
@@ -211,8 +214,6 @@ func (defaultFileInfoProvider) fileInfo(path string) (exist, isDir bool, err err
 	return true, fi.IsDir(), nil
 }
 
-var currentFileInfoProvider fileInfoProvider = defaultFileInfoProvider{}
-
 func (p *windowsParser) validateMountConfigReg(mnt *mount.Mount, destRegex string, additionalValidators ...mountValidator) error {
 
 	for _, v := range additionalValidators {
@@ -247,7 +248,7 @@ func (p *windowsParser) validateMountConfigReg(mnt *mount.Mount, destRegex strin
 			return &errMountConfig{mnt, err}
 		}
 
-		exists, isdir, err := currentFileInfoProvider.fileInfo(mnt.Source)
+		exists, isdir, err := p.fi.fileInfo(mnt.Source)
 		if err != nil {
 			return &errMountConfig{mnt, err}
 		}
@@ -302,7 +303,7 @@ func (p *windowsParser) ParseMountRaw(raw, volumeDriver string) (*MountPoint, er
 }
 
 func (p *windowsParser) parseMountRaw(raw, volumeDriver, destRegex string, convertTargetToBackslash bool, additionalValidators ...mountValidator) (*MountPoint, error) {
-	arr, err := windowsSplitRawSpec(raw, destRegex)
+	arr, err := p.windowsSplitRawSpec(raw, destRegex)
 	if err != nil {
 		return nil, err
 	}
@@ -454,3 +455,7 @@ func (p *windowsParser) IsBackwardCompatible(m *MountPoint) bool {
 func (p *windowsParser) ValidateTmpfsMountDestination(dest string) error {
 	return errors.New("Platform does not support tmpfs")
 }
+
+func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool {
+	return false
+}

volume/mounts/windows_parser_test.go

@@ -0,0 +1,191 @@
+package mounts // import "github.com/docker/docker/volume/mounts"
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/docker/docker/api/types/mount"
+	"gotest.tools/v3/assert"
+)
+
+func TestWindowsParseMountRaw(t *testing.T) {
+	valid := []string{
+		`d:\`,
+		`d:`,
+		`d:\path`,
+		`d:\path with space`,
+		`c:\:d:\`,
+		`c:\windows\:d:`,
+		`c:\windows:d:\s p a c e`,
+		`c:\windows:d:\s p a c e:RW`,
+		`c:\program files:d:\s p a c e i n h o s t d i r`,
+		`0123456789name:d:`,
+		`MiXeDcAsEnAmE:d:`,
+		`name:D:`,
+		`name:D::rW`,
+		`name:D::RW`,
+		`name:D::RO`,
+		`c:/:d:/forward/slashes/are/good/too`,
+		`c:/:d:/including with/spaces:ro`,
+		`c:\Windows`,                // With capital
+		`c:\Program Files (x86)`,    // With capitals and brackets
+		`\\?\c:\windows\:d:`,        // Long path handling (source)
+		`c:\windows\:\\?\d:\`,       // Long path handling (target)
+		`\\.\pipe\foo:\\.\pipe\foo`, // named pipe
+		`//./pipe/foo://./pipe/foo`, // named pipe forward slashes
+	}
+
+	invalid := map[string]string{
+		``:                                 "invalid volume specification: ",
+		`.`:                                "invalid volume specification: ",
+		`..\`:                              "invalid volume specification: ",
+		`c:\:..\`:                          "invalid volume specification: ",
+		`c:\:d:\:xyzzy`:                    "invalid volume specification: ",
+		`c:`:                               "cannot be `c:`",
+		`c:\`:                              "cannot be `c:`",
+		`c:\notexist:d:`:                   `source path does not exist: c:\notexist`,
+		`c:\windows\system32\ntdll.dll:d:`: `source path must be a directory`,
+		`name<:d:`:                         `invalid volume specification`,
+		`name>:d:`:                         `invalid volume specification`,
+		`name::d:`:                         `invalid volume specification`,
+		`name":d:`:                         `invalid volume specification`,
+		`name\:d:`:                         `invalid volume specification`,
+		`name*:d:`:                         `invalid volume specification`,
+		`name|:d:`:                         `invalid volume specification`,
+		`name?:d:`:                         `invalid volume specification`,
+		`name/:d:`:                         `invalid volume specification`,
+		`d:\pathandmode:rw`:                `invalid volume specification`,
+		`d:\pathandmode:ro`:                `invalid volume specification`,
+		`con:d:`:                           `cannot be a reserved word for Windows filenames`,
+		`PRN:d:`:                           `cannot be a reserved word for Windows filenames`,
+		`aUx:d:`:                           `cannot be a reserved word for Windows filenames`,
+		`nul:d:`:                           `cannot be a reserved word for Windows filenames`,
+		`com1:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com2:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com3:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com4:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com5:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com6:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com7:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com8:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`com9:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt1:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt2:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt3:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt4:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt5:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt6:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt7:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt8:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`lpt9:d:`:                          `cannot be a reserved word for Windows filenames`,
+		`c:\windows\system32\ntdll.dll`:    `Only directories can be mapped on this platform`,
+		`\\.\pipe\foo:c:\pipe`:             `'c:\pipe' is not a valid pipe path`,
+	}
+
+	parser := NewWindowsParser()
+	if p, ok := parser.(*windowsParser); ok {
+		p.fi = mockFiProvider{}
+	}
+
+	for _, path := range valid {
+		if _, err := parser.ParseMountRaw(path, "local"); err != nil {
+			t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
+		}
+	}
+
+	for path, expectedError := range invalid {
+		if mp, err := parser.ParseMountRaw(path, "local"); err == nil {
+			t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
+		} else {
+			if !strings.Contains(err.Error(), expectedError) {
+				t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+			}
+		}
+	}
+}
+
+func TestWindowsParseMountRawSplit(t *testing.T) {
+	cases := []struct {
+		bind      string
+		driver    string
+		expType   mount.Type
+		expDest   string
+		expSource string
+		expName   string
+		expDriver string
+		expRW     bool
+		fail      bool
+	}{
+		{`c:\:d:`, "local", mount.TypeBind, `d:`, `c:\`, ``, "", true, false},
+		{`c:\:d:\`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", true, false},
+		{`c:\:d:\:ro`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", false, false},
+		{`c:\:d:\:rw`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", true, false},
+		{`c:\:d:\:foo`, "local", mount.TypeBind, `d:\`, `c:\`, ``, "", false, true},
+		{`name:d::rw`, "local", mount.TypeVolume, `d:`, ``, `name`, "local", true, false},
+		{`name:d:`, "local", mount.TypeVolume, `d:`, ``, `name`, "local", true, false},
+		{`name:d::ro`, "local", mount.TypeVolume, `d:`, ``, `name`, "local", false, false},
+		{`name:c:`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
+		{`driver/name:c:`, "", mount.TypeVolume, ``, ``, ``, "", true, true},
+		{`\\.\pipe\foo:\\.\pipe\bar`, "local", mount.TypeNamedPipe, `\\.\pipe\bar`, `\\.\pipe\foo`, "", "", true, false},
+		{`\\.\pipe\foo:c:\foo\bar`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
+		{`c:\foo\bar:\\.\pipe\foo`, "local", mount.TypeNamedPipe, ``, ``, "", "", true, true},
+	}
+
+	parser := NewWindowsParser()
+	if p, ok := parser.(*windowsParser); ok {
+		p.fi = mockFiProvider{}
+	}
+
+	for i, c := range cases {
+		c := c
+		t.Run(fmt.Sprintf("%d_%s", i, c.bind), func(t *testing.T) {
+			m, err := parser.ParseMountRaw(c.bind, c.driver)
+			if c.fail {
+				assert.ErrorContains(t, err, "", "expected an error")
+				return
+			}
+
+			assert.NilError(t, err)
+			assert.Equal(t, m.Destination, c.expDest)
+			assert.Equal(t, m.Source, c.expSource)
+			assert.Equal(t, m.Name, c.expName)
+			assert.Equal(t, m.Driver, c.expDriver)
+			assert.Equal(t, m.RW, c.expRW)
+			assert.Equal(t, m.Type, c.expType)
+		})
+	}
+}
+
+// TestWindowsParseMountSpecBindWithFileinfoError makes sure that the parser returns
+// the error produced by the fileinfo provider.
+//
+// Some extra context for the future in case of changes and possible wtf are we
+// testing this for:
+//
+// Currently this "fileInfoProvider" returns (bool, bool, error)
+// The 1st bool is "does this path exist"
+// The 2nd bool is "is this path a dir"
+// Then of course the error is an error.
+//
+// The issue is the parser was ignoring the error and only looking at the
+// "does this path exist" boolean, which is always false if there is an error.
+// Then the error returned to the caller was a (slightly, maybe) friendlier
+// error string than what comes from `os.Stat`
+// So ...the caller was always getting an error saying the path doesn't exist
+// even if it does exist but got some other error (like a permission error).
+// This is confusing to users.
+func TestWindowsParseMountSpecBindWithFileinfoError(t *testing.T) {
+	parser := NewWindowsParser()
+	testErr := fmt.Errorf("some crazy error")
+	if pr, ok := parser.(*windowsParser); ok {
+		pr.fi = &mockFiProviderWithError{err: testErr}
+	}
+
+	_, err := parser.ParseMountSpec(mount.Mount{
+		Type:   mount.TypeBind,
+		Source: `c:\bananas`,
+		Target: `c:\bananas`,
+	})
+	assert.ErrorContains(t, err, testErr.Error())
+}

volume/service/store.go

@@ -6,7 +6,6 @@ import (
 	"net"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sync"
 	"time"
 
@@ -578,8 +577,7 @@ func (s *VolumeStore) create(ctx context.Context, name, driverName string, opts,
 	// Validate the name in a platform-specific manner
 
 	// volume name validation is specific to the host os and not on container image
-	// windows/lcow should have an equivalent volumename validation logic so we create a parser for current host OS
+	parser := volumemounts.NewParser()
-	parser := volumemounts.NewParser(runtime.GOOS)
 	err := parser.ValidateVolumeName(name)
 	if err != nil {
 		return nil, false, err