Merge pull request #5572 from hashicorp/dani/b-docker-volumes

Switch to pre-0.9 behaviour for handling volumes
hashicorp · Apr 18, 2019 · 11388ab · 11388ab
2 parents 4789948 + ccce364
commit 11388ab
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 22 deletions.
diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go
@@ -603,9 +603,9 @@ func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConf
 	secretDirBind := fmt.Sprintf("%s:%s", task.TaskDir().SecretsDir, task.Env[taskenv.SecretsDir])
 	binds := []string{allocDirBind, taskLocalBind, secretDirBind}
 
-	localBindVolume := driverConfig.VolumeDriver == "" || driverConfig.VolumeDriver == "local"
+	taskLocalBindVolume := driverConfig.VolumeDriver == ""
 
-	if !d.config.Volumes.Enabled && !localBindVolume {
+	if !d.config.Volumes.Enabled && !taskLocalBindVolume {
 		return nil, fmt.Errorf("volumes are not enabled; cannot use volume driver %q", driverConfig.VolumeDriver)
 	}
 
@@ -615,15 +615,21 @@ func (d *Driver) containerBinds(task *drivers.TaskConfig, driverConfig *TaskConf
 			return nil, fmt.Errorf("invalid docker volume: %q", userbind)
 		}
 
-		// Paths inside task dir are always allowed, Relative paths are always allowed as they mount within a container
-		// When a VolumeDriver is set, we assume we receive a binding in the format volume-name:container-dest
-		// Otherwise, we assume we receive a relative path binding in the format relative/to/task:/also/in/container
-		if localBindVolume {
+		// Paths inside task dir are always allowed when using the default driver,
+		// Relative paths are always allowed as they mount within a container
+		// When a VolumeDriver is set, we assume we receive a binding in the format
+		// volume-name:container-dest
+		// Otherwise, we assume we receive a relative path binding in the format
+		// relative/to/task:/also/in/container
+		if taskLocalBindVolume {
 			parts[0] = expandPath(task.TaskDir().Dir, parts[0])
+		} else {
+			// Resolve dotted path segments
+			parts[0] = filepath.Clean(parts[0])
+		}
 
-			if !d.config.Volumes.Enabled && !isParentPath(task.AllocDir, parts[0]) {
-				return nil, fmt.Errorf("volumes are not enabled; cannot mount host paths: %+q", userbind)
-			}
+		if !d.config.Volumes.Enabled && !isParentPath(task.AllocDir, parts[0]) {
+			return nil, fmt.Errorf("volumes are not enabled; cannot mount host paths: %+q", userbind)
 		}
 
 		binds = append(binds, strings.Join(parts, ":"))

diff --git a/drivers/docker/driver_unix_test.go b/drivers/docker/driver_unix_test.go
@@ -298,11 +298,11 @@ func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) {
 			expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"},
 		},
 		{
-			name:            "relative local driver",
-			requiresVolumes: false,
+			name:            "named volume local driver",
+			requiresVolumes: true,
 			volumeDriver:    "local",
 			volumes:         []string{"test-path:/tmp/taskpath"},
-			expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"},
+			expectedVolumes: []string{"test-path:/tmp/taskpath"},
 		},
 		{
 			name:            "relative outside task-dir default driver",
@@ -311,13 +311,6 @@ func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) {
 			volumes:         []string{"../test-path:/tmp/taskpath"},
 			expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"},
 		},
-		{
-			name:            "relative outside task-dir local driver",
-			requiresVolumes: false,
-			volumeDriver:    "local",
-			volumes:         []string{"../test-path:/tmp/taskpath"},
-			expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"},
-		},
 		{
 			name:            "relative outside alloc-dir default driver",
 			requiresVolumes: true,
@@ -326,11 +319,11 @@ func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) {
 			expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"},
 		},
 		{
-			name:            "relative outside task-dir local driver",
+			name:            "clean path local driver",
 			requiresVolumes: true,
 			volumeDriver:    "local",
-			volumes:         []string{"../../test-path:/tmp/taskpath"},
-			expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"},
+			volumes:         []string{"/tmp/nomad/../test-path:/tmp/taskpath"},
+			expectedVolumes: []string{"/tmp/test-path:/tmp/taskpath"},
 		},
 	}