diff --git a/.changelog/14203.txt b/.changelog/14203.txt new file mode 100644 index 000000000..f331d84c8 --- /dev/null +++ b/.changelog/14203.txt @@ -0,0 +1,3 @@ +```release-note:bug +template: Fixed a bug where job templates would use `uid` and `gid` 0 after upgrading to Nomad 1.3.3, causing tasks to fail with the error `failed looking up user: managing file ownership is not supported on Windows`. +``` diff --git a/api/jobs_test.go b/api/jobs_test.go index 78b042bc6..f6e3b3094 100644 --- a/api/jobs_test.go +++ b/api/jobs_test.go @@ -765,8 +765,6 @@ func TestJobs_Canonicalize(t *testing.T) { ChangeSignal: pointerOf(""), Splay: pointerOf(5 * time.Second), Perms: pointerOf("0644"), - Uid: pointerOf(-1), - Gid: pointerOf(-1), LeftDelim: pointerOf("{{"), RightDelim: pointerOf("}}"), Envvars: pointerOf(false), @@ -780,8 +778,6 @@ func TestJobs_Canonicalize(t *testing.T) { ChangeSignal: pointerOf(""), Splay: pointerOf(5 * time.Second), Perms: pointerOf("0644"), - Uid: pointerOf(-1), - Gid: pointerOf(-1), LeftDelim: pointerOf("{{"), RightDelim: pointerOf("}}"), Envvars: pointerOf(true), diff --git a/api/tasks.go b/api/tasks.go index 5c8db853e..b24c6b058 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -855,12 +855,6 @@ func (tmpl *Template) Canonicalize() { if tmpl.Perms == nil { tmpl.Perms = pointerOf("0644") } - if tmpl.Uid == nil { - tmpl.Uid = pointerOf(-1) - } - if tmpl.Gid == nil { - tmpl.Gid = pointerOf(-1) - } if tmpl.LeftDelim == nil { tmpl.LeftDelim = pointerOf("{{") } diff --git a/client/allocrunner/taskrunner/template/template.go b/client/allocrunner/taskrunner/template/template.go index 9e307ff01..d3536018c 100644 --- a/client/allocrunner/taskrunner/template/template.go +++ b/client/allocrunner/taskrunner/template/template.go @@ -627,9 +627,11 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa ct.Perms = &m } // Set ownership - if tmpl.Uid >= 0 && tmpl.Gid >= 0 { - ct.Uid = &tmpl.Uid - ct.Gid = &tmpl.Gid + if tmpl.Uid != nil && *tmpl.Uid >= 0 { + ct.Uid = tmpl.Uid + } + if tmpl.Gid != nil && *tmpl.Gid >= 0 { + ct.Gid = tmpl.Gid } ct.Finalize() diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go index 467882234..b168e11dc 100644 --- a/client/allocrunner/taskrunner/template/template_test.go +++ b/client/allocrunner/taskrunner/template/template_test.go @@ -514,8 +514,8 @@ func TestTaskTemplateManager_Permissions(t *testing.T) { DestPath: file, ChangeMode: structs.TemplateChangeModeNoop, Perms: "777", - Uid: 503, - Gid: 20, + Uid: pointer.Of(503), + Gid: pointer.Of(20), } harness := newTestHarness(t, []*structs.Template{template}, false, false) @@ -541,8 +541,8 @@ func TestTaskTemplateManager_Permissions(t *testing.T) { } sys := fi.Sys() - uid := int(sys.(*syscall.Stat_t).Uid) - gid := int(sys.(*syscall.Stat_t).Gid) + uid := pointer.Of(int(sys.(*syscall.Stat_t).Uid)) + gid := pointer.Of(int(sys.(*syscall.Stat_t).Gid)) must.Eq(t, template.Uid, uid) must.Eq(t, template.Gid, gid) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index c1df09cc9..20fbca217 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1209,14 +1209,6 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, if len(apiTask.Templates) > 0 { structsTask.Templates = []*structs.Template{} for _, template := range apiTask.Templates { - uid := -1 - if template.Uid != nil { - uid = *template.Uid - } - gid := -1 - if template.Gid != nil { - gid = *template.Gid - } structsTask.Templates = append(structsTask.Templates, &structs.Template{ SourcePath: *template.SourcePath, @@ -1226,8 +1218,8 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, ChangeSignal: *template.ChangeSignal, Splay: *template.Splay, Perms: *template.Perms, - Uid: uid, - Gid: gid, + Uid: template.Uid, + Gid: template.Gid, LeftDelim: *template.LeftDelim, RightDelim: *template.RightDelim, Envvars: *template.Envvars, diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 08a63838e..0302d7cc7 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -3139,8 +3139,8 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { ChangeSignal: "SIGNAL", Splay: 1 * time.Minute, Perms: "666", - Uid: 1000, - Gid: 1000, + Uid: pointer.Of(1000), + Gid: pointer.Of(1000), LeftDelim: "abc", RightDelim: "def", Envvars: true, diff --git a/jobspec/helper.go b/jobspec/helper.go index 826cba065..99bf80f83 100644 --- a/jobspec/helper.go +++ b/jobspec/helper.go @@ -18,11 +18,6 @@ func stringToPtr(str string) *string { return &str } -// intToPtr returns the pointer to an int -func intToPtr(i int) *int { - return &i -} - // timeToPtr returns the pointer to a time.Duration. func timeToPtr(t time.Duration) *time.Duration { return &t diff --git a/jobspec/helper_test.go b/jobspec/helper_test.go new file mode 100644 index 000000000..4f52f4de5 --- /dev/null +++ b/jobspec/helper_test.go @@ -0,0 +1,9 @@ +package jobspec + +// These functions are copied from helper/funcs.go +// added here to avoid jobspec depending on any other package + +// intToPtr returns the pointer to an int +func intToPtr(i int) *int { + return &i +} diff --git a/jobspec/parse_task.go b/jobspec/parse_task.go index 19bb669cc..016de86ff 100644 --- a/jobspec/parse_task.go +++ b/jobspec/parse_task.go @@ -462,8 +462,6 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error { ChangeMode: stringToPtr("restart"), Splay: timeToPtr(5 * time.Second), Perms: stringToPtr("0644"), - Uid: intToPtr(-1), - Gid: intToPtr(-1), } dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 0d7467c16..75a8325ff 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -374,8 +374,6 @@ func TestParse(t *testing.T) { 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), }, diff --git a/jobspec2/helper_test.go b/jobspec2/helper_test.go new file mode 100644 index 000000000..57a6cd360 --- /dev/null +++ b/jobspec2/helper_test.go @@ -0,0 +1,5 @@ +package jobspec2 + +func intToPtr(v int) *int { + return &v +} diff --git a/jobspec2/parse_job.go b/jobspec2/parse_job.go index 25ec2381a..9b533874f 100644 --- a/jobspec2/parse_job.go +++ b/jobspec2/parse_job.go @@ -107,12 +107,6 @@ func normalizeTemplates(templates []*api.Template) { if t.Perms == nil { t.Perms = stringToPtr("0644") } - if t.Uid == nil { - t.Uid = intToPtr(-1) - } - if t.Gid == nil { - t.Gid = intToPtr(-1) - } if t.Splay == nil { t.Splay = durationToPtr(5 * time.Second) } @@ -127,10 +121,6 @@ func boolToPtr(v bool) *bool { return &v } -func intToPtr(v int) *int { - return &v -} - func stringToPtr(v string) *string { return &v } diff --git a/nomad/structs/diff.go b/nomad/structs/diff.go index 8c16123e1..bf827bf63 100644 --- a/nomad/structs/diff.go +++ b/nomad/structs/diff.go @@ -1671,6 +1671,24 @@ func templateDiff(old, new *Template, contextual bool) *ObjectDiff { newPrimitiveFlat = flatmap.Flatten(new, nil, true) } + // Add the pointer primitive fields. + if old != nil { + if old.Uid != nil { + oldPrimitiveFlat["Uid"] = fmt.Sprintf("%v", *old.Uid) + } + if old.Gid != nil { + oldPrimitiveFlat["Gid"] = fmt.Sprintf("%v", *old.Gid) + } + } + if new != nil { + if new.Uid != nil { + newPrimitiveFlat["Uid"] = fmt.Sprintf("%v", *new.Uid) + } + if new.Gid != nil { + newPrimitiveFlat["Gid"] = fmt.Sprintf("%v", *new.Gid) + } + } + // Diff the primitive fields. diff.Fields = fieldDiffs(oldPrimitiveFlat, newPrimitiveFlat, contextual) diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 3e96ca8b0..f141f06a5 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -7044,8 +7044,8 @@ func TestTaskDiff(t *testing.T) { ChangeSignal: "SIGHUP", Splay: 1, Perms: "0644", - Uid: 1001, - Gid: 21, + Uid: pointer.Of(1001), + Gid: pointer.Of(21), Wait: &WaitConfig{ Min: pointer.Of(5 * time.Second), Max: pointer.Of(5 * time.Second), @@ -7059,8 +7059,8 @@ func TestTaskDiff(t *testing.T) { ChangeSignal: "SIGHUP2", Splay: 2, Perms: "0666", - Uid: 1000, - Gid: 20, + Uid: pointer.Of(1000), + Gid: pointer.Of(20), Envvars: true, }, }, @@ -7075,8 +7075,8 @@ func TestTaskDiff(t *testing.T) { ChangeSignal: "SIGHUP", Splay: 1, Perms: "0644", - Uid: 1001, - Gid: 21, + Uid: pointer.Of(1001), + Gid: pointer.Of(21), Wait: &WaitConfig{ Min: pointer.Of(5 * time.Second), Max: pointer.Of(10 * time.Second), @@ -7090,8 +7090,8 @@ func TestTaskDiff(t *testing.T) { ChangeSignal: "SIGHUP3", Splay: 3, Perms: "0776", - Uid: 1002, - Gid: 22, + Uid: pointer.Of(1002), + Gid: pointer.Of(22), Wait: &WaitConfig{ Min: pointer.Of(5 * time.Second), Max: pointer.Of(10 * time.Second), diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 2f07ce897..0e14b4267 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -7647,8 +7647,8 @@ type Template struct { // Perms is the permission the file should be written out with. Perms string // User and group that should own the file. - Uid int - Gid int + Uid *int + Gid *int // LeftDelim and RightDelim are optional configurations to control what // delimiter is utilized when parsing the template. diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 1e26dd1bf..5684994f7 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -2579,6 +2579,64 @@ func TestTask_Validate_Template(t *testing.T) { } } +func TestTemplate_Copy(t *testing.T) { + ci.Parallel(t) + + t1 := &Template{ + SourcePath: "/local/file.txt", + DestPath: "/local/dest.txt", + EmbeddedTmpl: "tpl", + ChangeMode: TemplateChangeModeSignal, + ChangeSignal: "SIGHUP", + Splay: 10 * time.Second, + Perms: "777", + Uid: pointer.Of(1000), + Gid: pointer.Of(2000), + LeftDelim: "[[", + RightDelim: "]]", + Envvars: true, + VaultGrace: time.Minute, + Wait: &WaitConfig{ + Min: pointer.Of(time.Second), + Max: pointer.Of(time.Minute), + }, + } + t2 := t1.Copy() + + t1.SourcePath = "/local/file2.txt" + t1.DestPath = "/local/dest2.txt" + t1.EmbeddedTmpl = "tpl2" + t1.ChangeMode = TemplateChangeModeRestart + t1.ChangeSignal = "" + t1.Splay = 5 * time.Second + t1.Perms = "700" + t1.Uid = pointer.Of(5000) + t1.Gid = pointer.Of(6000) + t1.LeftDelim = "((" + t1.RightDelim = "))" + t1.Envvars = false + t1.VaultGrace = 2 * time.Minute + t1.Wait.Min = pointer.Of(2 * time.Second) + t1.Wait.Max = pointer.Of(2 * time.Minute) + + require.NotEqual(t, t1.SourcePath, t2.SourcePath) + require.NotEqual(t, t1.DestPath, t2.DestPath) + require.NotEqual(t, t1.EmbeddedTmpl, t2.EmbeddedTmpl) + require.NotEqual(t, t1.ChangeMode, t2.ChangeMode) + require.NotEqual(t, t1.ChangeSignal, t2.ChangeSignal) + require.NotEqual(t, t1.Splay, t2.Splay) + require.NotEqual(t, t1.Perms, t2.Perms) + require.NotEqual(t, t1.Uid, t2.Uid) + require.NotEqual(t, t1.Gid, t2.Gid) + require.NotEqual(t, t1.LeftDelim, t2.LeftDelim) + require.NotEqual(t, t1.RightDelim, t2.RightDelim) + require.NotEqual(t, t1.Envvars, t2.Envvars) + require.NotEqual(t, t1.VaultGrace, t2.VaultGrace) + require.NotEqual(t, t1.Wait.Min, t2.Wait.Min) + require.NotEqual(t, t1.Wait.Max, t2.Wait.Max) + +} + func TestTemplate_Validate(t *testing.T) { ci.Parallel(t) diff --git a/website/content/docs/job-specification/template.mdx b/website/content/docs/job-specification/template.mdx index 43c9c934f..80121f5b3 100644 --- a/website/content/docs/job-specification/template.mdx +++ b/website/content/docs/job-specification/template.mdx @@ -84,16 +84,17 @@ refer to the [Learn Go Template Syntax][gt_learn] Learn guide. - `perms` `(string: "644")` - Specifies the rendered template's permissions. File permissions are given as octal of the Unix file permissions `rwxrwxrwx`. -- `uid` `(int: -1)` - Specifies the rendered template owner's user ID. Negative - values will use the ID of the Nomad agent user. +- `uid` `(int: nil)` - Specifies the rendered template owner's user ID. If + negative or not specified (`nil`) the ID of the Nomad agent user wil be used. ~> **Caveat:** Works only on Unix-based systems. Be careful when using containerized drivers, such as `docker` or `podman`, as groups and users inside the container may have different IDs than on the host system. This feature will also **not** work with Docker Desktop. -- `gid` `(int: -1)` - Specifies the rendered template owner's group ID. - Negative values will use the ID of the Nomad agent group. +- `gid` `(int: nil)` - Specifies the rendered template owner's group ID. If + negative or not specified (`nil`) the ID of the Nomad agent group will be + used. ~> **Caveat:** Works only on Unix-based systems. Be careful when using containerized drivers, such as `docker` or `podman`, as groups and users diff --git a/website/content/docs/upgrade/upgrade-specific.mdx b/website/content/docs/upgrade/upgrade-specific.mdx index f3b712283..86b249b20 100644 --- a/website/content/docs/upgrade/upgrade-specific.mdx +++ b/website/content/docs/upgrade/upgrade-specific.mdx @@ -23,6 +23,19 @@ to version 3, and in Nomad 1.4.0 Nomad requires the use of raft protocol version 3. If [`raft_protocol`] version is explicitly set, it must now be set to `3`. For more information see the [Upgrading to Raft Protocol 3] guide. +## Nomad 1.3.3 + +Environments that don't support the use of [`uid`][template_uid] and +[`gid`][template_gid] in `template` blocks, such as Windows clients, may +experience task failures with the following message after upgrading to Nomad +1.3.3: + +``` +Template failed: error rendering "(dynamic)" => "...": failed looking up user: managing file ownership is not supported on Windows +``` + +It is recommended to avoid this version of Nomad in such environments. + ## Nomad 1.3.2, 1.2.9, 1.1.15 #### Client `max_kill_timeout` now enforced @@ -1428,6 +1441,8 @@ deleted and then Nomad 0.3.0 can be launched. [vault_grace]: /docs/job-specification/template [node drain]: https://www.nomadproject.io/docs/upgrade#5-upgrade-clients [`template.disable_file_sandbox`]: /docs/configuration/client#template-parameters +[template_gid]: /docs/job-specification/template#gid +[template_uid]: /docs/job-specification/template#uid [pki]: https://www.vaultproject.io/docs/secrets/pki [`volume create`]: /docs/commands/volume/create [`volume register`]: /docs/commands/volume/register @@ -1448,4 +1463,4 @@ deleted and then Nomad 0.3.0 can be launched. [anon_token]: https://www.consul.io/docs/security/acl/acl-tokens#special-purpose-tokens [consul_acl]: https://github.com/hashicorp/consul/issues/7414 [kill_timeout]: /docs/job-specification/task#kill_timeout -[max_kill_timeout]: /docs/configuration/client#max_kill_timeout \ No newline at end of file +[max_kill_timeout]: /docs/configuration/client#max_kill_timeout