template: use pointer values for gid and uid

.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

@@ -837,12 +837,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