Merge component 'engine' from git@github.com:moby/moby master

components/engine/daemon/oci_linux.go

@@ -667,7 +667,7 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c
 	if s.Root.Readonly {
 		for i, m := range s.Mounts {
 			switch m.Destination {
-			case "/proc", "/dev/pts", "/dev/mqueue", "/dev":
+			case "/proc", "/dev/pts", "/dev/shm", "/dev/mqueue", "/dev":
 				continue
 			}
 			if _, ok := userMounts[m.Destination]; !ok {

components/engine/daemon/oci_linux_test.go

@@ -48,3 +48,41 @@ func TestTmpfsDevShmNoDupMount(t *testing.T) {
 	err = setMounts(&d, &s, c, ms)
 	assert.NoError(t, err)
 }
+
+// TestIpcPrivateVsReadonly checks that in case of IpcMode: private
+// and ReadonlyRootfs: true (as in "docker run --ipc private --read-only")
+// the resulting /dev/shm mount is NOT made read-only.
+// https://github.com/moby/moby/issues/36503
+func TestIpcPrivateVsReadonly(t *testing.T) {
+	d := Daemon{
+		// some empty structs to avoid getting a panic
+		// caused by a null pointer dereference
+		idMappings:  &idtools.IDMappings{},
+		configStore: &config.Config{},
+	}
+	c := &container.Container{
+		HostConfig: &containertypes.HostConfig{
+			IpcMode:        containertypes.IpcMode("private"),
+			ReadonlyRootfs: true,
+		},
+	}
+
+	// We can't call createSpec() so mimick the minimal part
+	// of its code flow, just enough to reproduce the issue.
+	ms, err := d.setupMounts(c)
+	assert.NoError(t, err)
+
+	s := oci.DefaultSpec()
+	s.Root.Readonly = c.HostConfig.ReadonlyRootfs
+
+	err = setMounts(&d, &s, c, ms)
+	assert.NoError(t, err)
+
+	// Find the /dev/shm mount in ms, check it does not have ro
+	for _, m := range s.Mounts {
+		if m.Destination != "/dev/shm" {
+			continue
+		}
+		assert.Equal(t, false, inSlice(m.Options, "ro"))
+	}
+}

components/engine/integration/container/diff_test.go

@@ -10,13 +10,11 @@ import (
 	"github.com/docker/docker/integration/internal/request"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/gotestyourself/gotestyourself/poll"
-	"github.com/gotestyourself/gotestyourself/skip"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-// ensure that an added file shows up in docker diff
-func TestDiffFilenameShownInOutput(t *testing.T) {
+func TestDiff(t *testing.T) {
 	defer setupTest(t)()
 	client := request.NewAPIClient(t)
 	ctx := context.Background()
@@ -27,72 +25,19 @@ func TestDiffFilenameShownInOutput(t *testing.T) {
 	// it will take a few seconds to exit. Also there's no way in Windows to
 	// differentiate between an Add or a Modify, and all files are under
 	// a "Files/" prefix.
-	lookingFor := containertypes.ContainerChangeResponseItem{Kind: archive.ChangeAdd, Path: "/foo/bar"}
+	expected := []containertypes.ContainerChangeResponseItem{
+		{Kind: archive.ChangeAdd, Path: "/foo"},
+		{Kind: archive.ChangeAdd, Path: "/foo/bar"},
+	}
 	if testEnv.OSType == "windows" {
 		poll.WaitOn(t, container.IsInState(ctx, client, cID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(60*time.Second))
-		lookingFor = containertypes.ContainerChangeResponseItem{Kind: archive.ChangeModify, Path: "Files/foo/bar"}
-	}
-
-	items, err := client.ContainerDiff(ctx, cID)
-	require.NoError(t, err)
-	assert.Contains(t, items, lookingFor)
-}
-
-// test to ensure GH #3840 doesn't occur any more
-func TestDiffEnsureInitLayerFilesAreIgnored(t *testing.T) {
-	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
-
-	defer setupTest(t)()
-	client := request.NewAPIClient(t)
-	ctx := context.Background()
-
-	// this is a list of files which shouldn't show up in `docker diff`
-	initLayerFiles := []string{"/etc/resolv.conf", "/etc/hostname", "/etc/hosts", "/.dockerenv"}
-	containerCount := 5
-
-	// we might not run into this problem from the first run, so start a few containers
-	for i := 0; i < containerCount; i++ {
-		cID := container.Run(t, ctx, client, container.WithCmd("sh", "-c", `echo foo > /root/bar`))
-
-		items, err := client.ContainerDiff(ctx, cID)
-		require.NoError(t, err)
-		for _, item := range items {
-			assert.NotContains(t, initLayerFiles, item.Path)
+		expected = []containertypes.ContainerChangeResponseItem{
+			{Kind: archive.ChangeModify, Path: "Files/foo"},
+			{Kind: archive.ChangeModify, Path: "Files/foo/bar"},
 		}
 	}
-}
-
-func TestDiffEnsureDefaultDevs(t *testing.T) {
-	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
-
-	defer setupTest(t)()
-	client := request.NewAPIClient(t)
-	ctx := context.Background()
-
-	cID := container.Run(t, ctx, client, container.WithCmd("sleep", "0"))
 
 	items, err := client.ContainerDiff(ctx, cID)
 	require.NoError(t, err)
-
-	expected := []containertypes.ContainerChangeResponseItem{
-		{Kind: archive.ChangeModify, Path: "/dev"},
-		{Kind: archive.ChangeAdd, Path: "/dev/full"},    // busybox
-		{Kind: archive.ChangeModify, Path: "/dev/ptmx"}, // libcontainer
-		{Kind: archive.ChangeAdd, Path: "/dev/mqueue"},
-		{Kind: archive.ChangeAdd, Path: "/dev/kmsg"},
-		{Kind: archive.ChangeAdd, Path: "/dev/fd"},
-		{Kind: archive.ChangeAdd, Path: "/dev/ptmx"},
-		{Kind: archive.ChangeAdd, Path: "/dev/null"},
-		{Kind: archive.ChangeAdd, Path: "/dev/random"},
-		{Kind: archive.ChangeAdd, Path: "/dev/stdout"},
-		{Kind: archive.ChangeAdd, Path: "/dev/stderr"},
-		{Kind: archive.ChangeAdd, Path: "/dev/tty1"},
-		{Kind: archive.ChangeAdd, Path: "/dev/stdin"},
-		{Kind: archive.ChangeAdd, Path: "/dev/tty"},
-		{Kind: archive.ChangeAdd, Path: "/dev/urandom"},
-	}
-
-	for _, item := range items {
-		assert.Contains(t, expected, item)
-	}
+	assert.Equal(t, expected, items)
 }