Add relative and absolute volume mounting to Docker and rkt

client/driver/docker.go

@@ -63,8 +63,9 @@ const (
 	dockerSELinuxLabelConfigOption = "docker.volumes.selinuxlabel"
 
 	// dockerVolumesConfigOption is the key for enabling the use of custom
-	// bind volumes.
-	dockerVolumesConfigOption = "docker.volumes.enabled"
+	// bind volumes to arbitrary host paths.
+	dockerVolumesConfigOption  = "docker.volumes.enabled"
+	dockerVolumesConfigDefault = true
 
 	// dockerPrivilegedConfigOption is the key for running containers in
 	// Docker's privileged mode.
@@ -369,7 +370,7 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool
 	node.Attributes["driver.docker.version"] = env.Get("Version")
 
 	// Advertise if this node supports Docker volumes (by default we do not)
-	if d.config.ReadBoolDefault(dockerVolumesConfigOption, false) {
+	if d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) {
 		node.Attributes["driver."+dockerVolumesConfigOption] = "1"
 	}
 
@@ -394,13 +395,30 @@ func (d *DockerDriver) containerBinds(driverConfig *DockerDriverConfig, alloc *a
 	secretDirBind := fmt.Sprintf("%s:%s", secret, allocdir.TaskSecretsContainerPath)
 	binds := []string{allocDirBind, taskLocalBind, secretDirBind}
 
-	volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, false)
-	if len(driverConfig.Volumes) > 0 && !volumesEnabled {
-		return nil, fmt.Errorf("%s is false; cannot use Docker Volumes: %+q", dockerVolumesConfigOption, driverConfig.Volumes)
-	}
+	volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault)
+	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()
-	cfg.Options = map[string]string{dockerVolumesConfigOption: "true"}
 
-	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)

client/driver/rkt.go

@@ -45,7 +45,8 @@ const (
 
 	// rktVolumesConfigOption is the key for enabling the use of custom
 	// bind volumes.
-	rktVolumesConfigOption = "rkt.volumes.enabled"
+	rktVolumesConfigOption  = "rkt.volumes.enabled"
+	rktVolumesConfigDefault = true
 
 	// rktCmd is the command rkt is installed as.
 	rktCmd = "rkt"
@@ -233,7 +234,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e
 
 	// Mount arbitrary volumes if enabled
 	if len(driverConfig.Volumes) > 0 {
-		if enabled := d.config.ReadBoolDefault(rktVolumesConfigOption, false); !enabled {
+		if enabled := d.config.ReadBoolDefault(rktVolumesConfigOption, rktVolumesConfigDefault); !enabled {
 			return nil, fmt.Errorf("%s is false; cannot use rkt volumes: %+q", rktVolumesConfigOption, driverConfig.Volumes)
 		}
 

client/driver/rkt_test.go

@@ -259,7 +259,6 @@ func TestRktDriver_Start_Wait_AllocDir(t *testing.T) {
 	}
 
 	driverCtx, execCtx := testDriverContexts(task)
-	driverCtx.config.Options = map[string]string{rktVolumesConfigOption: "1"}
 	defer execCtx.AllocDir.Destroy()
 	d := NewRktDriver(driverCtx)
 

website/source/docs/drivers/docker.html.md

@@ -162,12 +162,19 @@ The `docker` driver supports the following configuration in the job spec:
     ```
 
 * `volumes` - (Optional) A list of `host_path:container_path` strings to bind
-  host paths to container paths. Can only be run on clients with the
-  `docker.volumes.enabled` option set to true.
+  host paths to container paths. Mounting host paths outside of the alloc
+  directory tasks normally have access to can be disabled on clients by setting
+  the `docker.volumes.enabled` option set to false.
 
     ```hcl
     config {
-      volumes = ["/path/on/host:/path/in/container"]
+      volumes = [
+        # Use absolute paths to mount arbitrary paths on the host
+        "/path/on/host:/path/in/container",
+
+        # Use relative paths to rebind paths already in the allocation dir
+        "relative/to/alloc:/also/in/container"
+      ]
     }
     ```
 
@@ -363,9 +370,9 @@ options](/docs/agent/config.html#options):
 * `docker.cleanup.image` Defaults to `true`. Changing this to `false` will
   prevent Nomad from removing images from stopped tasks.
 
-* `docker.volumes.enabled`: Defaults to `false`. Allows tasks to bind host
-  paths (`volumes`) or other containers (`volums_from`) inside their container.
-  Disabled by default as it removes the isolation between containers' data.
+* `docker.volumes.enabled`: Defaults to `true`. Allows tasks to bind host paths
+  (`volumes`) inside their container. Binding relative paths is always allowed
+  and will be resolved relative to the allocation's directory.
 
 * `docker.volumes.selinuxlabel`: Allows the operator to set a SELinux
   label to the allocation and task local bind-mounts to containers. If used

website/source/docs/drivers/rkt.html.md

@@ -76,12 +76,19 @@ The `rkt` driver supports the following configuration in the job spec:
 * `debug` - (Optional) Enable rkt command debug option.
 
 * `volumes` - (Optional) A list of `host_path:container_path` strings to bind
-  host paths to container paths. Can only be run on clients with the
-  `rkt.volumes.enabled` option set to true.
+  host paths to container paths. Mounting host paths outside of the alloc
+  directory tasks normally have access to can be disabled on clients by setting
+  the `rkt.volumes.enabled` option set to false.
 
     ```hcl
     config {
-      volumes = ["/path/on/host:/path/in/container"]
+      volumes = [
+        # Use absolute paths to mount arbitrary paths on the host
+        "/path/on/host:/path/in/container",
+
+        # Use relative paths to rebind paths already in the allocation dir
+        "relative/to/alloc:/also/in/container"
+      ]
     }
     ```
 
@@ -97,9 +104,10 @@ over HTTP.
 The `rkt` driver has the following [client configuration
 options](/docs/agent/config.html#options):
 
-* `rkt.volumes.enabled`: Defaults to `false`. Allows tasks to bind host paths
-  (`volumes`) inside their container.  Disabled by default as it removes the
-  isolation between containers' data.
+* `rkt.volumes.enabled`: Defaults to `true`. Allows tasks to bind host paths
+  (`volumes`) inside their container. Binding relative paths is always allowed
+  and will be resolved relative to the allocation's directory.
+
 
 ## Client Attributes