diff --git a/.changelog/14851.txt b/.changelog/14851.txt new file mode 100644 index 000000000..31c532fc4 --- /dev/null +++ b/.changelog/14851.txt @@ -0,0 +1,3 @@ +```release-note:improvement +exec: Allow running commands from mounted host volumes +``` diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index fc91fc367..4ab8367be 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -120,32 +120,15 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro l.container = container // Look up the binary path and make it executable - absPath, err := lookupTaskBin(command) - + taskPath, hostPath, err := lookupTaskBin(command) if err != nil { return nil, err } - - if err := makeExecutable(absPath); err != nil { + if err := makeExecutable(hostPath); err != nil { return nil, err } - path := absPath - - // Ensure that the path is contained in the chroot, and find it relative to the container - rel, err := filepath.Rel(command.TaskDir, path) - if err != nil { - return nil, fmt.Errorf("failed to determine relative path base=%q target=%q: %v", command.TaskDir, path, err) - } - - // Turn relative-to-chroot path into absolute path to avoid - // libcontainer trying to resolve the binary using $PATH. - // Do *not* use filepath.Join as it will translate ".."s returned by - // filepath.Rel. Prepending "/" will cause the path to be rooted in the - // chroot which is the desired behavior. - path = "/" + rel - - combined := append([]string{path}, command.Args...) + combined := append([]string{taskPath}, command.Args...) stdout, err := command.Stdout() if err != nil { return nil, err @@ -805,52 +788,135 @@ func cmdMounts(mounts []*drivers.MountConfig) []*lconfigs.Mount { return r } -// lookupTaskBin finds the file `bin` in taskDir/local, taskDir in that order, then performs -// a PATH search inside taskDir. It returns an absolute path. See also executor.lookupBin -func lookupTaskBin(command *ExecCommand) (string, error) { +// lookupTaskBin finds the file `bin`, searching in order: +// - taskDir/local +// - taskDir +// - each mount, in order listed in the jobspec +// - a PATH-like search of usr/local/bin/, usr/bin/, and bin/ inside the taskDir +// +// Returns an absolute path inside the container that will get passed as arg[0] +// to the launched process, and the absolute path to that binary as seen by the +// host (these will be identical for binaries that don't come from mounts). +// +// See also executor.lookupBin for a version used by non-isolated drivers. +func lookupTaskBin(command *ExecCommand) (string, string, error) { taskDir := command.TaskDir bin := command.Cmd // Check in the local directory localDir := filepath.Join(taskDir, allocdir.TaskLocal) - local := filepath.Join(localDir, bin) - if _, err := os.Stat(local); err == nil { - return local, nil + taskPath, hostPath, err := getPathInTaskDir(command.TaskDir, localDir, bin) + if err == nil { + return taskPath, hostPath, nil } // Check at the root of the task's directory - root := filepath.Join(taskDir, bin) - if _, err := os.Stat(root); err == nil { - return root, nil + taskPath, hostPath, err = getPathInTaskDir(command.TaskDir, command.TaskDir, bin) + if err == nil { + return taskPath, hostPath, nil } + // Check in our mounts + for _, mount := range command.Mounts { + taskPath, hostPath, err = getPathInMount(mount.HostPath, mount.TaskPath, bin) + if err == nil { + return taskPath, hostPath, nil + } + } + + // If there's a / in the binary's path, we can't fallback to a PATH search if strings.Contains(bin, "/") { - return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) + return "", "", fmt.Errorf("file %s not found under path %s", bin, taskDir) } - path := "/usr/local/bin:/usr/bin:/bin" + // look for a file using a PATH-style lookup inside the directory + // root. Similar to the stdlib's exec.LookPath except: + // - uses a restricted lookup PATH rather than the agent process's PATH env var. + // - does not require that the file is already executable (this will be ensured + // by the caller) + // - does not prevent using relative path as added to exec.LookPath in go1.19 + // (this gets fixed-up in the caller) - return lookPathIn(path, taskDir, bin) + // This is a fake PATH so that we're not using the agent's PATH + restrictedPaths := []string{"/usr/local/bin", "/usr/bin", "/bin"} + + for _, dir := range restrictedPaths { + pathDir := filepath.Join(command.TaskDir, dir) + taskPath, hostPath, err = getPathInTaskDir(command.TaskDir, pathDir, bin) + if err == nil { + return taskPath, hostPath, nil + } + } + + return "", "", fmt.Errorf("file %s not found under path", bin) } -// lookPathIn looks for a file with PATH inside the directory root. Like exec.LookPath -func lookPathIn(path string, root string, bin string) (string, error) { - // exec.LookPath(file string) - for _, dir := range filepath.SplitList(path) { - if dir == "" { - // match unix shell behavior, empty path element == . - dir = "." - } - path := filepath.Join(root, dir, bin) - f, err := os.Stat(path) - if err != nil { - continue - } - if m := f.Mode(); !m.IsDir() { - return path, nil - } +// getPathInTaskDir searches for the binary in the task directory and nested +// search directory. It returns the absolute path rooted inside the container +// and the absolute path on the host. +func getPathInTaskDir(taskDir, searchDir, bin string) (string, string, error) { + + hostPath := filepath.Join(searchDir, bin) + err := filepathIsRegular(hostPath) + if err != nil { + return "", "", err } - return "", fmt.Errorf("file %s not found under path %s", bin, root) + + // Find the path relative to the task directory + rel, err := filepath.Rel(taskDir, hostPath) + if rel == "" || err != nil { + return "", "", fmt.Errorf( + "failed to determine relative path base=%q target=%q: %v", + taskDir, hostPath, err) + } + + // Turn relative-to-taskdir path into re-rooted absolute path to avoid + // libcontainer trying to resolve the binary using $PATH. + // Do *not* use filepath.Join as it will translate ".."s returned by + // filepath.Rel. Prepending "/" will cause the path to be rooted in the + // chroot which is the desired behavior. + return filepath.Clean("/" + rel), hostPath, nil +} + +// getPathInMount for the binary in the mount's host path, constructing the path +// considering that the bin path is rooted in the mount's task path and not its +// host path. It returns the absolute path rooted inside the container and the +// absolute path on the host. +func getPathInMount(mountHostPath, mountTaskPath, bin string) (string, string, error) { + + // Find the path relative to the mount point in the task so that we can + // trim off any shared prefix when we search on the host path + mountRel, err := filepath.Rel(mountTaskPath, bin) + if mountRel == "" || err != nil { + return "", "", fmt.Errorf("path was not relative to the mount task path") + } + + hostPath := filepath.Join(mountHostPath, mountRel) + + err = filepathIsRegular(hostPath) + if err != nil { + return "", "", err + } + + // Turn relative-to-taskdir path into re-rooted absolute path to avoid + // libcontainer trying to resolve the binary using $PATH. + // Do *not* use filepath.Join as it will translate ".."s returned by + // filepath.Rel. Prepending "/" will cause the path to be rooted in the + // chroot which is the desired behavior. + return filepath.Clean("/" + bin), hostPath, nil +} + +// filepathIsRegular verifies that a filepath is a regular file (i.e. not a +// directory, socket, device, etc.) +func filepathIsRegular(path string) error { + f, err := os.Stat(path) + if err != nil { + return err + } + if !f.Mode().Type().IsRegular() { + return fmt.Errorf("path was not a regular file") + } + return nil } func newSetCPUSetCgroupHook(cgroupPath string) lconfigs.Hook { diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index d6b2d137f..8827c63b2 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -25,6 +25,8 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" lconfigs "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/devices" + "github.com/shoenig/test" + "github.com/shoenig/test/must" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" ) @@ -410,43 +412,121 @@ func TestExecutor_CgroupPathsAreDestroyed(t *testing.T) { } } -func TestUniversalExecutor_LookupTaskBin(t *testing.T) { +func TestExecutor_LookupTaskBin(t *testing.T) { ci.Parallel(t) - require := require.New(t) // Create a temp dir - tmpDir := t.TempDir() + taskDir := t.TempDir() + mountDir := t.TempDir() - // Create the command - cmd := &ExecCommand{Env: []string{"PATH=/bin"}, TaskDir: tmpDir} + // Create the command with mounts + cmd := &ExecCommand{ + Env: []string{"PATH=/bin"}, + TaskDir: taskDir, + Mounts: []*drivers.MountConfig{{TaskPath: "/srv", HostPath: mountDir}}, + } - // Make a foo subdir - os.MkdirAll(filepath.Join(tmpDir, "foo"), 0700) + // Make a /foo /local/foo and /usr/local/bin subdirs under task dir + // and /bar under mountdir + must.NoError(t, os.MkdirAll(filepath.Join(taskDir, "foo"), 0700)) + must.NoError(t, os.MkdirAll(filepath.Join(taskDir, "local/foo"), 0700)) + must.NoError(t, os.MkdirAll(filepath.Join(taskDir, "usr/local/bin"), 0700)) + must.NoError(t, os.MkdirAll(filepath.Join(mountDir, "bar"), 0700)) - // Write a file under foo - filePath := filepath.Join(tmpDir, "foo", "tmp.txt") - err := ioutil.WriteFile(filePath, []byte{1, 2}, os.ModeAppend) - require.NoError(err) + writeFile := func(paths ...string) { + t.Helper() + path := filepath.Join(paths...) + must.NoError(t, os.WriteFile(path, []byte("hello"), 0o700)) + } - // Lookout with an absolute path to the binary - cmd.Cmd = "/foo/tmp.txt" - _, err = lookupTaskBin(cmd) - require.NoError(err) + // Write some files + writeFile(taskDir, "usr/local/bin", "tmp0.txt") // under /usr/local/bin in taskdir + writeFile(taskDir, "foo", "tmp1.txt") // under foo in taskdir + writeFile(taskDir, "local", "tmp2.txt") // under root of task-local dir + writeFile(taskDir, "local/foo", "tmp3.txt") // under foo in task-local dir + writeFile(mountDir, "tmp4.txt") // under root of mount dir + writeFile(mountDir, "bar/tmp5.txt") // under bar in mount dir - // Write a file under local subdir - os.MkdirAll(filepath.Join(tmpDir, "local"), 0700) - filePath2 := filepath.Join(tmpDir, "local", "tmp.txt") - ioutil.WriteFile(filePath2, []byte{1, 2}, os.ModeAppend) + testCases := []struct { + name string + cmd string + expectErr string + expectTaskPath string + expectHostPath string + }{ + { + name: "lookup with file name in PATH", + cmd: "tmp0.txt", + expectTaskPath: "/usr/local/bin/tmp0.txt", + expectHostPath: filepath.Join(taskDir, "usr/local/bin/tmp0.txt"), + }, + { + name: "lookup with absolute path to binary", + cmd: "/foo/tmp1.txt", + expectTaskPath: "/foo/tmp1.txt", + expectHostPath: filepath.Join(taskDir, "foo/tmp1.txt"), + }, + { + name: "lookup in task local dir with absolute path to binary", + cmd: "/local/tmp2.txt", + expectTaskPath: "/local/tmp2.txt", + expectHostPath: filepath.Join(taskDir, "local/tmp2.txt"), + }, + { + name: "lookup in task local dir with relative path to binary", + cmd: "local/tmp2.txt", + expectTaskPath: "/local/tmp2.txt", + expectHostPath: filepath.Join(taskDir, "local/tmp2.txt"), + }, + { + name: "lookup in task local dir with file name", + cmd: "tmp2.txt", + expectTaskPath: "/local/tmp2.txt", + expectHostPath: filepath.Join(taskDir, "local/tmp2.txt"), + }, + { + name: "lookup in task local subdir with absolute path to binary", + cmd: "/local/foo/tmp3.txt", + expectTaskPath: "/local/foo/tmp3.txt", + expectHostPath: filepath.Join(taskDir, "local/foo/tmp3.txt"), + }, + { + name: "lookup host absolute path outside taskdir", + cmd: "/bin/sh", + expectErr: "file /bin/sh not found under path " + taskDir, + }, + { + name: "lookup file from mount with absolute path", + cmd: "/srv/tmp4.txt", + expectTaskPath: "/srv/tmp4.txt", + expectHostPath: filepath.Join(mountDir, "tmp4.txt"), + }, + { + name: "lookup file from mount with file name fails", + cmd: "tmp4.txt", + expectErr: "file tmp4.txt not found under path", + }, + { + name: "lookup file from mount with subdir", + cmd: "/srv/bar/tmp5.txt", + expectTaskPath: "/srv/bar/tmp5.txt", + expectHostPath: filepath.Join(mountDir, "bar/tmp5.txt"), + }, + } - // Lookup with file name, should find the one we wrote above - cmd.Cmd = "tmp.txt" - _, err = lookupTaskBin(cmd) - require.NoError(err) - - // Lookup a host absolute path - cmd.Cmd = "/bin/sh" - _, err = lookupTaskBin(cmd) - require.Error(err) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cmd.Cmd = tc.cmd + taskPath, hostPath, err := lookupTaskBin(cmd) + if tc.expectErr == "" { + must.NoError(t, err) + test.Eq(t, tc.expectTaskPath, taskPath) + test.Eq(t, tc.expectHostPath, hostPath) + } else { + test.EqError(t, err, tc.expectErr) + } + }) + } } // Exec Launch looks for the binary only inside the chroot diff --git a/website/content/docs/drivers/exec.mdx b/website/content/docs/drivers/exec.mdx index b2206d7ee..09bedadfa 100644 --- a/website/content/docs/drivers/exec.mdx +++ b/website/content/docs/drivers/exec.mdx @@ -31,9 +31,19 @@ The `exec` driver supports the following configuration in the job spec: - `command` - The command to execute. Must be provided. If executing a binary that exists on the host, the path must be absolute and within the task's - [chroot](#chroot). If executing a binary that is downloaded from - an [`artifact`](/docs/job-specification/artifact), the path can be - relative from the allocations's root directory. + [chroot](#chroot) or in a [host volume][] mounted with a + [`volume_mount`][volume_mount] block. The driver will make the binary + executable and will search, in order: + + - The `local` directory with the task directory. + - The task directory. + - Any mounts, in the order listed in the job specification. + - The `usr/local/bin`, `usr/bin` and `bin` directories inside the task + directory. + + If executing a binary that is downloaded + from an [`artifact`](/docs/job-specification/artifact), the path can be + relative from the allocation's root directory. - `args` - (Optional) A list of arguments to the `command`. References to environment variables or any [interpretable Nomad @@ -243,3 +253,5 @@ This list is configurable through the agent client [no_net_raw]: /docs/upgrade/upgrade-specific#nomad-1-1-0-rc1-1-0-5-0-12-12 [allow_caps]: /docs/drivers/exec#allow_caps [docker_caps]: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities +[host volume]: /docs/configuration/client#host_volume-stanza +[volume_mount]: /docs/job-specification/volume_mount