Allow mounting alloc-dir-relative paths in docker

client/driver/docker.go

@@ -63,7 +63,7 @@ const (
 	dockerSELinuxLabelConfigOption = "docker.volumes.selinuxlabel"
 
 	// dockerVolumesConfigOption is the key for enabling the use of custom
-	// bind volumes.
+	// bind volumes to arbitrary host paths.
 	dockerVolumesConfigOption  = "docker.volumes.enabled"
 	dockerVolumesConfigDefault = true
 
@@ -396,12 +396,29 @@ func (d *DockerDriver) containerBinds(driverConfig *DockerDriverConfig, alloc *a
 	binds := []string{allocDirBind, taskLocalBind, secretDirBind}
 
 	volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault)
-	if len(driverConfig.Volumes) > 0 && !volumesEnabled {
-		return nil, fmt.Errorf("%s is false; cannot use Docker Volumes: %+q", dockerVolumesConfigOption, driverConfig.Volumes)
-	}
+	for _, userbind := range driverConfig.Volumes {
+		parts := strings.Split(userbind, ":")
+		if len(parts) < 2 {
+			return nil, fmt.Errorf("invalid docker volume: %q", userbind)
+		}
+
+		// Resolve dotted path segments
+		parts[0] = filepath.Clean(parts[0])
+
+		// Absolute paths aren't always supported
+		if filepath.IsAbs(parts[0]) {
+			if !volumesEnabled {
+				// Disallow mounting arbitrary absolute paths
+				return nil, fmt.Errorf("%s is false; cannot mount host paths: %+q", dockerVolumesConfigOption, userbind)
+			}
+			binds = append(binds, userbind)
+			continue
+		}
 
-	if len(driverConfig.Volumes) > 0 {
-		binds = append(binds, driverConfig.Volumes...)
+		// Relative paths are always allowed as they mount within a container
+		// Expand path relative to alloc dir
+		parts[0] = filepath.Join(shared, parts[0])
+		binds = append(binds, strings.Join(parts, ":"))
 	}
 
 	if selinuxLabel := d.config.Read(dockerSELinuxLabelConfigOption); selinuxLabel != "" {

client/driver/docker_test.go

@@ -5,7 +5,6 @@ import (
 	"io/ioutil"
 	"math/rand"
 	"os"
-	"path"
 	"path/filepath"
 	"reflect"
 	"runtime/debug"
@@ -864,28 +863,23 @@ func TestDockerDriver_Stats(t *testing.T) {
 
 }
 
-func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver, *ExecContext, string, func()) {
+func setupDockerVolumes(t *testing.T, cfg *config.Config, hostpath string) (*structs.Task, Driver, *ExecContext, string, func()) {
 	if !testutil.DockerIsConnected(t) {
 		t.SkipNow()
 	}
 
-	tmpvol, err := ioutil.TempDir("", "nomadtest_dockerdriver_volumes")
-	if err != nil {
-		t.Fatalf("error creating temporary dir: %v", err)
-	}
-
 	randfn := fmt.Sprintf("test-%d", rand.Int())
-	hostpath := path.Join(tmpvol, randfn)
-	contpath := path.Join("/mnt/vol", randfn)
+	hostfile := filepath.Join(hostpath, randfn)
+	containerFile := filepath.Join("/mnt/vol", randfn)
 
 	task := &structs.Task{
 		Name: "ls",
 		Config: map[string]interface{}{
 			"image":   "busybox",
 			"load":    []string{"busybox.tar"},
 			"command": "touch",
-			"args":    []string{contpath},
-			"volumes": []string{fmt.Sprintf("%s:/mnt/vol", tmpvol)},
+			"args":    []string{containerFile},
+			"volumes": []string{fmt.Sprintf("%s:/mnt/vol", hostpath)},
 		},
 		LogConfig: &structs.LogConfig{
 			MaxFiles:      10,
@@ -900,7 +894,9 @@ func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver
 	execCtx := NewExecContext(allocDir, alloc.ID)
 	cleanup := func() {
 		execCtx.AllocDir.Destroy()
-		os.RemoveAll(tmpvol)
+		if filepath.IsAbs(hostpath) {
+			os.RemoveAll(hostpath)
+		}
 	}
 
 	taskEnv, err := GetTaskEnv(allocDir, cfg.Node, task, alloc, "")
@@ -913,26 +909,63 @@ func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver
 	driver := NewDockerDriver(driverCtx)
 	copyImage(execCtx, task, "busybox.tar", t)
 
-	return task, driver, execCtx, hostpath, cleanup
+	return task, driver, execCtx, hostfile, cleanup
 }
 
 func TestDockerDriver_VolumesDisabled(t *testing.T) {
 	cfg := testConfig()
 	cfg.Options = map[string]string{dockerVolumesConfigOption: "false"}
 
-	task, driver, execCtx, _, cleanup := setupDockerVolumes(t, cfg)
-	defer cleanup()
+	{
+		tmpvol, err := ioutil.TempDir("", "nomadtest_docker_volumesdisabled")
+		if err != nil {
+			t.Fatalf("error creating temporary dir: %v", err)
+		}
 
-	_, err := driver.Start(execCtx, task)
-	if err == nil {
-		t.Fatalf("Started driver successfully when volumes should have been disabled.")
+		task, driver, execCtx, _, cleanup := setupDockerVolumes(t, cfg, tmpvol)
+		defer cleanup()
+
+		if _, err := driver.Start(execCtx, task); err == nil {
+			t.Fatalf("Started driver successfully when volumes should have been disabled.")
+		}
 	}
+
+	// Relative paths should still be allowed
+	{
+		task, driver, execCtx, fn, cleanup := setupDockerVolumes(t, cfg, ".")
+		defer cleanup()
+
+		handle, err := driver.Start(execCtx, task)
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		defer handle.Kill()
+
+		select {
+		case res := <-handle.WaitCh():
+			if !res.Successful() {
+				t.Fatalf("unexpected err: %v", res)
+			}
+		case <-time.After(time.Duration(tu.TestMultiplier()*10) * time.Second):
+			t.Fatalf("timeout")
+		}
+
+		if _, err := ioutil.ReadFile(filepath.Join(execCtx.AllocDir.SharedDir, fn)); err != nil {
+			t.Fatalf("unexpected error reading %s: %v", fn, err)
+		}
+	}
+
 }
 
 func TestDockerDriver_VolumesEnabled(t *testing.T) {
 	cfg := testConfig()
 
-	task, driver, execCtx, hostpath, cleanup := setupDockerVolumes(t, cfg)
+	tmpvol, err := ioutil.TempDir("", "nomadtest_docker_volumesenabled")
+	if err != nil {
+		t.Fatalf("error creating temporary dir: %v", err)
+	}
+
+	task, driver, execCtx, hostpath, cleanup := setupDockerVolumes(t, cfg, tmpvol)
 	defer cleanup()
 
 	handle, err := driver.Start(execCtx, task)