diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 5c173aade43d7..07cdf08811225 100644 --- a/api/server/router/container/container_routes.go +++ b/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 } } diff --git a/api/swagger.yaml b/api/swagger.yaml index 94f4d789dd0e4..084c0d11a87e8 100644 --- a/api/swagger.yaml +++ b/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: diff --git a/integration/container/mounts_linux_test.go b/integration/container/mounts_linux_test.go index 47f5f32d8f40c..87da674b50d38 100644 --- a/integration/container/mounts_linux_test.go +++ b/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")