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

template: error on missing key #14002

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 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
5 changes: 5 additions & 0 deletions .changelog/14002.txt
Original file line number Diff line number Diff line change
@@ -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.
```
54 changes: 28 additions & 26 deletions api/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -758,34 +758,36 @@ func TestJobs_Canonicalize(t *testing.T) {
LogConfig: DefaultLogConfig(),
Templates: []*Template{
{
SourcePath: stringToPtr(""),
DestPath: stringToPtr("local/file.yml"),
EmbeddedTmpl: stringToPtr("---"),
ChangeMode: stringToPtr("restart"),
ChangeSignal: stringToPtr(""),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("0644"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
LeftDelim: stringToPtr("{{"),
RightDelim: stringToPtr("}}"),
Envvars: boolToPtr(false),
VaultGrace: timeToPtr(0),
SourcePath: stringToPtr(""),
DestPath: stringToPtr("local/file.yml"),
EmbeddedTmpl: stringToPtr("---"),
ChangeMode: stringToPtr("restart"),
ChangeSignal: stringToPtr(""),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("0644"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
LeftDelim: stringToPtr("{{"),
RightDelim: stringToPtr("}}"),
Envvars: boolToPtr(false),
VaultGrace: timeToPtr(0),
ErrMissingKey: boolToPtr(false),
},
{
SourcePath: stringToPtr(""),
DestPath: stringToPtr("local/file.env"),
EmbeddedTmpl: stringToPtr("FOO=bar\n"),
ChangeMode: stringToPtr("restart"),
ChangeSignal: stringToPtr(""),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("0644"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
LeftDelim: stringToPtr("{{"),
RightDelim: stringToPtr("}}"),
Envvars: boolToPtr(true),
VaultGrace: timeToPtr(0),
SourcePath: stringToPtr(""),
DestPath: stringToPtr("local/file.env"),
EmbeddedTmpl: stringToPtr("FOO=bar\n"),
ChangeMode: stringToPtr("restart"),
ChangeSignal: stringToPtr(""),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("0644"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
LeftDelim: stringToPtr("{{"),
RightDelim: stringToPtr("}}"),
Envvars: boolToPtr(true),
VaultGrace: timeToPtr(0),
ErrMissingKey: boolToPtr(false),
},
},
},
Expand Down
32 changes: 18 additions & 14 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,20 +792,21 @@ func (wc *WaitConfig) Copy() *WaitConfig {
}

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"`
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"`
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"`
angrycub marked this conversation as resolved.
Show resolved Hide resolved
}

func (tmpl *Template) Canonicalize() {
Expand Down Expand Up @@ -852,6 +853,9 @@ func (tmpl *Template) Canonicalize() {
if tmpl.Envvars == nil {
tmpl.Envvars = boolToPtr(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 {
Expand Down
1 change: 1 addition & 0 deletions client/allocrunner/taskrunner/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,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
Expand Down
40 changes: 40 additions & 0 deletions client/allocrunner/taskrunner/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2176,6 +2176,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) {
Expand Down
29 changes: 15 additions & 14 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1219,20 +1219,21 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
}
structsTask.Templates = append(structsTask.Templates,
&structs.Template{
SourcePath: *template.SourcePath,
DestPath: *template.DestPath,
EmbeddedTmpl: *template.EmbeddedTmpl,
ChangeMode: *template.ChangeMode,
ChangeSignal: *template.ChangeSignal,
Splay: *template.Splay,
Perms: *template.Perms,
Uid: uid,
Gid: 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,
Splay: *template.Splay,
Perms: *template.Perms,
Uid: uid,
Gid: gid,
LeftDelim: *template.LeftDelim,
RightDelim: *template.RightDelim,
Envvars: *template.Envvars,
VaultGrace: *template.VaultGrace,
Wait: ApiWaitConfigToStructsWaitConfig(template.Wait),
ErrMissingKey: *template.ErrMissingKey,
})
}
}
Expand Down
3 changes: 3 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
api "github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/pointer"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -2742,6 +2743,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Min: helper.TimeToPtr(5 * time.Second),
Max: helper.TimeToPtr(10 * time.Second),
},
ErrMissingKey: pointer.Of[bool](true),
Copy link
Contributor

Choose a reason for hiding this comment

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

This will cause the backport to fail like in the other PR. Since all the other tests use the helper package already it may be good to avoid pointer here for now to get a clean merge and save some manual work.

angrycub marked this conversation as resolved.
Show resolved Hide resolved
},
},
DispatchPayload: &api.DispatchPayloadConfig{
Expand Down Expand Up @@ -3149,6 +3151,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Min: helper.TimeToPtr(5 * time.Second),
Max: helper.TimeToPtr(10 * time.Second),
},
ErrMissingKey: true,
},
},
DispatchPayload: &structs.DispatchPayloadConfig{
Expand Down
13 changes: 8 additions & 5 deletions jobspec/parse_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,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
Expand All @@ -459,11 +461,12 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error {
}

templ := &api.Template{
ChangeMode: stringToPtr("restart"),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("0644"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
ChangeMode: stringToPtr("restart"),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("0644"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
ErrMissingKey: boolToPtr(false),
}

dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
Expand Down
40 changes: 21 additions & 19 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,27 +368,29 @@ 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"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
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"),
Uid: intToPtr(-1),
Gid: intToPtr(-1),
Envvars: boolToPtr(true),
VaultGrace: timeToPtr(33 * time.Second),
ErrMissingKey: boolToPtr(true),
},
{
SourcePath: stringToPtr("bar"),
DestPath: stringToPtr("bar"),
ChangeMode: stringToPtr(templateChangeModeRestart),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("777"),
Uid: intToPtr(1001),
Gid: intToPtr(20),
LeftDelim: stringToPtr("--"),
RightDelim: stringToPtr("__"),
SourcePath: stringToPtr("bar"),
DestPath: stringToPtr("bar"),
ChangeMode: stringToPtr(templateChangeModeRestart),
Splay: timeToPtr(5 * time.Second),
Perms: stringToPtr("777"),
Uid: intToPtr(1001),
Gid: intToPtr(20),
LeftDelim: stringToPtr("--"),
RightDelim: stringToPtr("__"),
ErrMissingKey: boolToPtr(false),
},
},
Leader: true,
Expand Down
15 changes: 8 additions & 7 deletions jobspec/test-fixtures/basic.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions jobspec2/parse_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ func normalizeTemplates(templates []*api.Template) {
if t.Splay == nil {
t.Splay = durationToPtr(5 * time.Second)
}
if t.ErrMissingKey == nil {
t.ErrMissingKey = boolToPtr(false)
}
}
}

Expand Down
22 changes: 21 additions & 1 deletion jobspec2/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ func TestParseServiceCheck(t *testing.T) {

func TestWaitConfig(t *testing.T) {
ci.Parallel(t)

hclBytes, err := os.ReadFile("test-fixtures/template-wait-config.hcl")
require.NoError(t, err)

Expand All @@ -1051,3 +1051,23 @@ 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)
}
9 changes: 9 additions & 0 deletions jobspec2/test-fixtures/template-err-missing-key.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
job "example" {
group "group" {
task "task" {
template {
error_on_missing_key = true
}
}
}
}
Loading