Backport of exec: allow running commands from host volume into releas…

…e/1.4.x (#15216)

This pull request was automerged via backport-assistant

.changelog/14851.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+exec: Allow running commands from mounted host volumes
+```

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"}
 
-// 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
+	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 %s", bin, root)
+
+	return "", "", fmt.Errorf("file %s not found under path", bin)
+}
+
+// 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
+	}
+
+	// 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 {

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()
-
-	// Create the command
-	cmd := &ExecCommand{Env: []string{"PATH=/bin"}, TaskDir: tmpDir}
-
-	// Make a foo subdir
-	os.MkdirAll(filepath.Join(tmpDir, "foo"), 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)
+	taskDir := t.TempDir()
+	mountDir := t.TempDir()
 
-	// Lookout with an absolute path to the binary
-	cmd.Cmd = "/foo/tmp.txt"
-	_, err = lookupTaskBin(cmd)
-	require.NoError(err)
+	// Create the command with mounts
+	cmd := &ExecCommand{
+		Env:     []string{"PATH=/bin"},
+		TaskDir: taskDir,
+		Mounts:  []*drivers.MountConfig{{TaskPath: "/srv", HostPath: mountDir}},
+	}
 
-	// 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)
+	// 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))
+
+	writeFile := func(paths ...string) {
+		t.Helper()
+		path := filepath.Join(paths...)
+		must.NoError(t, os.WriteFile(path, []byte("hello"), 0o700))
+	}
 
-	// Lookup with file name, should find the one we wrote above
-	cmd.Cmd = "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
+
+	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 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

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