Merge pull request #6772 from thaJeztah/cleanup_testfile

cli/command: TestGetDefaultAuthConfig: cleanup test file

cli/command/registry_test.go

@@ -3,6 +3,7 @@ package command_test
 import (
 	"bytes"
 	"path"
+	"path/filepath"
 	"testing"
 
 	"github.com/docker/cli/cli/command"
@@ -58,7 +59,9 @@ func TestGetDefaultAuthConfig(t *testing.T) {
 			expectedAuthConfig: testAuthConfigs[1],
 		},
 	}
-	cfg := configfile.New("filename")
+
+	tmpDir := t.TempDir()
+	cfg := configfile.New(filepath.Join(tmpDir, "cli-config.json"))
 	for _, authConfig := range testAuthConfigs {
 		assert.Check(t, cfg.GetCredentialsStore(authConfig.ServerAddress).Store(configtypes.AuthConfig{
 			Username:      authConfig.Username,

opts/mount.go

@@ -22,72 +22,43 @@ type MountOpt struct {
 //
 //nolint:gocyclo
 func (m *MountOpt) Set(value string) error {
+	value = strings.TrimSpace(value)
+	if value == "" {
+		return errors.New("value is empty")
+	}
+
 	csvReader := csv.NewReader(strings.NewReader(value))
 	fields, err := csvReader.Read()
 	if err != nil {
 		return err
 	}
 
-	mount := mounttypes.Mount{}
+	mount := mounttypes.Mount{
+		Type: mounttypes.TypeVolume, // default to volume mounts
+	}
 
-	volumeOptions := func() *mounttypes.VolumeOptions {
-		if mount.VolumeOptions == nil {
-			mount.VolumeOptions = &mounttypes.VolumeOptions{
-				Labels: make(map[string]string),
+	for _, field := range fields {
+		key, val, hasValue := strings.Cut(field, "=")
+		if k := strings.TrimSpace(key); k != key {
+			return fmt.Errorf("invalid option '%s' in '%s': option should not have whitespace", k, field)
+		}
+		if hasValue {
+			v := strings.TrimSpace(val)
+			if v == "" {
+				return fmt.Errorf("invalid value for '%s': value is empty", key)
+			}
+			if v != val {
+				return fmt.Errorf("invalid value for '%s' in '%s': value should not have whitespace", key, field)
 			}
 		}
-		if mount.VolumeOptions.DriverConfig == nil {
-			mount.VolumeOptions.DriverConfig = &mounttypes.Driver{}
-		}
-		return mount.VolumeOptions
-	}
-
-	imageOptions := func() *mounttypes.ImageOptions {
-		if mount.ImageOptions == nil {
-			mount.ImageOptions = new(mounttypes.ImageOptions)
-		}
-		return mount.ImageOptions
-	}
-
-	bindOptions := func() *mounttypes.BindOptions {
-		if mount.BindOptions == nil {
-			mount.BindOptions = new(mounttypes.BindOptions)
-		}
-		return mount.BindOptions
-	}
-
-	tmpfsOptions := func() *mounttypes.TmpfsOptions {
-		if mount.TmpfsOptions == nil {
-			mount.TmpfsOptions = new(mounttypes.TmpfsOptions)
-		}
-		return mount.TmpfsOptions
-	}
-
-	setValueOnMap := func(target map[string]string, value string) {
-		k, v, _ := strings.Cut(value, "=")
-		if k != "" {
-			target[k] = v
-		}
-	}
-
-	mount.Type = mounttypes.TypeVolume // default to volume mounts
-	// Set writable as the default
-	for _, field := range fields {
-		key, val, ok := strings.Cut(field, "=")
 
 		// TODO(thaJeztah): these options should not be case-insensitive.
 		key = strings.ToLower(key)
 
-		if !ok {
+		if !hasValue {
 			switch key {
-			case "readonly", "ro":
-				mount.ReadOnly = true
-				continue
-			case "volume-nocopy":
-				volumeOptions().NoCopy = true
-				continue
-			case "bind-nonrecursive":
-				return errors.New("bind-nonrecursive is deprecated, use bind-recursive=disabled instead")
+			case "readonly", "ro", "volume-nocopy", "bind-nonrecursive":
+				// boolean values
 			default:
 				return fmt.Errorf("invalid field '%s' must be a key=value pair", field)
 			}
@@ -106,14 +77,14 @@ func (m *MountOpt) Set(value string) error {
 		case "target", "dst", "destination":
 			mount.Target = val
 		case "readonly", "ro":
-			mount.ReadOnly, err = strconv.ParseBool(val)
+			mount.ReadOnly, err = parseBoolValue(key, val, hasValue)
 			if err != nil {
-				return fmt.Errorf("invalid value for %s: %s", key, val)
+				return err
 			}
 		case "consistency":
 			mount.Consistency = mounttypes.Consistency(strings.ToLower(val))
 		case "bind-propagation":
-			bindOptions().Propagation = mounttypes.Propagation(strings.ToLower(val))
+			ensureBindOptions(&mount).Propagation = mounttypes.Propagation(strings.ToLower(val))
 		case "bind-nonrecursive":
 			return errors.New("bind-nonrecursive is deprecated, use bind-recursive=disabled instead")
 		case "bind-recursive":
@@ -121,86 +92,52 @@ func (m *MountOpt) Set(value string) error {
 			case "enabled": // read-only mounts are recursively read-only if Engine >= v25 && kernel >= v5.12, otherwise writable
 				// NOP
 			case "disabled": // previously "bind-nonrecursive=true"
-				bindOptions().NonRecursive = true
+				ensureBindOptions(&mount).NonRecursive = true
 			case "writable": // conforms to the default read-only bind-mount of Docker v24; read-only mounts are recursively mounted but not recursively read-only
-				bindOptions().ReadOnlyNonRecursive = true
+				ensureBindOptions(&mount).ReadOnlyNonRecursive = true
 			case "readonly": // force recursively read-only, or raise an error
-				bindOptions().ReadOnlyForceRecursive = true
+				ensureBindOptions(&mount).ReadOnlyForceRecursive = true
 				// TODO: implicitly set propagation and error if the user specifies a propagation in a future refactor/UX polish pass
 				// https://github.com/docker/cli/pull/4316#discussion_r1341974730
 			default:
 				return fmt.Errorf(`invalid value for %s: %s (must be "enabled", "disabled", "writable", or "readonly")`, key, val)
 			}
 		case "volume-subpath":
-			volumeOptions().Subpath = val
+			ensureVolumeOptions(&mount).Subpath = val
 		case "volume-nocopy":
-			volumeOptions().NoCopy, err = strconv.ParseBool(val)
+			ensureVolumeOptions(&mount).NoCopy, err = parseBoolValue(key, val, hasValue)
 			if err != nil {
-				return fmt.Errorf("invalid value for volume-nocopy: %s", val)
+				return err
 			}
 		case "volume-label":
-			setValueOnMap(volumeOptions().Labels, val)
+			volumeOpts := ensureVolumeOptions(&mount)
+			volumeOpts.Labels = setValueOnMap(volumeOpts.Labels, val)
 		case "volume-driver":
-			volumeOptions().DriverConfig.Name = val
+			ensureVolumeDriver(&mount).Name = val
 		case "volume-opt":
-			if volumeOptions().DriverConfig.Options == nil {
-				volumeOptions().DriverConfig.Options = make(map[string]string)
-			}
-			setValueOnMap(volumeOptions().DriverConfig.Options, val)
+			volumeDriver := ensureVolumeDriver(&mount)
+			volumeDriver.Options = setValueOnMap(volumeDriver.Options, val)
 		case "image-subpath":
-			imageOptions().Subpath = val
+			ensureImageOptions(&mount).Subpath = val
 		case "tmpfs-size":
 			sizeBytes, err := units.RAMInBytes(val)
 			if err != nil {
 				return fmt.Errorf("invalid value for %s: %s", key, val)
 			}
-			tmpfsOptions().SizeBytes = sizeBytes
+			ensureTmpfsOptions(&mount).SizeBytes = sizeBytes
 		case "tmpfs-mode":
 			ui64, err := strconv.ParseUint(val, 8, 32)
 			if err != nil {
 				return fmt.Errorf("invalid value for %s: %s", key, val)
 			}
-			tmpfsOptions().Mode = os.FileMode(ui64)
+			ensureTmpfsOptions(&mount).Mode = os.FileMode(ui64)
 		default:
-			return fmt.Errorf("unexpected key '%s' in '%s'", key, field)
+			return fmt.Errorf("unknown option '%s' in '%s'", key, field)
 		}
 	}
 
-	if mount.Type == "" {
-		return errors.New("type is required")
-	}
-
-	if mount.Target == "" {
-		return errors.New("target is required")
-	}
-
-	if mount.VolumeOptions != nil && mount.Type != mounttypes.TypeVolume {
-		return fmt.Errorf("cannot mix 'volume-*' options with mount type '%s'", mount.Type)
-	}
-	if mount.ImageOptions != nil && mount.Type != mounttypes.TypeImage {
-		return fmt.Errorf("cannot mix 'image-*' options with mount type '%s'", mount.Type)
-	}
-	if mount.BindOptions != nil && mount.Type != mounttypes.TypeBind {
-		return fmt.Errorf("cannot mix 'bind-*' options with mount type '%s'", mount.Type)
-	}
-	if mount.TmpfsOptions != nil && mount.Type != mounttypes.TypeTmpfs {
-		return fmt.Errorf("cannot mix 'tmpfs-*' options with mount type '%s'", mount.Type)
-	}
-
-	if mount.BindOptions != nil {
-		if mount.BindOptions.ReadOnlyNonRecursive {
-			if !mount.ReadOnly {
-				return errors.New("option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction")
-			}
-		}
-		if mount.BindOptions.ReadOnlyForceRecursive {
-			if !mount.ReadOnly {
-				return errors.New("option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction")
-			}
-			if mount.BindOptions.Propagation != mounttypes.PropagationRPrivate {
-				return errors.New("option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction")
-			}
-		}
+	if err := validateMountOptions(&mount); err != nil {
+		return err
 	}
 
 	m.values = append(m.values, mount)

opts/mount_test.go

@@ -73,14 +73,13 @@ func TestMountRelative(t *testing.T) {
 	}
 }
 
-// TestMountOptSetBindNoErrorBind tests several aliases that should have
+// TestMountOptSourceTargetAliases tests several aliases that should have
 // the same result.
-func TestMountOptSetBindNoErrorBind(t *testing.T) {
+func TestMountOptSourceTargetAliases(t *testing.T) {
 	for _, tc := range []string{
-		"type=bind,target=/target,source=/source",
 		"type=bind,src=/source,dst=/target",
-		"type=bind,source=/source,dst=/target",
-		"type=bind,src=/source,target=/target",
+		"type=bind,source=/source,target=/target",
+		"type=bind,source=/source,destination=/target",
 	} {
 		t.Run(tc, func(t *testing.T) {
 			var m MountOpt
@@ -98,31 +97,6 @@ func TestMountOptSetBindNoErrorBind(t *testing.T) {
 	}
 }
 
-// TestMountOptSetVolumeNoError tests several aliases that should have
-// the same result.
-func TestMountOptSetVolumeNoError(t *testing.T) {
-	for _, tc := range []string{
-		"type=volume,target=/target,source=/source",
-		"type=volume,src=/source,dst=/target",
-		"type=volume,source=/source,dst=/target",
-		"type=volume,src=/source,target=/target",
-	} {
-		t.Run(tc, func(t *testing.T) {
-			var m MountOpt
-
-			assert.NilError(t, m.Set(tc))
-
-			mounts := m.Value()
-			assert.Assert(t, is.Len(mounts, 1))
-			assert.Check(t, is.DeepEqual(mount.Mount{
-				Type:   mount.TypeVolume,
-				Source: "/source",
-				Target: "/target",
-			}, mounts[0]))
-		})
-	}
-}
-
 // TestMountOptDefaultType ensures that a mount without the type defaults to a
 // volume mount.
 func TestMountOptDefaultType(t *testing.T) {
@@ -131,78 +105,327 @@ func TestMountOptDefaultType(t *testing.T) {
 	assert.Check(t, is.Equal(mount.TypeVolume, m.values[0].Type))
 }
 
-func TestMountOptSetErrorNoTarget(t *testing.T) {
-	var m MountOpt
-	assert.Error(t, m.Set("type=volume,source=/foo"), "target is required")
+func TestMountOptErrors(t *testing.T) {
+	tests := []struct {
+		doc, value, expErr string
+	}{
+		{
+			doc:    "empty value",
+			expErr: "value is empty",
+		},
+		{
+			doc:    "invalid key=value",
+			value:  "type=volume,target=/foo,bogus=foo",
+			expErr: "unknown option 'bogus' in 'bogus=foo'",
+		},
+		{
+			doc:    "invalid key with leading whitespace",
+			value:  "type=volume, src=/foo,target=/foo",
+			expErr: "invalid option 'src' in ' src=/foo': option should not have whitespace",
+		},
+		{
+			doc:    "invalid key with trailing whitespace",
+			value:  "type=volume,src =/foo,target=/foo",
+			expErr: "invalid option 'src' in 'src =/foo': option should not have whitespace",
+		},
+		{
+			doc:    "invalid value is empty",
+			value:  "type=volume,src=,target=/foo",
+			expErr: "invalid value for 'src': value is empty",
+		},
+		{
+			doc:    "invalid value with leading whitespace",
+			value:  "type=volume,src= /foo,target=/foo",
+			expErr: "invalid value for 'src' in 'src= /foo': value should not have whitespace",
+		},
+		{
+			doc:    "invalid value with trailing whitespace",
+			value:  "type=volume,src=/foo ,target=/foo",
+			expErr: "invalid value for 'src' in 'src=/foo ': value should not have whitespace",
+		},
+		{
+			doc:    "missing value",
+			value:  "type=volume,target=/foo,bogus",
+			expErr: "invalid field 'bogus' must be a key=value pair",
+		},
+		{
+			doc:    "invalid tmpfs-size",
+			value:  "type=tmpfs,target=/foo,tmpfs-size=foo",
+			expErr: "invalid value for tmpfs-size: foo",
+		},
+		{
+			doc:    "invalid tmpfs-mode",
+			value:  "type=tmpfs,target=/foo,tmpfs-mode=foo",
+			expErr: "invalid value for tmpfs-mode: foo",
+		},
+		{
+			doc:    "mixed bind and volume",
+			value:  "type=volume,target=/foo,source=/foo,bind-propagation=rprivate",
+			expErr: "cannot mix 'bind-*' options with mount type 'volume'",
+		},
+		{
+			doc:    "mixed volume and bind",
+			value:  "type=bind,target=/foo,source=/foo,volume-nocopy=true",
+			expErr: "cannot mix 'volume-*' options with mount type 'bind'",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			err := (&MountOpt{}).Set(tc.value)
+			assert.Error(t, err, tc.expErr)
+		})
+	}
 }
 
-func TestMountOptSetErrorInvalidKey(t *testing.T) {
-	var m MountOpt
-	assert.Error(t, m.Set("type=volume,bogus=foo"), "unexpected key 'bogus' in 'bogus=foo'")
-}
+func TestMountOptReadOnly(t *testing.T) {
+	tests := []struct {
+		value  string
+		exp    bool
+		expErr string
+	}{
+		{value: "", exp: false},
+		{value: "readonly", exp: true},
+		{value: "readonly=", expErr: `invalid value for 'readonly': value is empty`},
+		{value: "readonly= true", expErr: `invalid value for 'readonly' in 'readonly= true': value should not have whitespace`},
+		{value: "readonly=no", expErr: `invalid value for 'readonly': invalid boolean value ("no"): must be one of "true", "1", "false", or "0" (default "true")`},
+		{value: "readonly=1", exp: true},
+		{value: "readonly=true", exp: true},
+		{value: "readonly=0", exp: false},
+		{value: "readonly=false", exp: false},
+		{value: "ro", exp: true},
+		{value: "ro=1", exp: true},
+		{value: "ro=true", exp: true},
+		{value: "ro=0", exp: false},
+		{value: "ro=false", exp: false},
+	}
 
-func TestMountOptSetErrorInvalidField(t *testing.T) {
-	var m MountOpt
-	assert.Error(t, m.Set("type=volume,bogus"), "invalid field 'bogus' must be a key=value pair")
-}
-
-func TestMountOptSetErrorInvalidReadOnly(t *testing.T) {
-	var m MountOpt
-	assert.Error(t, m.Set("type=volume,readonly=no"), "invalid value for readonly: no")
-	assert.Error(t, m.Set("type=volume,readonly=invalid"), "invalid value for readonly: invalid")
-}
-
-func TestMountOptDefaultEnableReadOnly(t *testing.T) {
-	var m MountOpt
-	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo"))
-	assert.Check(t, !m.values[0].ReadOnly)
-
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo,readonly"))
-	assert.Check(t, m.values[0].ReadOnly)
-
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo,readonly=1"))
-	assert.Check(t, m.values[0].ReadOnly)
-
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo,readonly=true"))
-	assert.Check(t, m.values[0].ReadOnly)
-
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo,readonly=0"))
-	assert.Check(t, !m.values[0].ReadOnly)
+	for _, tc := range tests {
+		name := tc.value
+		if name == "" {
+			name = "not set"
+		}
+		t.Run(name, func(t *testing.T) {
+			val := "type=bind,target=/foo,source=/foo"
+			if tc.value != "" {
+				val += "," + tc.value
+			}
+			var m MountOpt
+			err := m.Set(val)
+			if tc.expErr != "" {
+				assert.Error(t, err, tc.expErr)
+				return
+			}
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(m.values[0].ReadOnly, tc.exp))
+		})
+	}
 }
 
 func TestMountOptVolumeNoCopy(t *testing.T) {
-	var m MountOpt
-	assert.NilError(t, m.Set("type=volume,target=/foo,volume-nocopy"))
-	assert.Check(t, is.Equal("", m.values[0].Source))
+	tests := []struct {
+		value  string
+		exp    bool
+		expErr string
+	}{
+		{value: "", exp: false},
+		{value: "volume-nocopy", exp: true},
+		{value: "volume-nocopy=", expErr: `invalid value for 'volume-nocopy': value is empty`},
+		{value: "volume-nocopy= true", expErr: `invalid value for 'volume-nocopy' in 'volume-nocopy= true': value should not have whitespace`},
+		{value: "volume-nocopy=no", expErr: `invalid value for 'volume-nocopy': invalid boolean value ("no"): must be one of "true", "1", "false", or "0" (default "true")`},
+		{value: "volume-nocopy=1", exp: true},
+		{value: "volume-nocopy=true", exp: true},
+		{value: "volume-nocopy=0", exp: false},
+		{value: "volume-nocopy=false", exp: false},
+	}
 
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo"))
-	assert.Check(t, m.values[0].VolumeOptions == nil)
-
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo,volume-nocopy=true"))
-	assert.Check(t, m.values[0].VolumeOptions != nil)
-	assert.Check(t, m.values[0].VolumeOptions.NoCopy)
-
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo,volume-nocopy"))
-	assert.Check(t, m.values[0].VolumeOptions != nil)
-	assert.Check(t, m.values[0].VolumeOptions.NoCopy)
-
-	m = MountOpt{}
-	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo,volume-nocopy=1"))
-	assert.Check(t, m.values[0].VolumeOptions != nil)
-	assert.Check(t, m.values[0].VolumeOptions.NoCopy)
+	for _, tc := range tests {
+		name := tc.value
+		if name == "" {
+			name = "not set"
+		}
+		t.Run(name, func(t *testing.T) {
+			val := "type=volume,target=/foo,source=foo"
+			if tc.value != "" {
+				val += "," + tc.value
+			}
+			var m MountOpt
+			err := m.Set(val)
+			if tc.expErr != "" {
+				assert.Error(t, err, tc.expErr)
+				return
+			}
+			assert.NilError(t, err)
+			if tc.value == "" {
+				assert.Check(t, is.Nil(m.values[0].VolumeOptions))
+			} else {
+				assert.Check(t, m.values[0].VolumeOptions != nil)
+				assert.Check(t, is.Equal(m.values[0].VolumeOptions.NoCopy, tc.exp))
+			}
+		})
+	}
 }
 
-func TestMountOptTypeConflict(t *testing.T) {
-	var m MountOpt
-	assert.ErrorContains(t, m.Set("type=bind,target=/foo,source=/foo,volume-nocopy=true"), "cannot mix")
-	assert.ErrorContains(t, m.Set("type=volume,target=/foo,source=/foo,bind-propagation=rprivate"), "cannot mix")
+func TestMountOptVolumeOptions(t *testing.T) {
+	tests := []struct {
+		doc   string
+		value string
+		exp   mount.Mount
+	}{
+		{
+			doc:   "volume-label single",
+			value: `type=volume,target=/foo,volume-label=foo=foo-value`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					Labels: map[string]string{
+						"foo": "foo-value",
+					},
+				},
+			},
+		},
+		{
+			doc:   "volume-label multiple",
+			value: `type=volume,target=/foo,volume-label=foo=foo-value,volume-label=bar=bar-value`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					Labels: map[string]string{
+						"foo": "foo-value",
+						"bar": "bar-value",
+					},
+				},
+			},
+		},
+		{
+			doc:   "volume-label empty values",
+			value: `type=volume,target=/foo,volume-label=foo=,volume-label=bar`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					Labels: map[string]string{
+						"foo": "",
+						"bar": "",
+					},
+				},
+			},
+		},
+		{
+			// TODO(thaJeztah): this should probably be an error instead
+			doc:   "volume-label empty key",
+			value: `type=volume,target=/foo,volume-label==foo-value`,
+			exp: mount.Mount{
+				Type:          mount.TypeVolume,
+				Target:        "/foo",
+				VolumeOptions: &mount.VolumeOptions{},
+			},
+		},
+		{
+			doc:   "volume-driver",
+			value: `type=volume,target=/foo,volume-driver=my-driver`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					DriverConfig: &mount.Driver{
+						Name: "my-driver",
+					},
+				},
+			},
+		},
+		{
+			doc:   "volume-opt single",
+			value: `type=volume,target=/foo,volume-opt=foo=foo-value`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					DriverConfig: &mount.Driver{
+						Options: map[string]string{
+							"foo": "foo-value",
+						},
+					},
+				},
+			},
+		},
+		{
+			doc:   "volume-opt multiple",
+			value: `type=volume,target=/foo,volume-opt=foo=foo-value,volume-opt=bar=bar-value`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					DriverConfig: &mount.Driver{
+						Options: map[string]string{
+							"foo": "foo-value",
+							"bar": "bar-value",
+						},
+					},
+				},
+			},
+		},
+		{
+			doc:   "volume-opt empty values",
+			value: `type=volume,target=/foo,volume-opt=foo=,volume-opt=bar`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					DriverConfig: &mount.Driver{
+						Options: map[string]string{
+							"foo": "",
+							"bar": "",
+						},
+					},
+				},
+			},
+		},
+		{
+			// TODO(thaJeztah): this should probably be an error instead
+			doc:   "volume-opt empty key",
+			value: `type=volume,target=/foo,volume-opt==foo-value`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					DriverConfig: &mount.Driver{},
+				},
+			},
+		},
+		{
+			doc:   "volume-label and volume-opt",
+			value: `type=volume,volume-driver=my-driver,target=/foo,volume-label=foo=foo-value,volume-label=empty=,volume-opt=foo=foo-value,volume-opt=empty=`,
+			exp: mount.Mount{
+				Type:   mount.TypeVolume,
+				Target: "/foo",
+				VolumeOptions: &mount.VolumeOptions{
+					Labels: map[string]string{
+						"foo":   "foo-value",
+						"empty": "",
+					},
+					DriverConfig: &mount.Driver{
+						Name: "my-driver",
+						Options: map[string]string{
+							"foo":   "foo-value",
+							"empty": "",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			var m MountOpt
+
+			assert.NilError(t, m.Set(tc.value))
+			assert.Check(t, is.DeepEqual(m.values[0], tc.exp))
+		})
+	}
 }
 
 func TestMountOptSetImageNoError(t *testing.T) {
@@ -252,13 +475,6 @@ func TestMountOptSetTmpfsNoError(t *testing.T) {
 	}
 }
 
-func TestMountOptSetTmpfsError(t *testing.T) {
-	var m MountOpt
-	assert.ErrorContains(t, m.Set("type=tmpfs,target=/foo,tmpfs-size=foo"), "invalid value for tmpfs-size")
-	assert.ErrorContains(t, m.Set("type=tmpfs,target=/foo,tmpfs-mode=foo"), "invalid value for tmpfs-mode")
-	assert.ErrorContains(t, m.Set("type=tmpfs"), "target is required")
-}
-
 func TestMountOptSetBindRecursive(t *testing.T) {
 	t.Run("enabled", func(t *testing.T) {
 		var m MountOpt

opts/mount_utils.go

@@ -0,0 +1,135 @@
+package opts
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/moby/moby/api/types/mount"
+)
+
+// validateMountOptions performs client-side validation of mount options. Similar
+// validation happens on the daemon side, but this validation allows us to
+// produce user-friendly errors matching command-line options.
+func validateMountOptions(m *mount.Mount) error {
+	if err := validateExclusiveOptions(m); err != nil {
+		return err
+	}
+
+	if m.BindOptions != nil {
+		if m.BindOptions.ReadOnlyNonRecursive && !m.ReadOnly {
+			return errors.New("option 'bind-recursive=writable' requires 'readonly' to be specified in conjunction")
+		}
+		if m.BindOptions.ReadOnlyForceRecursive {
+			if !m.ReadOnly {
+				return errors.New("option 'bind-recursive=readonly' requires 'readonly' to be specified in conjunction")
+			}
+			if m.BindOptions.Propagation != mount.PropagationRPrivate {
+				// FIXME(thaJeztah): this is missing daemon-side validation
+				//
+				//	docker run --rm --mount type=bind,src=/var/run,target=/foo,bind-recursive=readonly,readonly alpine
+				//	# no error
+				return errors.New("option 'bind-recursive=readonly' requires 'bind-propagation=rprivate' to be specified in conjunction")
+			}
+		}
+	}
+
+	return nil
+}
+
+// validateExclusiveOptions checks if the given mount config only contains
+// options for the given mount-type.
+//
+// This is the client-side equivalent of [mounts.validateExclusiveOptions] in
+// the daemon, but with error-messages matching client-side flags / options.
+//
+// [mounts.validateExclusiveOptions]: https://github.com/moby/moby/blob/v2.0.0-beta.6/daemon/volume/mounts/validate.go#L31-L50
+func validateExclusiveOptions(m *mount.Mount) error {
+	if m.Type == "" {
+		return errors.New("type is required")
+	}
+
+	if m.Type != mount.TypeBind && m.BindOptions != nil {
+		return fmt.Errorf("cannot mix 'bind-*' options with mount type '%s'", m.Type)
+	}
+	if m.Type != mount.TypeVolume && m.VolumeOptions != nil {
+		return fmt.Errorf("cannot mix 'volume-*' options with mount type '%s'", m.Type)
+	}
+	if m.Type != mount.TypeImage && m.ImageOptions != nil {
+		return fmt.Errorf("cannot mix 'image-*' options with mount type '%s'", m.Type)
+	}
+	if m.Type != mount.TypeTmpfs && m.TmpfsOptions != nil {
+		return fmt.Errorf("cannot mix 'tmpfs-*' options with mount type '%s'", m.Type)
+	}
+	if m.Type != mount.TypeCluster && m.ClusterOptions != nil {
+		return fmt.Errorf("cannot mix 'cluster-*' options with mount type '%s'", m.Type)
+	}
+	return nil
+}
+
+// parseBoolValue returns the boolean value represented by the string. It returns
+// true if no value is set.
+//
+// It is similar to [strconv.ParseBool], but only accepts 1, true, 0, false.
+// Any other value returns an error.
+func parseBoolValue(key string, val string, hasValue bool) (bool, error) {
+	if !hasValue {
+		return true, nil
+	}
+	switch val {
+	case "1", "true":
+		return true, nil
+	case "0", "false":
+		return false, nil
+	default:
+		return false, fmt.Errorf(`invalid value for '%s': invalid boolean value (%q): must be one of "true", "1", "false", or "0" (default "true")`, key, val)
+	}
+}
+
+func ensureVolumeOptions(m *mount.Mount) *mount.VolumeOptions {
+	if m.VolumeOptions == nil {
+		m.VolumeOptions = &mount.VolumeOptions{}
+	}
+	return m.VolumeOptions
+}
+
+func ensureVolumeDriver(m *mount.Mount) *mount.Driver {
+	ensureVolumeOptions(m)
+	if m.VolumeOptions.DriverConfig == nil {
+		m.VolumeOptions.DriverConfig = &mount.Driver{}
+	}
+	return m.VolumeOptions.DriverConfig
+}
+
+func ensureImageOptions(m *mount.Mount) *mount.ImageOptions {
+	if m.ImageOptions == nil {
+		m.ImageOptions = &mount.ImageOptions{}
+	}
+	return m.ImageOptions
+}
+
+func ensureBindOptions(m *mount.Mount) *mount.BindOptions {
+	if m.BindOptions == nil {
+		m.BindOptions = &mount.BindOptions{}
+	}
+	return m.BindOptions
+}
+
+func ensureTmpfsOptions(m *mount.Mount) *mount.TmpfsOptions {
+	if m.TmpfsOptions == nil {
+		m.TmpfsOptions = &mount.TmpfsOptions{}
+	}
+	return m.TmpfsOptions
+}
+
+func setValueOnMap(target map[string]string, keyValue string) map[string]string {
+	k, v, _ := strings.Cut(keyValue, "=")
+	if k == "" {
+		return target
+	}
+	if target == nil {
+		target = map[string]string{}
+	}
+	target[k] = v
+	return target
+}