artifact: enable inheriting environment variables from client (#15514)

* artifact: enable inheriting environment variables from client

This PR adds client configuration for specifying environment variables that
should be inherited by the artifact sandbox process from the Nomad Client agent.

Most users should not need to set these values but the configuration is provided
to ensure backwards compatability. Configuration of go-getter should ideally be
done through the artifact block in a jobspec task.

e.g.

```hcl
client {
  artifact {
    set_environment_variables = "TMPDIR,GIT_SSH_OPTS"
  }
}
```

Closes #15498

* website: update set_environment_variables text to mention PATH

client/allocrunner/taskrunner/getter/params.go

@@ -28,6 +28,7 @@ type parameters struct {
 	HgTimeout                  time.Duration `json:"hg_timeout"`
 	S3Timeout                  time.Duration `json:"s3_timeout"`
 	DisableFilesystemIsolation bool          `json:"disable_filesystem_isolation"`
+	SetEnvironmentVariables    string        `json:"set_environment_variables"`
 
 	// Artifact
 	Mode        getter.ClientMode   `json:"artifact_mode"`
@@ -88,6 +89,8 @@ func (p *parameters) Equal(o *parameters) bool {
 		return false
 	case p.DisableFilesystemIsolation != o.DisableFilesystemIsolation:
 		return false
+	case p.SetEnvironmentVariables != o.SetEnvironmentVariables:
+		return false
 	case p.Mode != o.Mode:
 		return false
 	case p.Source != o.Source:

client/allocrunner/taskrunner/getter/params_test.go

@@ -20,6 +20,7 @@ const paramsAsJSON = `
   "hg_timeout": 4000000000,
   "s3_timeout": 5000000000,
   "disable_filesystem_isolation": true,
+  "set_environment_variables": "",
   "artifact_mode": 2,
   "artifact_source": "https://example.com/file.txt",
   "artifact_destination": "local/out.txt",

client/allocrunner/taskrunner/getter/sandbox.go

@@ -47,6 +47,7 @@ func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact
 		HgTimeout:                  s.ac.HgTimeout,
 		S3Timeout:                  s.ac.S3Timeout,
 		DisableFilesystemIsolation: s.ac.DisableFilesystemIsolation,
+		SetEnvironmentVariables:    s.ac.SetEnvironmentVariables,
 
 		// artifact configuration
 		Mode:        mode,

client/allocrunner/taskrunner/getter/util.go

@@ -5,9 +5,12 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"os"
 	"os/exec"
 	"path/filepath"
+	"sort"
 	"strings"
+	"unicode"
 
 	"github.com/hashicorp/go-getter"
 	"github.com/hashicorp/nomad/client/interfaces"
@@ -95,6 +98,26 @@ func getTaskDir(env interfaces.EnvReplacer) string {
 	return filepath.Dir(p)
 }
 
+// environment merges the default minimal environment per-OS with the set of
+// environment variables configured to be inherited from the Client
+func environment(taskDir string, inherit string) []string {
+	chomp := func(s string) []string {
+		return strings.FieldsFunc(s, func(c rune) bool {
+			return c == ',' || unicode.IsSpace(c)
+		})
+	}
+	env := defaultEnvironment(taskDir)
+	for _, name := range chomp(inherit) {
+		env[name] = os.Getenv(name)
+	}
+	result := make([]string, 0, len(env))
+	for k, v := range env {
+		result = append(result, fmt.Sprintf("%s=%s", k, v))
+	}
+	sort.Strings(result)
+	return result
+}
+
 func (s *Sandbox) runCmd(env *parameters) error {
 	// find the nomad process
 	bin := subproc.Self()
@@ -106,7 +129,7 @@ func (s *Sandbox) runCmd(env *parameters) error {
 	// start the subprocess, passing in parameters via stdin
 	output := new(bytes.Buffer)
 	cmd := exec.CommandContext(ctx, bin, SubCommand)
-	cmd.Env = minimalVars(env.TaskDir)
+	cmd.Env = environment(env.TaskDir, env.SetEnvironmentVariables)
 	cmd.Stdin = env.reader()
 	cmd.Stdout = output
 	cmd.Stderr = output

client/allocrunner/taskrunner/getter/util_default.go

@@ -3,7 +3,6 @@
 package getter
 
 import (
-	"fmt"
 	"path/filepath"
 	"syscall"
 )
@@ -27,13 +26,13 @@ func credentials() (uint32, uint32) {
 	return uint32(uid), uint32(gid)
 }
 
-// minimalVars returns the minimal environment set for artifact
-// downloader sandbox
-func minimalVars(taskDir string) []string {
+// defaultEnvironment is the default minimal environment variables for Unix-like
+// operating systems.
+func defaultEnvironment(taskDir string) map[string]string {
 	tmpDir := filepath.Join(taskDir, "tmp")
-	return []string{
-		fmt.Sprintf("PATH=/usr/local/bin:/usr/bin:/bin"),
-		fmt.Sprintf("TMPDIR=%s", tmpDir),
+	return map[string]string{
+		"PATH":   "/usr/local/bin:/usr/bin:/bin",
+		"TMPDIR": tmpDir,
 	}
 }
 

client/allocrunner/taskrunner/getter/util_linux.go

@@ -3,7 +3,6 @@
 package getter
 
 import (
-	"fmt"
 	"path/filepath"
 	"syscall"
 
@@ -49,13 +48,12 @@ func credentials() (uint32, uint32) {
 	}
 }
 
-// minimalVars returns the minimal environment set for artifact
-// downloader sandbox
-func minimalVars(taskDir string) []string {
+// defaultEnvironment is the default minimal environment variables for Linux.
+func defaultEnvironment(taskDir string) map[string]string {
 	tmpDir := filepath.Join(taskDir, "tmp")
-	return []string{
-		"PATH=/usr/local/bin:/usr/bin:/bin",
-		fmt.Sprintf("TMPDIR=%s", tmpDir),
+	return map[string]string{
+		"PATH":   "/usr/local/bin:/usr/bin:/bin",
+		"TMPDIR": tmpDir,
 	}
 }
 

client/allocrunner/taskrunner/getter/util_test.go

@@ -2,15 +2,18 @@ package getter
 
 import (
 	"errors"
-	"runtime"
 	"testing"
 
 	"github.com/hashicorp/go-getter"
+	"github.com/hashicorp/nomad/ci"
+	"github.com/hashicorp/nomad/client/testutil"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/shoenig/test/must"
 )
 
 func TestUtil_getURL(t *testing.T) {
+	ci.Parallel(t)
+
 	cases := []struct {
 		name     string
 		artifact *structs.TaskArtifact
@@ -61,6 +64,8 @@ func TestUtil_getURL(t *testing.T) {
 }
 
 func TestUtil_getDestination(t *testing.T) {
+	ci.Parallel(t)
+
 	env := noopTaskEnv("/path/to/task")
 	t.Run("ok", func(t *testing.T) {
 		result, err := getDestination(env, &structs.TaskArtifact{
@@ -80,6 +85,8 @@ func TestUtil_getDestination(t *testing.T) {
 }
 
 func TestUtil_getMode(t *testing.T) {
+	ci.Parallel(t)
+
 	cases := []struct {
 		mode string
 		exp  getter.ClientMode
@@ -99,6 +106,8 @@ func TestUtil_getMode(t *testing.T) {
 }
 
 func TestUtil_getHeaders(t *testing.T) {
+	ci.Parallel(t)
+
 	env := upTaskEnv("/path/to/task")
 
 	t.Run("empty", func(t *testing.T) {
@@ -123,26 +132,53 @@ func TestUtil_getHeaders(t *testing.T) {
 }
 
 func TestUtil_getTaskDir(t *testing.T) {
+	ci.Parallel(t)
+
 	env := noopTaskEnv("/path/to/task")
 	result := getTaskDir(env)
 	must.Eq(t, "/path/to/task", result)
 }
 
-func TestUtil_minimalVars(t *testing.T) {
-	var exp []string
-	switch runtime.GOOS {
-	case "windows":
-		exp = []string{
-			`PATH=C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;`,
-			`TMP=C:\path\to\task\tmp`,
-			`TEMP=C:\path\to\task\tmp`,
-		}
-	default:
-		exp = []string{
+func TestUtil_environment(t *testing.T) {
+	// not parallel
+	testutil.RequireLinux(t)
+
+	t.Run("default", func(t *testing.T) {
+		result := environment("/a/b/c", "")
+		must.Eq(t, []string{
 			"PATH=/usr/local/bin:/usr/bin:/bin",
-			"TMPDIR=/path/to/task/tmp",
-		}
-	}
-	result := minimalVars("/path/to/task")
-	must.Eq(t, exp, result)
+			"TMPDIR=/a/b/c/tmp",
+		}, result)
+	})
+
+	t.Run("append", func(t *testing.T) {
+		t.Setenv("ONE", "1")
+		t.Setenv("TWO", "2")
+		result := environment("/a/b/c", "ONE,TWO")
+		must.Eq(t, []string{
+			"ONE=1",
+			"PATH=/usr/local/bin:/usr/bin:/bin",
+			"TMPDIR=/a/b/c/tmp",
+			"TWO=2",
+		}, result)
+	})
+
+	t.Run("override", func(t *testing.T) {
+		t.Setenv("PATH", "/opt/bin")
+		t.Setenv("TMPDIR", "/scratch")
+		result := environment("/a/b/c", "PATH,TMPDIR")
+		must.Eq(t, []string{
+			"PATH=/opt/bin",
+			"TMPDIR=/scratch",
+		}, result)
+	})
+
+	t.Run("missing", func(t *testing.T) {
+		result := environment("/a/b/c", "DOES_NOT_EXIST")
+		must.Eq(t, []string{
+			"DOES_NOT_EXIST=",
+			"PATH=/usr/local/bin:/usr/bin:/bin",
+			"TMPDIR=/a/b/c/tmp",
+		}, result)
+	})
 }

client/allocrunner/taskrunner/getter/util_windows.go

@@ -3,7 +3,6 @@
 package getter
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"syscall"
@@ -24,14 +23,15 @@ func lockdown(string) error {
 	return nil
 }
 
-func minimalVars(taskDir string) []string {
+// defaultEnvironment is the default minimal environment variables for Windows.
+func defaultEnvironment(taskDir string) map[string]string {
 	tmpDir := filepath.Join(taskDir, "tmp")
-	return []string{
-		fmt.Sprintf("HOMEPATH=%s", os.Getenv("HOMEPATH")),
-		fmt.Sprintf("HOMEDRIVE=%s", os.Getenv("HOMEDRIVE")),
-		fmt.Sprintf("USERPROFILE=%s", os.Getenv("USERPROFILE")),
-		fmt.Sprintf("PATH=%s", os.Getenv("PATH")),
-		fmt.Sprintf("TMP=%s", tmpDir),
-		fmt.Sprintf("TEMP=%s", tmpDir),
+	return map[string]string{
+		"HOMEPATH":    os.Getenv("HOMEPATH"),
+		"HOMEDRIVE":   os.Getenv("HOMEDRIVE"),
+		"USERPROFILE": os.Getenv("USERPROFILE"),
+		"PATH":        os.Getenv("PATH"),
+		"TMP":         tmpDir,
+		"TEMP":        tmpDir,
 	}
 }

client/config/artifact.go

@@ -20,6 +20,7 @@ type ArtifactConfig struct {
 	S3Timeout  time.Duration
 
 	DisableFilesystemIsolation bool
+	SetEnvironmentVariables    string
 }
 
 // ArtifactConfigFromAgent creates a new internal readonly copy of the client
@@ -63,6 +64,7 @@ func ArtifactConfigFromAgent(c *config.ArtifactConfig) (*ArtifactConfig, error)
 		HgTimeout:                  hgTimeout,
 		S3Timeout:                  s3Timeout,
 		DisableFilesystemIsolation: *c.DisableFilesystemIsolation,
+		SetEnvironmentVariables:    *c.SetEnvironmentVariables,
 	}, nil
 }