docker: fix delimiter for selinux label for read-only volumes

.changelog/23750.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+docker: Fixed a bug where plugin SELinux labels would conflict with read-only `volume` options
+```

client/testutil/driver_compatible.go

@@ -63,13 +63,21 @@ func RequireLinux(t *testing.T) {
 }
 
 // RequireNotWindows skips tests whenever:
-// - running on Window
+// - running on Windows
 func RequireNotWindows(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("Test requires non-Windows")
 	}
 }
 
+// RequireWindows skips tests whenever:
+// - not running on Windows
+func RequireWindows(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("Test requires non-Windows")
+	}
+}
+
 // ExecCompatible skips tests unless:
 // - running as root
 // - running on Linux

drivers/docker/driver.go

@@ -745,15 +745,22 @@ func (d *Driver) convertAllocPathsForWindowsLCOW(task *drivers.TaskConfig, image
 }
 
 func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConfig) ([]string, error) {
+	taskLocalBindVolume := driverConfig.VolumeDriver == ""
+	if !d.config.Volumes.Enabled && !taskLocalBindVolume {
+		return nil, fmt.Errorf("volumes are not enabled; cannot use volume driver %q", driverConfig.VolumeDriver)
+	}
+
 	allocDirBind := fmt.Sprintf("%s:%s", task.TaskDir().SharedAllocDir, task.Env[taskenv.AllocDir])
 	taskLocalBind := fmt.Sprintf("%s:%s", task.TaskDir().LocalDir, task.Env[taskenv.TaskLocalDir])
 	secretDirBind := fmt.Sprintf("%s:%s", task.TaskDir().SecretsDir, task.Env[taskenv.SecretsDir])
 	binds := []string{allocDirBind, taskLocalBind, secretDirBind}
 
-	taskLocalBindVolume := driverConfig.VolumeDriver == ""
-
-	if !d.config.Volumes.Enabled && !taskLocalBindVolume {
-		return nil, fmt.Errorf("volumes are not enabled; cannot use volume driver %q", driverConfig.VolumeDriver)
+	selinuxLabel := d.config.Volumes.SelinuxLabel
+	if selinuxLabel != "" {
+		// Apply SELinux Label to each built-in bind
+		for i := range binds {
+			binds[i] = fmt.Sprintf("%s:%s", binds[i], selinuxLabel)
+		}
 	}
 
 	for _, userbind := range driverConfig.Volumes {
@@ -782,17 +789,18 @@ func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConf
 		}
 
 		bind := src + ":" + dst
-		if mode != "" {
-			bind += ":" + mode
+		opts := mode
+		if opts != "" {
+			if selinuxLabel != "" {
+				opts += "," + selinuxLabel
+			}
+		} else {
+			opts = selinuxLabel
 		}
-		binds = append(binds, bind)
-	}
-
-	if selinuxLabel := d.config.Volumes.SelinuxLabel; selinuxLabel != "" {
-		// Apply SELinux Label to each volume
-		for i := range binds {
-			binds[i] = fmt.Sprintf("%s:%s", binds[i], selinuxLabel)
+		if opts != "" {
+			bind += ":" + opts
 		}
+		binds = append(binds, bind)
 	}
 
 	return binds, nil

drivers/docker/driver_test.go

@@ -12,7 +12,6 @@ import (
 	"reflect"
 	"runtime"
 	"runtime/debug"
-	"sort"
 	"strings"
 	"syscall"
 	"testing"
@@ -92,7 +91,10 @@ func dockerTask(t *testing.T) (*drivers.TaskConfig, *TaskConfig, []int) {
 		Name:    "redis-demo",
 		AllocID: uuid.Generate(),
 		Env: map[string]string{
-			"test": t.Name(),
+			"test":              t.Name(),
+			"NOMAD_ALLOC_DIR":   "/alloc",
+			"NOMAD_TASK_DIR":    "/local",
+			"NOMAD_SECRETS_DIR": "/secrets",
 		},
 		DeviceEnv: make(map[string]string),
 		Resources: &drivers.Resources{
@@ -119,6 +121,12 @@ func dockerTask(t *testing.T) (*drivers.TaskConfig, *TaskConfig, []int) {
 		},
 	}
 
+	if runtime.GOOS == "windows" {
+		task.Env["NOMAD_ALLOC_DIR"] = "c:/alloc"
+		task.Env["NOMAD_TASK_DIR"] = "c:/local"
+		task.Env["NOMAD_SECRETS_DIR"] = "c:/secrets"
+	}
+
 	require.NoError(t, task.EncodeConcreteDriverConfig(&cfg))
 
 	return task, &cfg, ports
@@ -1285,6 +1293,7 @@ func TestDockerDriver_CreateContainerConfig_Logging(t *testing.T) {
 
 func TestDockerDriver_CreateContainerConfig_Mounts(t *testing.T) {
 	ci.Parallel(t)
+	testutil.RequireLinux(t)
 
 	task, cfg, _ := dockerTask(t)
 
@@ -1310,17 +1319,26 @@ func TestDockerDriver_CreateContainerConfig_Mounts(t *testing.T) {
 			Target: "/list-tmpfs-target",
 		},
 	}
-
-	expectedSrcPrefix := "/"
-	if runtime.GOOS == "windows" {
-		expectedSrcPrefix = "redis-demo\\"
+	cfg.Volumes = []string{
+		"/etc/ssl/certs:/etc/ssl/certs:ro",
+		"/var/www:/srv/www",
 	}
-	expected := []docker.HostMount{
+
+	must.NoError(t, task.EncodeConcreteDriverConfig(cfg))
+	dh := dockerDriverHarness(t, nil)
+	driver := dh.Impl().(*Driver)
+	driver.config.Volumes.Enabled = true
+	driver.config.Volumes.SelinuxLabel = "z"
+
+	cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1")
+	must.NoError(t, err)
+
+	must.Eq(t, []docker.HostMount{
 		// from mount map
 		{
 			Type:        "bind",
 			Target:      "/map-bind-target",
-			Source:      expectedSrcPrefix + "map-source",
+			Source:      "/map-source",
 			BindOptions: &docker.BindOptions{},
 		},
 		{
@@ -1332,32 +1350,100 @@ func TestDockerDriver_CreateContainerConfig_Mounts(t *testing.T) {
 		{
 			Type:        "bind",
 			Target:      "/list-bind-target",
-			Source:      expectedSrcPrefix + "list-source",
+			Source:      "/list-source",
 			BindOptions: &docker.BindOptions{},
 		},
 		{
 			Type:          "tmpfs",
 			Target:        "/list-tmpfs-target",
 			TempfsOptions: &docker.TempfsOptions{},
 		},
-	}
+	}, cc.HostConfig.Mounts)
 
-	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
+	must.Eq(t, []string{
+		"alloc:/alloc:z",
+		"redis-demo/local:/local:z",
+		"redis-demo/secrets:/secrets:z",
+		"/etc/ssl/certs:/etc/ssl/certs:ro,z",
+		"/var/www:/srv/www:z",
+	}, cc.HostConfig.Binds)
+}
+
+func TestDockerDriver_CreateContainerConfig_Mounts_Windows(t *testing.T) {
+	ci.Parallel(t)
+	testutil.RequireWindows(t)
+
+	task, cfg, _ := dockerTask(t)
 
+	cfg.Mounts = []DockerMount{
+		{
+			Type:   "bind",
+			Target: "/map-bind-target",
+			Source: "/map-source",
+		},
+		{
+			Type:   "tmpfs",
+			Target: "/map-tmpfs-target",
+		},
+	}
+	cfg.MountsList = []DockerMount{
+		{
+			Type:   "bind",
+			Target: "/list-bind-target",
+			Source: "/list-source",
+		},
+		{
+			Type:   "tmpfs",
+			Target: "/list-tmpfs-target",
+		},
+	}
+	cfg.Volumes = []string{
+		"c:/etc/ssl/certs:c:/etc/ssl/certs",
+		"c:/var/www:c:/srv/www",
+	}
+
+	must.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 	dh := dockerDriverHarness(t, nil)
 	driver := dh.Impl().(*Driver)
 	driver.config.Volumes.Enabled = true
 
 	cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1")
-	require.NoError(t, err)
+	must.NoError(t, err)
 
-	found := cc.HostConfig.Mounts
-	sort.Slice(found, func(i, j int) bool { return strings.Compare(found[i].Target, found[j].Target) < 0 })
-	sort.Slice(expected, func(i, j int) bool {
-		return strings.Compare(expected[i].Target, expected[j].Target) < 0
-	})
+	must.Eq(t, []docker.HostMount{
+		// from mount map
+		{
+			Type:        "bind",
+			Target:      "/map-bind-target",
+			Source:      "redis-demo\\map-source",
+			BindOptions: &docker.BindOptions{},
+		},
+		{
+			Type:          "tmpfs",
+			Target:        "/map-tmpfs-target",
+			TempfsOptions: &docker.TempfsOptions{},
+		},
+		// from mount list
+		{
+			Type:        "bind",
+			Target:      "/list-bind-target",
+			Source:      "redis-demo\\list-source",
+			BindOptions: &docker.BindOptions{},
+		},
+		{
+			Type:          "tmpfs",
+			Target:        "/list-tmpfs-target",
+			TempfsOptions: &docker.TempfsOptions{},
+		},
+	}, cc.HostConfig.Mounts)
 
-	require.Equal(t, expected, found)
+	must.Eq(t, []string{
+		`alloc:c:/alloc`,
+		`redis-demo\local:c:/local`,
+		`redis-demo\secrets:c:/secrets`,
+		`c:\etc\ssl\certs:c:/etc/ssl/certs`,
+		`c:\var\www:c:/srv/www`,
+	}, cc.HostConfig.Binds)
 }
 
 func TestDockerDriver_CreateContainerConfigWithRuntimes(t *testing.T) {