Merge pull request #5253 from hashicorp/b-template-env

Fix env templates having interpolated destinations

CHANGELOG.md

@@ -55,8 +55,10 @@ BUG FIXES:
  * client: Fix an issue where driver attributes are not updated in node API responses if they change after after startup [[GH-4984](https://github.com/hashicorp/nomad/pull/4984)]
  * driver/docker: Fix a path traversal issue where mounting paths outside alloc dir might be possible despite `docker.volumes.enabled` set to false [[GH-4983](https://github.com/hashicorp/nomad/pull/4983)]
  * driver/raw_exec: Fix an issue where tasks that used an interpolated command in driver configuration would not start [[GH-4813](https://github.com/hashicorp/nomad/pull/4813)]
- * server/vault: Fixed bug in Vault token renewal that could panic on a malformed Vault response [[GH-4904](https://github.com/hashicorp/nomad/issues/4904)], [[GH-4937](https://github.com/hashicorp/nomad/pull/4937)]
  * quota: Fixed a bug in Nomad enterprise where quota specifications were not being replicated to non authoritative regions correctly.
+ * server/vault: Fixed bug in Vault token renewal that could panic on a malformed Vault response [[GH-4904](https://github.com/hashicorp/nomad/issues/4904)], [[GH-4937](https://github.com/hashicorp/nomad/pull/4937)]
+ * template: Fix parsing of environment templates when destination path is
+   interpolated [[GH-5253](https://github.com/hashicorp/nomad/issues/5253)]
  * ui: Fixed an issue where distribution bar corners weren't rounded when there was only one or two slices in the chart [[GH-4507](https://github.com/hashicorp/nomad/issues/4507)]
  * ui: Fixed an issue where dispatched jobs would get the wrong template type which could cause runtime errors [[GH-4852](https://github.com/hashicorp/nomad/issues/4852)]
  * ui: Added an empty state for the tasks list on the allocation detail page, for when an alloc has no tasks [[GH-4860](https://github.com/hashicorp/nomad/issues/4860)]

client/allocrunner/taskrunner/template/template.go

@@ -205,7 +205,7 @@ func (tm *TaskTemplateManager) run() {
 	}
 
 	// Read environment variables from env templates before we unblock
-	envMap, err := loadTemplateEnv(tm.config.Templates, tm.config.TaskDir)
+	envMap, err := loadTemplateEnv(tm.config.Templates, tm.config.TaskDir, tm.config.EnvBuilder.Build())
 	if err != nil {
 		tm.config.Lifecycle.Kill(context.Background(),
 			structs.NewTaskEvent(structs.TaskKilling).
@@ -393,7 +393,7 @@ func (tm *TaskTemplateManager) handleTemplateRerenders(allRenderedTime time.Time
 				}
 
 				// Read environment variables from templates
-				envMap, err := loadTemplateEnv(tm.config.Templates, tm.config.TaskDir)
+				envMap, err := loadTemplateEnv(tm.config.Templates, tm.config.TaskDir, tm.config.EnvBuilder.Build())
 				if err != nil {
 					tm.config.Lifecycle.Kill(context.Background(),
 						structs.NewTaskEvent(structs.TaskKilling).
@@ -676,13 +676,15 @@ func newRunnerConfig(config *TaskTemplateManagerConfig,
 }
 
 // loadTemplateEnv loads task environment variables from all templates.
-func loadTemplateEnv(tmpls []*structs.Template, taskDir string) (map[string]string, error) {
+func loadTemplateEnv(tmpls []*structs.Template, taskDir string, taskEnv *taskenv.TaskEnv) (map[string]string, error) {
 	all := make(map[string]string, 50)
 	for _, t := range tmpls {
 		if !t.Envvars {
 			continue
 		}
-		f, err := os.Open(filepath.Join(taskDir, t.DestPath))
+
+		dest := filepath.Join(taskDir, taskEnv.ReplaceEnv(t.DestPath))
+		f, err := os.Open(dest)
 		if err != nil {
 			return nil, fmt.Errorf("error opening env template: %v", err)
 		}
@@ -691,7 +693,7 @@ func loadTemplateEnv(tmpls []*structs.Template, taskDir string) (map[string]stri
 		// Parse environment fil
 		vars, err := envparse.Parse(f)
 		if err != nil {
-			return nil, fmt.Errorf("error parsing env template %q: %v", t.DestPath, err)
+			return nil, fmt.Errorf("error parsing env template %q: %v", dest, err)
 		}
 		for k, v := range vars {
 			all[k] = v

client/allocrunner/taskrunner/template/template_test.go

@@ -1087,11 +1087,48 @@ func TestTaskTemplateManager_Env_Missing(t *testing.T) {
 		},
 	}
 
-	if vars, err := loadTemplateEnv(templates, d); err == nil {
+	if vars, err := loadTemplateEnv(templates, d, taskenv.NewEmptyTaskEnv()); err == nil {
 		t.Fatalf("expected an error but instead got env vars: %#v", vars)
 	}
 }
 
+// TestTaskTemplateManager_Env_InterpolatedDest asserts the core env
+// template processing function handles interpolated destinations
+func TestTaskTemplateManager_Env_InterpolatedDest(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+
+	d, err := ioutil.TempDir("", "ct_env_interpolated")
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	defer os.RemoveAll(d)
+
+	// Fake writing the file so we don't have to run the whole template manager
+	err = ioutil.WriteFile(filepath.Join(d, "exists.env"), []byte("FOO=bar\n"), 0644)
+	if err != nil {
+		t.Fatalf("error writing template file: %v", err)
+	}
+
+	templates := []*structs.Template{
+		{
+			EmbeddedTmpl: "FOO=bar\n",
+			DestPath:     "${NOMAD_META_path}.env",
+			Envvars:      true,
+		},
+	}
+
+	// Build the env
+	taskEnv := taskenv.NewTaskEnv(
+		map[string]string{"NOMAD_META_path": "exists"},
+		map[string]string{}, map[string]string{})
+
+	vars, err := loadTemplateEnv(templates, d, taskEnv)
+	require.NoError(err)
+	require.Contains(vars, "FOO")
+	require.Equal(vars["FOO"], "bar")
+}
+
 // TestTaskTemplateManager_Env_Multi asserts the core env
 // template processing function returns combined env vars from multiple
 // templates correctly.
@@ -1125,9 +1162,9 @@ func TestTaskTemplateManager_Env_Multi(t *testing.T) {
 		},
 	}
 
-	vars, err := loadTemplateEnv(templates, d)
+	vars, err := loadTemplateEnv(templates, d, taskenv.NewEmptyTaskEnv())
 	if err != nil {
-		t.Fatalf("expected an error but instead got env vars: %#v", vars)
+		t.Fatalf("expected no error: %v", err)
 	}
 	if vars["FOO"] != "bar" {
 		t.Errorf("expected FOO=bar but found %q", vars["FOO"])

client/taskenv/env.go

@@ -126,6 +126,15 @@ func NewTaskEnv(env, deviceEnv, node map[string]string) *TaskEnv {
 	}
 }
 
+// NewEmptyTaskEnv creates a new empty task environment.
+func NewEmptyTaskEnv() *TaskEnv {
+	return &TaskEnv{
+		NodeAttrs: make(map[string]string),
+		deviceEnv: make(map[string]string),
+		EnvMap:    make(map[string]string),
+	}
+}
+
 // List returns the task's environment as a slice of NAME=value pair strings.
 func (t *TaskEnv) List() []string {
 	if t.envList != nil {