template: error on missing key

.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.
+```

api/jobs_test.go

@@ -758,30 +758,36 @@ 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"),
+										Uid:           pointerOf(-1),
+										Gid:           pointerOf(-1),
+										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"),
+										Uid:           pointerOf(-1),
+										Gid:           pointerOf(-1),
+										LeftDelim:     pointerOf("{{"),
+										RightDelim:    pointerOf("}}"),
+										Envvars:       pointerOf(true),
+										VaultGrace:    pointerOf(time.Duration(0)),
+										ErrMissingKey: pointerOf(false),
 									},
 								},
 							},

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"`
+	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"`
+	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,6 +891,9 @@ func (tmpl *Template) Canonicalize() {
 	if tmpl.Envvars == nil {
 		tmpl.Envvars = pointerOf(false)
 	}
+	if tmpl.ErrMissingKey == nil {
+		tmpl.ErrMissingKey = boolToPtr(false)
+	}
 
 	//COMPAT(0.12) VaultGrace is deprecated and unused as of Vault 0.5
 	if tmpl.VaultGrace == nil {

client/allocrunner/taskrunner/template/template.go

@@ -469,27 +469,27 @@ func (tm *TaskTemplateManager) onTemplateRendered(handledRenders map[string]time
 	}
 
 	// Apply splay timeout to avoid applying change_mode too frequently.
-	if splay != 0 {
-		ns := splay.Nanoseconds()
-		offset := rand.Int63n(ns)
-		t := time.Duration(offset)
-
-		select {
-		case <-time.After(t):
-		case <-tm.shutdownCh:
-			return
+		if splay != 0 {
+			ns := splay.Nanoseconds()
+			offset := rand.Int63n(ns)
+			t := time.Duration(offset)
+
+			select {
+			case <-time.After(t):
+			case <-tm.shutdownCh:
+				return
+			}
 		}
-	}
 
-	// Update handle time
-	for _, id := range handling {
-		handledRenders[id] = events[id].LastDidRender
-	}
+		// Update handle time
+		for _, id := range handling {
+			handledRenders[id] = events[id].LastDidRender
+		}
 
-	if restart {
-		tm.config.Lifecycle.Restart(context.Background(),
-			structs.NewTaskEvent(structs.TaskRestartSignal).
-				SetDisplayMessage("Template with change_mode restart re-rendered"), false)
+		if restart {
+			tm.config.Lifecycle.Restart(context.Background(),
+				structs.NewTaskEvent(structs.TaskRestartSignal).
+					SetDisplayMessage("Template with change_mode restart re-rendered"), false)
 	} else {
 		// Handle signals and scripts since the task may have multiple
 		// templates with mixed change_mode values.
@@ -499,26 +499,26 @@ func (tm *TaskTemplateManager) onTemplateRendered(handledRenders map[string]time
 }
 
 func (tm *TaskTemplateManager) handleChangeModeSignal(signals map[string]struct{}) {
-	var mErr multierror.Error
-	for signal := range signals {
-		s := tm.signals[signal]
-		event := structs.NewTaskEvent(structs.TaskSignaling).SetTaskSignal(s).SetDisplayMessage("Template re-rendered")
-		if err := tm.config.Lifecycle.Signal(event, signal); err != nil {
-			_ = multierror.Append(&mErr, err)
-		}
-	}
+			var mErr multierror.Error
+			for signal := range signals {
+				s := tm.signals[signal]
+				event := structs.NewTaskEvent(structs.TaskSignaling).SetTaskSignal(s).SetDisplayMessage("Template re-rendered")
+				if err := tm.config.Lifecycle.Signal(event, signal); err != nil {
+					_ = multierror.Append(&mErr, err)
+				}
+			}
 
-	if err := mErr.ErrorOrNil(); err != nil {
-		flat := make([]os.Signal, 0, len(signals))
-		for signal := range signals {
-			flat = append(flat, tm.signals[signal])
-		}
+			if err := mErr.ErrorOrNil(); err != nil {
+				flat := make([]os.Signal, 0, len(signals))
+				for signal := range signals {
+					flat = append(flat, tm.signals[signal])
+				}
 
-		tm.config.Lifecycle.Kill(context.Background(),
-			structs.NewTaskEvent(structs.TaskKilling).
-				SetFailsTask().
-				SetDisplayMessage(fmt.Sprintf("Template failed to send signals %v: %v", flat, err)))
-	}
+				tm.config.Lifecycle.Kill(context.Background(),
+					structs.NewTaskEvent(structs.TaskKilling).
+						SetFailsTask().
+						SetDisplayMessage(fmt.Sprintf("Template failed to send signals %v: %v", flat, err)))
+			}
 }
 
 func (tm *TaskTemplateManager) handleChangeModeScript(scripts []*structs.ChangeScript) {
@@ -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

client/allocrunner/taskrunner/template/template_test.go

@@ -2471,6 +2471,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) {

command/agent/job_endpoint.go

@@ -1212,21 +1212,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,
+					ErrMissingKey: *template.ErrMissingKey,
+					Wait:          apiWaitConfigToStructsWaitConfig(template.Wait),
 				})
 		}
 	}

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{

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: boolToPtr(false),
 		}
 
 		dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{

jobspec/parse_test.go

@@ -372,31 +372,33 @@ 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),
+										VaultGrace:    timeToPtr(33 * time.Second),
+										ErrMissingKey: boolToPtr(true),
 									},
 									{
-										SourcePath: stringToPtr("bar"),
-										DestPath:   stringToPtr("bar"),
+										SourcePath:    stringToPtr("bar"),
+										DestPath:      stringToPtr("bar"),
 										ChangeMode: stringToPtr(templateChangeModeScript),
 										ChangeScript: &api.ChangeScript{
 											Args:        []string{"-debug", "-verbose"},
 											Command:     stringToPtr("/bin/foo"),
 											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,