api/pre-1.44: Default ReadOnlyNonRecursive to true

Don't change the behavior for older clients and keep the same behavior.
Otherwise client can't opt-out (because `ReadOnlyNonRecursive` is
unsupported before 1.44).

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>

api/server/router/container/container_routes.go

@@ -562,13 +562,20 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
 			config.Healthcheck.StartInterval = 0
 		}
 
-		for _, m := range hostConfig.Mounts {
-			if m.BindOptions != nil {
-				// Ignore ReadOnlyNonRecursive because it was added in API 1.44.
-				m.BindOptions.ReadOnlyNonRecursive = false
-				if m.BindOptions.ReadOnlyForceRecursive {
+		for idx, m := range hostConfig.Mounts {
+			if m.Type == mount.TypeBind && m.ReadOnly {
+				bo := m.BindOptions
+				if bo != nil && bo.ReadOnlyForceRecursive {
 					return errdefs.InvalidParameter(errors.New("BindOptions.ReadOnlyForceRecursive needs API v1.44 or newer"))
+				} else {
+					bo = &mount.BindOptions{}
+					hostConfig.Mounts[idx].BindOptions = bo
 				}
+
+				// ReadOnlyNonRecursive was added in API 1.44.
+				// Before that all read-only mounts were non-recursive.
+				// Keep that behavior for clients on older APIs.
+				bo.ReadOnlyNonRecursive = true
 			}
 		}
 

api/swagger.yaml

@@ -391,7 +391,11 @@ definitions:
           ReadOnlyNonRecursive:
             description: |
                Make the mount non-recursively read-only, but still leave the mount recursive
-               (unless NonRecursive is set to true in conjunction).
+               (unless NonRecursive is set to `true` in conjunction).
+
+               Addded in v1.44, before that version all read-only mounts were
+               non-recursive by default. To match the previous behaviour this
+               will default to `true` for clients on versions prior to v1.44.
             type: "boolean"
             default: false
           ReadOnlyForceRecursive:

integration/container/mounts_linux_test.go

@@ -426,6 +426,60 @@ func TestContainerCopyLeaksMounts(t *testing.T) {
 	assert.Equal(t, mountsBefore, mountsAfter)
 }
 
+func TestContainerBindMountReadOnlyDefault(t *testing.T) {
+	skip.If(t, testEnv.IsRemoteDaemon)
+
+	ctx := setupTest(t)
+
+	// The test will run a container with a simple readonly /dev bind mount (-v /dev:/dev:ro)
+	// It will then check /proc/self/mountinfo for the mount type of /dev/shm (submount of /dev)
+	// If /dev/shm is rw, that will mean that the read-only mounts are NOT recursive by default.
+	const nonRecursive = " /dev/shm rw,"
+	// If /dev/shm is ro, that will mean that the read-only mounts ARE recursive by default.
+	const recursive = " /dev/shm ro,"
+
+	for _, tc := range []struct {
+		version     string
+		expectedOut string
+		name        string
+	}{
+		{version: "", expectedOut: recursive, name: "latest should be the same as 1.44"},
+		{version: "1.44", expectedOut: recursive, name: "submount should be recursive by default on 1.44"},
+
+		{version: "1.43", expectedOut: nonRecursive, name: "older should be non-recursive by default"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			apiClient := testEnv.APIClient()
+
+			if tc.version != "" {
+				skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), tc.version), "requires API v"+tc.version)
+				c, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion(tc.version))
+				assert.NilError(t, err, "failed to create client with version v%s", tc.version)
+				apiClient = c
+			}
+
+			cid := container.Run(ctx, t, apiClient,
+				container.WithMount(mounttypes.Mount{
+					Type:     mounttypes.TypeBind,
+					Source:   "/dev",
+					Target:   "/dev",
+					ReadOnly: true,
+				}),
+				container.WithCmd("sh", "-c", "grep /dev/shm /proc/self/mountinfo"),
+			)
+			out, err := container.Output(ctx, apiClient, cid)
+			assert.NilError(t, err)
+
+			assert.Check(t, is.Equal(out.Stderr, ""))
+			// Output should be either:
+			// 545 526 0:160 / /dev/shm ro,nosuid,nodev,noexec,relatime shared:90 - tmpfs shm rw,size=65536k
+			// or
+			// 545 526 0:160 / /dev/shm rw,nosuid,nodev,noexec,relatime shared:90 - tmpfs shm rw,size=65536k
+			assert.Check(t, is.Contains(out.Stdout, tc.expectedOut))
+		})
+	}
+}
+
 func TestContainerBindMountRecursivelyReadOnly(t *testing.T) {
 	skip.If(t, testEnv.IsRemoteDaemon)
 	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.44"), "requires API v1.44")