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.

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

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

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

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

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)

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,

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,

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

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

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{

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

jobspec2/helper_test.go

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

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
 }

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)
 

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

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.

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)
 

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

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