opts: MountOpt: improve validation of boolean values

thaJeztah · thaJeztah · commit fe1af9206c88 · 2026-02-02T15:29:23.000+01:00
Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
diff --git a/opts/mount.go b/opts/mount.go
@@ -97,14 +97,8 @@ func (m *MountOpt) Set(value string) error {
 
 		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)
 			}
@@ -123,9 +117,9 @@ 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))
@@ -151,9 +145,9 @@ func (m *MountOpt) Set(value string) error {
 		case "volume-subpath":
 			volumeOptions().Subpath = val
 		case "volume-nocopy":
-			volumeOptions().NoCopy, err = strconv.ParseBool(val)
+			volumeOptions().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)
diff --git a/opts/mount_test.go b/opts/mount_test.go
@@ -188,7 +188,7 @@ func TestMountOptReadOnly(t *testing.T) {
 		{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: no`},
+		{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},
@@ -232,7 +232,7 @@ func TestMountOptVolumeNoCopy(t *testing.T) {
 		{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: no`},
+		{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},
diff --git a/opts/mount_utils.go b/opts/mount_utils.go
@@ -0,0 +1,24 @@
+package opts
+
+import (
+	"fmt"
+)
+
+// 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)
+	}
+}
