From ccce364cbdf7560725a25b54ff5a420d11a44b5a Mon Sep 17 00:00:00 2001 From: Danielle Lancashire Date: Wed, 17 Apr 2019 11:13:34 +0200 Subject: [PATCH] Switch to pre-0.9 behaviour for handling volumes In Nomad 0.9, we made volume driver handling the same for `""`, and `"local"` volumes. Prior to Nomad 0.9 however these had slightly different behaviour for relative paths and named volumes. Prior to 0.9 the empty string would expand relative paths within the task dir, and `"local"` volumes that are not absolute paths would be treated as docker named volumes. This commit reverts to the previous behaviour as follows: | Nomad Version | Driver | Volume Spec | Behaviour | |------------------------------------------------------------------------- | all | "" | testing:/testing | allocdir/testing | | 0.8.7 | "local" | testing:/testing | "testing" as named volume | | 0.9.0 | "local" | testing:/testing | allocdir/testing | | 0.9.1 | "local" | testing:/testing | "testing" as named volume | --- drivers/docker/driver.go | 24 +++++++++++++++--------- drivers/docker/driver_unix_test.go | 19 ++++++------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index 62830089b0d7..ea98209a5f92 100644 --- 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 index e2b9f35c3749..bac2a0e9294b 100644 --- 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"}, }, }