Merge pull request #4992 from mayur-tolexo/fix/mount-remove-rw

fix(mount): remove the non-Docker `rw` option from --mount

Author: AkihiroSuda

docs/command-reference.md

@@ -302,7 +302,8 @@ Volume flags:
   - Common Options:
     - :whale: `src`, `source`: Mount source spec for bind and volume. Mandatory for bind.
     - :whale: `dst`, `destination`, `target`: Mount destination spec.
-    - :whale: `readonly`, `ro`, `rw`, `rro`: Filesystem permissions.
+    - :whale: `readonly`, `ro`: mount the filesystem read-only.
+    - :nerd_face: `rro`: mount the filesystem recursively read-only.
   - Options specific to `bind`:
     - :whale: `bind-propagation`: `shared`, `slave`, `private`, `rshared`, `rslave`, or `rprivate`(default).
     - :whale: `bind-nonrecursive`: `true` or `false`(default). If set to true, submounts are not recursively bind-mounted. This option is useful for readonly bind mount.

pkg/mountutil/mountutil_linux.go

@@ -332,7 +332,7 @@ func ProcessFlagMount(s string, volStore volumestore.VolumeStore) (*Processed, e
 
 		if len(parts) == 1 {
 			switch key {
-			case "readonly", "ro", "rw", "rro":
+			case "readonly", "ro", "rro":
 				rwOption = key
 				continue
 			case "bind-nonrecursive":
@@ -361,7 +361,7 @@ func ProcessFlagMount(s string, volStore volumestore.VolumeStore) (*Processed, e
 			src = value
 		case "target", "dst", "destination":
 			dst = value
-		case "readonly", "ro", "rw", "rro":
+		case "readonly", "ro", "rro":
 			trueValue, err := strconv.ParseBool(value)
 			if err != nil {
 				return nil, fmt.Errorf("invalid value for %s: %s", key, value)

pkg/mountutil/mountutil_linux_test.go

@@ -352,3 +352,66 @@ func TestProcessFlagVAnonymousVolumes(t *testing.T) {
 		})
 	}
 }
+
+// TestProcessFlagMountRW verifies that the non-Docker `rw` option is no longer
+// accepted by --mount, while ro/readonly/rro remain valid read-only flags.
+func TestProcessFlagMountRW(t *testing.T) {
+	// rw is no longer a valid --mount option.
+	rejected := []struct {
+		spec string
+		want string
+	}{
+		{"type=bind,source=/foo,target=/bar,rw", "must be a key=value pair"},
+		{"type=bind,source=/foo,target=/bar,rw=true", "unexpected key 'rw'"},
+		{"type=bind,source=/foo,target=/bar,rw=false", "unexpected key 'rw'"},
+	}
+	for _, tt := range rejected {
+		t.Run(tt.spec, func(t *testing.T) {
+			_, err := ProcessFlagMount(tt.spec, nil)
+			assert.ErrorContains(t, err, tt.want)
+		})
+	}
+
+	// ro/rro still parse into a complete read-only bind mount.
+	src := t.TempDir()
+	accepted := []struct {
+		spec  string
+		wants *Processed
+	}{
+		{
+			spec: "type=bind,source=" + src + ",target=/bar,ro",
+			wants: &Processed{
+				Type: Bind,
+				Mount: specs.Mount{
+					Type:        "bind",
+					Source:      src,
+					Destination: "/bar",
+					Options:     []string{"rbind", "ro", "rprivate"},
+				},
+			},
+		},
+		{
+			spec: "type=bind,source=" + src + ",target=/bar,rro",
+			wants: &Processed{
+				Type: Bind,
+				Mount: specs.Mount{
+					Type:        "bind",
+					Source:      src,
+					Destination: "/bar",
+					Options:     []string{"rbind", "ro", "rro", "rprivate"},
+				},
+			},
+		},
+	}
+	for _, tt := range accepted {
+		t.Run(tt.spec, func(t *testing.T) {
+			got, err := ProcessFlagMount(tt.spec, nil)
+			assert.NilError(t, err)
+			assert.Equal(t, got.Type, tt.wants.Type)
+			assert.Equal(t, got.Mount.Type, tt.wants.Mount.Type)
+			assert.Equal(t, got.Mount.Source, tt.wants.Mount.Source)
+			assert.Equal(t, got.Mount.Destination, tt.wants.Mount.Destination)
+			assert.DeepEqual(t, got.Mount.Options, tt.wants.Mount.Options)
+		})
+	}
+}