luxolus
/
open-nomad
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

template: use pointer values for gid and uid (#14203) 

When a Nomad agent starts and loads jobs that already existed in the
cluster, the default template uid and gid was being set to 0, since this
is the zero value for int. This caused these jobs to fail in
environments where it was not possible to use 0, such as in Windows
clients.

In order to differentiate between an explicit 0 and a template where
these properties were not set we need to use a pointer.
This commit is contained in:
Luiz Aoqui 2022-08-22 16:25:49 -04:00 committed by GitHub
parent d36e0c02c9
commit dbffdca92e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 137 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/14203.txt Normal file
View File

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

4
api/jobs_test.go
View File

@ -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),

6
api/tasks.go
View File

@ -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("{{")
}

8
client/allocrunner/taskrunner/template/template.go
View File

@ -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()

8
client/allocrunner/taskrunner/template/template_test.go
View File

@ -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)

12
command/agent/job_endpoint.go
View File

@ -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,

4
command/agent/job_endpoint_test.go
View File

@ -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,

5
jobspec/helper.go
View File

@ -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

9
jobspec/helper_test.go Normal file
View File

@ -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
}

2
jobspec/parse_task.go
View File

@ -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{

2
jobspec/parse_test.go
View File

@ -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),
},

5
jobspec2/helper_test.go Normal file
View File

@ -0,0 +1,5 @@
package jobspec2
func intToPtr(v int) *int {
return &v
}

10
jobspec2/parse_job.go
View File

@ -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
}

18
nomad/structs/diff.go
View File

@ -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)

16
nomad/structs/diff_test.go
View File

@ -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),

4
nomad/structs/structs.go
View File

@ -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.

58
nomad/structs/structs_test.go
View File

@ -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)

9
website/content/docs/job-specification/template.mdx
View File

@ -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

17
website/content/docs/upgrade/upgrade-specific.mdx
View File

@ -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
[max_kill_timeout]: /docs/configuration/client#max_kill_timeout