Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix env templates having interpolated destinations #5253

Merged
merged 2 commits into from
Jan 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
12 changes: 7 additions & 5 deletions client/allocrunner/taskrunner/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
Expand Down
43 changes: 40 additions & 3 deletions client/allocrunner/taskrunner/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"])
Expand Down
9 changes: 9 additions & 0 deletions client/taskenv/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ func NewTaskEnv(env, deviceEnv, node map[string]string) *TaskEnv {
}
}

// NewEmptyTaskEnv creates a new empty task environment.
func NewEmptyTaskEnv() *TaskEnv {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot believe we never needed this before, but it's handy I imagine

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 {
Expand Down