hashicorp · hc-github-team-nomad-core · Nov 4, 2022 · Nov 3, 2022 · Nov 3, 2022 · Nov 4, 2022
diff --git a/.changelog/14002.txt b/.changelog/14002.txt
@@ -0,0 +1,5 @@
+```release-note:improvement
+template: Expose per-template configuration for `error_on_missing_key`. This allows jobspec authors to specify that a
+template should fail if it references a struct or map key that does not exist. The default value is false and should be
+fully backward compatible.
+```
diff --git a/api/jobs_test.go b/api/jobs_test.go
@@ -758,30 +758,32 @@ func TestJobs_Canonicalize(t *testing.T) {
 								LogConfig:   DefaultLogConfig(),
 								Templates: []*Template{
 									{
-										SourcePath:   pointerOf(""),
-										DestPath:     pointerOf("local/file.yml"),
-										EmbeddedTmpl: pointerOf("---"),
-										ChangeMode:   pointerOf("restart"),
-										ChangeSignal: pointerOf(""),
-										Splay:        pointerOf(5 * time.Second),
-										Perms:        pointerOf("0644"),
-										LeftDelim:    pointerOf("{{"),
-										RightDelim:   pointerOf("}}"),
-										Envvars:      pointerOf(false),
-										VaultGrace:   pointerOf(time.Duration(0)),
+										SourcePath:    pointerOf(""),
+										DestPath:      pointerOf("local/file.yml"),
+										EmbeddedTmpl:  pointerOf("---"),
+										ChangeMode:    pointerOf("restart"),
+										ChangeSignal:  pointerOf(""),
+										Splay:         pointerOf(5 * time.Second),
+										Perms:         pointerOf("0644"),
+										LeftDelim:     pointerOf("{{"),
+										RightDelim:    pointerOf("}}"),
+										Envvars:       pointerOf(false),
+										VaultGrace:    pointerOf(time.Duration(0)),
+										ErrMissingKey: pointerOf(false),
 									},
 									{
-										SourcePath:   pointerOf(""),
-										DestPath:     pointerOf("local/file.env"),
-										EmbeddedTmpl: pointerOf("FOO=bar\n"),
-										ChangeMode:   pointerOf("restart"),
-										ChangeSignal: pointerOf(""),
-										Splay:        pointerOf(5 * time.Second),
-										Perms:        pointerOf("0644"),
-										LeftDelim:    pointerOf("{{"),
-										RightDelim:   pointerOf("}}"),
-										Envvars:      pointerOf(true),
-										VaultGrace:   pointerOf(time.Duration(0)),
+										SourcePath:    pointerOf(""),
+										DestPath:      pointerOf("local/file.env"),
+										EmbeddedTmpl:  pointerOf("FOO=bar\n"),
+										ChangeMode:    pointerOf("restart"),
+										ChangeSignal:  pointerOf(""),
+										Splay:         pointerOf(5 * time.Second),
+										Perms:         pointerOf("0644"),
+										LeftDelim:     pointerOf("{{"),
+										RightDelim:    pointerOf("}}"),
+										Envvars:       pointerOf(true),
+										VaultGrace:    pointerOf(time.Duration(0)),
+										ErrMissingKey: pointerOf(false),
 									},
 								},
 							},

diff --git a/api/tasks.go b/api/tasks.go
@@ -832,21 +832,22 @@ func (ch *ChangeScript) Canonicalize() {
 }
 
 type Template struct {
-	SourcePath   *string        `mapstructure:"source" hcl:"source,optional"`
-	DestPath     *string        `mapstructure:"destination" hcl:"destination,optional"`
-	EmbeddedTmpl *string        `mapstructure:"data" hcl:"data,optional"`
-	ChangeMode   *string        `mapstructure:"change_mode" hcl:"change_mode,optional"`
-	ChangeScript *ChangeScript  `mapstructure:"change_script" hcl:"change_script,block"`
-	ChangeSignal *string        `mapstructure:"change_signal" hcl:"change_signal,optional"`
-	Splay        *time.Duration `mapstructure:"splay" hcl:"splay,optional"`
-	Perms        *string        `mapstructure:"perms" hcl:"perms,optional"`
-	Uid          *int           `mapstructure:"uid" hcl:"uid,optional"`
-	Gid          *int           `mapstructure:"gid" hcl:"gid,optional"`
-	LeftDelim    *string        `mapstructure:"left_delimiter" hcl:"left_delimiter,optional"`
-	RightDelim   *string        `mapstructure:"right_delimiter" hcl:"right_delimiter,optional"`
-	Envvars      *bool          `mapstructure:"env" hcl:"env,optional"`
-	VaultGrace   *time.Duration `mapstructure:"vault_grace" hcl:"vault_grace,optional"`
-	Wait         *WaitConfig    `mapstructure:"wait" hcl:"wait,block"`
+	SourcePath    *string        `mapstructure:"source" hcl:"source,optional"`
+	DestPath      *string        `mapstructure:"destination" hcl:"destination,optional"`
+	EmbeddedTmpl  *string        `mapstructure:"data" hcl:"data,optional"`
+	ChangeMode    *string        `mapstructure:"change_mode" hcl:"change_mode,optional"`
+	ChangeScript  *ChangeScript  `mapstructure:"change_script" hcl:"change_script,block"`
+	ChangeSignal  *string        `mapstructure:"change_signal" hcl:"change_signal,optional"`
+	Splay         *time.Duration `mapstructure:"splay" hcl:"splay,optional"`
+	Perms         *string        `mapstructure:"perms" hcl:"perms,optional"`
+	Uid           *int           `mapstructure:"uid" hcl:"uid,optional"`
+	Gid           *int           `mapstructure:"gid" hcl:"gid,optional"`
+	LeftDelim     *string        `mapstructure:"left_delimiter" hcl:"left_delimiter,optional"`
+	RightDelim    *string        `mapstructure:"right_delimiter" hcl:"right_delimiter,optional"`
+	Envvars       *bool          `mapstructure:"env" hcl:"env,optional"`
+	VaultGrace    *time.Duration `mapstructure:"vault_grace" hcl:"vault_grace,optional"`
+	Wait          *WaitConfig    `mapstructure:"wait" hcl:"wait,block"`
+	ErrMissingKey *bool          `mapstructure:"error_on_missing_key" hcl:"error_on_missing_key,optional"`
 }
 
 func (tmpl *Template) Canonicalize() {
@@ -890,7 +891,9 @@ func (tmpl *Template) Canonicalize() {
 	if tmpl.Envvars == nil {
 		tmpl.Envvars = pointerOf(false)
 	}
-
+	if tmpl.ErrMissingKey == nil {
+		tmpl.ErrMissingKey = pointerOf(false)
+	}
 	//COMPAT(0.12) VaultGrace is deprecated and unused as of Vault 0.5
 	if tmpl.VaultGrace == nil {
 		tmpl.VaultGrace = pointerOf(time.Duration(0))

diff --git a/client/allocrunner/taskrunner/template/template.go b/client/allocrunner/taskrunner/template/template.go
@@ -694,6 +694,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
 		ct.Contents = &tmpl.EmbeddedTmpl
 		ct.LeftDelim = &tmpl.LeftDelim
 		ct.RightDelim = &tmpl.RightDelim
+		ct.ErrMissingKey = &tmpl.ErrMissingKey
 		ct.FunctionDenylist = config.ClientConfig.TemplateConfig.FunctionDenylist
 		if sandboxEnabled {
 			ct.SandboxPath = &config.TaskDir

diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go
@@ -2469,6 +2469,46 @@ func TestTaskTemplateManager_Template_Wait_Set(t *testing.T) {
 	}
 }
 
+// TestTaskTemplateManager_Template_ErrMissingKey_Set asserts that all template level
+// configuration is accurately mapped from the template to the TaskTemplateManager's
+// template config.
+func TestTaskTemplateManager_Template_ErrMissingKey_Set(t *testing.T) {
+	ci.Parallel(t)
+
+	c := config.DefaultConfig()
+	c.Node = mock.Node()
+
+	alloc := mock.Alloc()
+
+	ttmConfig := &TaskTemplateManagerConfig{
+		ClientConfig: c,
+		VaultToken:   "token",
+		EnvBuilder:   taskenv.NewBuilder(c.Node, alloc, alloc.Job.TaskGroups[0].Tasks[0], c.Region),
+		Templates: []*structs.Template{
+			{
+				EmbeddedTmpl:  "test-false",
+				ErrMissingKey: false,
+			},
+			{
+				EmbeddedTmpl:  "test-true",
+				ErrMissingKey: true,
+			},
+		},
+	}
+
+	templateMapping, err := parseTemplateConfigs(ttmConfig)
+	require.NoError(t, err)
+
+	for k, tmpl := range templateMapping {
+		if tmpl.EmbeddedTmpl == "test-false" {
+			require.False(t, *k.ErrMissingKey)
+		}
+		if tmpl.EmbeddedTmpl == "test-true" {
+			require.True(t, *k.ErrMissingKey)
+		}
+	}
+}
+
 // TestTaskTemplateManager_writeToFile_Disabled asserts the consul-template function
 // writeToFile is disabled by default.
 func TestTaskTemplateManager_writeToFile_Disabled(t *testing.T) {

diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go
@@ -1222,21 +1222,22 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
 		for _, template := range apiTask.Templates {
 			structsTask.Templates = append(structsTask.Templates,
 				&structs.Template{
-					SourcePath:   *template.SourcePath,
-					DestPath:     *template.DestPath,
-					EmbeddedTmpl: *template.EmbeddedTmpl,
-					ChangeMode:   *template.ChangeMode,
-					ChangeSignal: *template.ChangeSignal,
-					ChangeScript: apiChangeScriptToStructsChangeScript(template.ChangeScript),
-					Splay:        *template.Splay,
-					Perms:        *template.Perms,
-					Uid:          template.Uid,
-					Gid:          template.Gid,
-					LeftDelim:    *template.LeftDelim,
-					RightDelim:   *template.RightDelim,
-					Envvars:      *template.Envvars,
-					VaultGrace:   *template.VaultGrace,
-					Wait:         apiWaitConfigToStructsWaitConfig(template.Wait),
+					SourcePath:    *template.SourcePath,
+					DestPath:      *template.DestPath,
+					EmbeddedTmpl:  *template.EmbeddedTmpl,
+					ChangeMode:    *template.ChangeMode,
+					ChangeSignal:  *template.ChangeSignal,
+					ChangeScript:  apiChangeScriptToStructsChangeScript(template.ChangeScript),
+					Splay:         *template.Splay,
+					Perms:         *template.Perms,
+					Uid:           template.Uid,
+					Gid:           template.Gid,
+					LeftDelim:     *template.LeftDelim,
+					RightDelim:    *template.RightDelim,
+					Envvars:       *template.Envvars,
+					VaultGrace:    *template.VaultGrace,
+					Wait:          apiWaitConfigToStructsWaitConfig(template.Wait),
+					ErrMissingKey: *template.ErrMissingKey,
 				})
 		}
 	}

diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go
@@ -2747,6 +2747,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 									Min: pointer.Of(5 * time.Second),
 									Max: pointer.Of(10 * time.Second),
 								},
+								ErrMissingKey: pointer.Of(true),
 							},
 						},
 						DispatchPayload: &api.DispatchPayloadConfig{
@@ -3160,6 +3161,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 									Min: pointer.Of(5 * time.Second),
 									Max: pointer.Of(10 * time.Second),
 								},
+								ErrMissingKey: true,
 							},
 						},
 						DispatchPayload: &structs.DispatchPayloadConfig{

diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go
@@ -9,6 +9,7 @@ import (
 	"github.com/hashicorp/hcl"
 	"github.com/hashicorp/hcl/hcl/ast"
 	"github.com/hashicorp/nomad/api"
+	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/mitchellh/mapstructure"
 )
 
@@ -458,6 +459,8 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error {
 			"splay",
 			"env",
 			"vault_grace", //COMPAT(0.12) not used; emits warning in 0.11.
+			"wait",
+			"error_on_missing_key",
 		}
 		if err := checkHCLKeys(o.Val, valid); err != nil {
 			return err
@@ -470,9 +473,12 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error {
 		delete(m, "change_script") // change_script is its own object
 
 		templ := &api.Template{
-			ChangeMode: stringToPtr("restart"),
-			Splay:      timeToPtr(5 * time.Second),
-			Perms:      stringToPtr("0644"),
+			ChangeMode:    stringToPtr("restart"),
+			Splay:         timeToPtr(5 * time.Second),
+			Perms:         stringToPtr("0644"),
+			Uid:           pointer.Of(-1),
+			Gid:           pointer.Of(-1),
+			ErrMissingKey: pointer.Of(false),
 		}
 
 		dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{

diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go
@@ -372,14 +372,17 @@ func TestParse(t *testing.T) {
 								},
 								Templates: []*api.Template{
 									{
-										SourcePath:   stringToPtr("foo"),
-										DestPath:     stringToPtr("foo"),
-										ChangeMode:   stringToPtr("foo"),
-										ChangeSignal: stringToPtr("foo"),
-										Splay:        timeToPtr(10 * time.Second),
-										Perms:        stringToPtr("0644"),
-										Envvars:      boolToPtr(true),
-										VaultGrace:   timeToPtr(33 * time.Second),
+										SourcePath:    stringToPtr("foo"),
+										DestPath:      stringToPtr("foo"),
+										ChangeMode:    stringToPtr("foo"),
+										ChangeSignal:  stringToPtr("foo"),
+										Splay:         timeToPtr(10 * time.Second),
+										Perms:         stringToPtr("0644"),
+										Envvars:       boolToPtr(true),
+										Uid:           intToPtr(-1),
+										Gid:           intToPtr(-1),
+										VaultGrace:    timeToPtr(33 * time.Second),
+										ErrMissingKey: boolToPtr(true),
 									},
 									{
 										SourcePath: stringToPtr("bar"),
@@ -391,12 +394,13 @@ func TestParse(t *testing.T) {
 											Timeout:     timeToPtr(5 * time.Second),
 											FailOnError: boolToPtr(false),
 										},
-										Splay:      timeToPtr(5 * time.Second),
-										Perms:      stringToPtr("777"),
-										Uid:        intToPtr(1001),
-										Gid:        intToPtr(20),
-										LeftDelim:  stringToPtr("--"),
-										RightDelim: stringToPtr("__"),
+										Splay:         timeToPtr(5 * time.Second),
+										Perms:         stringToPtr("777"),
+										Uid:           intToPtr(1001),
+										Gid:           intToPtr(20),
+										LeftDelim:     stringToPtr("--"),
+										RightDelim:    stringToPtr("__"),
+										ErrMissingKey: boolToPtr(false),
 									},
 								},
 								Leader:     true,

diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl
@@ -305,13 +305,14 @@ job "binstore-storagelocker" {
       }
 
       template {
-        source        = "foo"
-        destination   = "foo"
-        change_mode   = "foo"
-        change_signal = "foo"
-        splay         = "10s"
-        env           = true
-        vault_grace   = "33s"
+        source               = "foo"
+        destination          = "foo"
+        change_mode          = "foo"
+        change_signal        = "foo"
+        splay                = "10s"
+        env                  = true
+        vault_grace          = "33s"
+        error_on_missing_key = true
       }
 
       template {

diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go
@@ -111,6 +111,9 @@ func normalizeTemplates(templates []*api.Template) {
 		if t.Splay == nil {
 			t.Splay = pointer.Of(5 * time.Second)
 		}
+		if t.ErrMissingKey == nil {
+			t.ErrMissingKey = pointer.Of(false)
+		}
 		normalizeChangeScript(t.ChangeScript)
 	}
 }

diff --git a/jobspec2/parse_test.go b/jobspec2/parse_test.go
@@ -1052,3 +1052,19 @@ func TestWaitConfig(t *testing.T) {
 	require.Equal(t, 5*time.Second, *tmpl.Wait.Min)
 	require.Equal(t, 60*time.Second, *tmpl.Wait.Max)
 }
+
+func TestErrMissingKey(t *testing.T) {
+	ci.Parallel(t)
+	hclBytes, err := os.ReadFile("test-fixtures/template-err-missing-key.hcl")
+	require.NoError(t, err)
+	job, err := ParseWithConfig(&ParseConfig{
+		Path:    "test-fixtures/template-err-missing-key.hcl",
+		Body:    hclBytes,
+		AllowFS: false,
+	})
+	require.NoError(t, err)
+	tmpl := job.TaskGroups[0].Tasks[0].Templates[0]
+	require.NotNil(t, tmpl)
+	require.NotNil(t, tmpl.ErrMissingKey)
+	require.True(t, *tmpl.ErrMissingKey)
+}
diff --git a/jobspec2/test-fixtures/template-err-missing-key.hcl b/jobspec2/test-fixtures/template-err-missing-key.hcl
@@ -0,0 +1,9 @@
+job "example" {
+  group "group" {
+    task "task" {
+      template {
+        error_on_missing_key = true
+      }
+    }
+  }
+}