diff --git a/client/driver/docker.go b/client/driver/docker.go index 5e02e34c5..bafde0517 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -64,7 +64,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 @@ -399,12 +399,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) + } - if len(driverConfig.Volumes) > 0 { - binds = append(binds, driverConfig.Volumes...) + // 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 + } + + // 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 != "" { diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 0dabfcf8f..4c02905d4 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "math/rand" "os" - "path" "path/filepath" "reflect" "runtime/debug" @@ -944,19 +943,14 @@ done } } -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", @@ -964,8 +958,8 @@ func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver "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, @@ -980,7 +974,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, "") @@ -993,26 +989,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)