Fix WorkloadIdentity.TTL handling, jobspec2 testing, and hcl1 vs 2 pa…

…rsing (#19024)

* make the little dots consistent
* don't trim delimiter as that over matches
* test jobspec2 package
* copy api/WorkloadIdentity.TTL -> structs
* test ttl parsing
* fix hcl1 v 2 parsing mismatch
* make jobspec(1) tests match jobspec2 tests

GNUmakefile

@@ -178,13 +178,13 @@ check: ## Lint the source code
 	@$(MAKE) hclfmt
 	@if (git status -s | grep -q -e '\.hcl$$' -e '\.nomad$$' -e '\.tf$$'); then echo the following HCL files are out of sync; git status -s | grep -e '\.hcl$$' -e '\.nomad$$' -e '\.tf$$'; exit 1; fi
 
-	@echo "==> Check API package is isolated from rest"
+	@echo "==> Check API package is isolated from rest..."
 	@cd ./api && if go list --test -f '{{ join .Deps "\n" }}' . | grep github.com/hashicorp/nomad/ | grep -v -e /nomad/api/ -e nomad/api.test; then echo "  /api package depends the ^^ above internal nomad packages.  Remove such dependency"; exit 1; fi
 
-	@echo "==> Check command package does not import structs"
+	@echo "==> Check command package does not import structs..."
 	@cd ./command && if go list -f '{{ join .Imports "\n" }}' . | grep github.com/hashicorp/nomad/nomad/structs; then echo "  /command package imports the structs pkg. Remove such import"; exit 1; fi
 
-	@echo "==> Checking Go mod.."
+	@echo "==> Checking Go mod..."
 	@GO111MODULE=on $(MAKE) tidy
 	@if (git status --porcelain | grep -Eq "go\.(mod|sum)"); then \
 		echo go.mod or go.sum needs updating; \

ci/test-core.json

@@ -33,6 +33,7 @@
     "helper/...",
     "internal/...",
     "jobspec/...",
+    "jobspec2/...",
     "lib/...",
     "nomad/auth/...",
     "nomad/deploymentwatcher/...",

command/agent/job_endpoint.go

@@ -1645,6 +1645,7 @@ func apiWorkloadIdentityToStructs(in *api.WorkloadIdentity) *structs.WorkloadIde
 		Env:          in.Env,
 		File:         in.File,
 		ServiceName:  in.ServiceName,
+		TTL:          in.TTL,
 	}
 }
 

command/agent/job_endpoint_test.go

@@ -4250,7 +4250,9 @@ func TestConversion_ApiConsulConnectToStructs(t *testing.T) {
 
 func Test_apiWorkloadIdentityToStructs(t *testing.T) {
 	ci.Parallel(t)
+
 	must.Nil(t, apiWorkloadIdentityToStructs(nil))
+
 	must.Eq(t, &structs.WorkloadIdentity{
 		Name:        "consul/test",
 		Audience:    []string{"consul.io"},
@@ -4264,19 +4266,22 @@ func Test_apiWorkloadIdentityToStructs(t *testing.T) {
 		File:        false,
 		ServiceName: "web",
 	}))
+
 	must.Eq(t, &structs.WorkloadIdentity{
 		Name:         "aws",
 		Audience:     []string{"s3"},
 		Env:          true,
 		File:         true,
 		ChangeMode:   "signal",
 		ChangeSignal: "SIGHUP",
+		TTL:          2 * time.Hour,
 	}, apiWorkloadIdentityToStructs(&api.WorkloadIdentity{
 		Name:         "aws",
 		Audience:     []string{"s3"},
 		Env:          true,
 		File:         true,
 		ChangeMode:   "signal",
 		ChangeSignal: "SIGHUP",
+		TTL:          2 * time.Hour,
 	}))
 }

jobspec/parse.go

@@ -195,6 +195,7 @@ func parseConstraints(result *[]*api.Constraint, list *ast.ObjectList) error {
 			}
 
 			m["Operand"] = api.ConstraintDistinctHosts
+			m["RTarget"] = strconv.FormatBool(enabled)
 		}
 
 		if property, ok := m[api.ConstraintDistinctProperty]; ok {

jobspec/parse_task.go

@@ -494,8 +494,6 @@ func parseTemplates(result *[]*api.Template, list *ast.ObjectList) error {
 			ChangeMode:    stringToPtr("restart"),
 			Splay:         timeToPtr(5 * time.Second),
 			Perms:         stringToPtr("0644"),
-			Uid:           pointer.Of(-1),
-			Gid:           pointer.Of(-1),
 			ErrMissingKey: pointer.Of(false),
 		}
 

jobspec/parse_test.go

@@ -386,8 +386,6 @@ func TestParse(t *testing.T) {
 										Splay:         timeToPtr(10 * time.Second),
 										Perms:         stringToPtr("0644"),
 										Envvars:       boolToPtr(true),
-										Uid:           intToPtr(-1),
-										Gid:           intToPtr(-1),
 										VaultGrace:    timeToPtr(33 * time.Second),
 										ErrMissingKey: boolToPtr(true),
 									},
@@ -533,6 +531,7 @@ func TestParse(t *testing.T) {
 				Constraints: []*api.Constraint{
 					{
 						Operand: api.ConstraintDistinctHosts,
+						RTarget: "true",
 					},
 				},
 			},

jobspec2/parse_test.go

@@ -1121,7 +1121,14 @@ func TestIdentity(t *testing.T) {
 	must.NotNil(t, job.TaskGroups[0].Tasks[0].Identity)
 	must.True(t, job.TaskGroups[0].Tasks[0].Identity.Env)
 	must.True(t, job.TaskGroups[0].Tasks[0].Identity.File)
+
 	must.Len(t, 1, job.TaskGroups[0].Tasks[0].Identities)
-	must.Eq(t, "foo", job.TaskGroups[0].Tasks[0].Identities[0].Name)
-	must.Eq(t, []string{"bar"}, job.TaskGroups[0].Tasks[0].Identities[0].Audience)
+	altID := job.TaskGroups[0].Tasks[0].Identities[0]
+	must.Eq(t, "foo", altID.Name)
+	must.Eq(t, []string{"bar"}, altID.Audience)
+	must.True(t, altID.File)
+	must.False(t, altID.Env)
+	must.Eq(t, "signal", altID.ChangeMode)
+	must.Eq(t, "sighup", altID.ChangeSignal)
+	must.Eq(t, 2*time.Hour, altID.TTL)
 }

jobspec2/test-fixtures/identity-compat.nomad.hcl

@@ -29,8 +29,12 @@ job "identitycompat" {
       # ensure parsing handles both the default and alternate identities
       # properly.
       identity {
-        name = "foo"
-        aud  = ["bar"]
+        name          = "foo"
+        aud           = ["bar"]
+        file          = true
+        ttl           = "2h"
+        change_mode   = "signal"
+        change_signal = "sighup"
       }
 
       resources {

tools/missing/main.go

@@ -125,8 +125,8 @@ func isCoveredOne(p string, pkg string) bool {
 	}
 
 	if strings.HasSuffix(p, "/...") {
-		prefix := strings.TrimSuffix(p, "/...")
-		if strings.HasPrefix(pkg, prefix) {
+		prefix := strings.TrimSuffix(p, "...")
+		if strings.HasPrefix(pkg+"/", prefix) {
 			return true
 		}
 	}

tools/missing/main_test.go

@@ -11,14 +11,17 @@ import (
 
 func Test_isCoveredOne(t *testing.T) {
 	try := func(p string, exp bool) {
-		result := isCoveredOne(p, "foo/bar")
-		must.Eq(t, exp, result)
+		t.Run(p, func(t *testing.T) {
+			result := isCoveredOne(p, "foo/bar")
+			must.Eq(t, exp, result)
+		})
 	}
 	try("baz", false)
 	try("foo", false)
 	try("foo/bar/baz", false)
 	try("foo/bar", true)
 	try("foo/bar/...", true)
+	try("foo/b/...", false)
 	try("foo/...", true)
 	try("abc/...", false)
 }