nomad,client: apply more comment/style PR tweaks

client/allocrunner/taskrunner/envoybootstrap_hook.go

@@ -230,6 +230,9 @@ func (h *envoyBootstrapHook) execute(cmd *exec.Cmd) (string, error) {
 	return stdout.String(), nil
 }
 
+// envoyBootstrapArgs is used to accumulate CLI arguments that will be passed
+// along to the exec invocation of consul which will then generate the bootstrap
+// configuration file for envoy.
 type envoyBootstrapArgs struct {
 	sidecarFor     string
 	grpcAddr       string
@@ -238,6 +241,8 @@ type envoyBootstrapArgs struct {
 	siToken        string
 }
 
+// args returns the CLI arguments consul needs in the correct order, with the
+// -token argument present or not present depending on whether it is set.
 func (e envoyBootstrapArgs) args() []string {
 	arguments := []string{
 		"connect",

client/allocrunner/taskrunner/envoybootstrap_hook_test.go

@@ -1,3 +1,7 @@
+// +build !windows
+// todo(shoenig): Once Connect is supported on Windows, we'll need to make this
+//  set of tests work there too.
+
 package taskrunner
 
 import (
@@ -40,6 +44,9 @@ func writeTmp(t *testing.T, s string, fm os.FileMode) string {
 func TestEnvoyBootstrapHook_maybeLoadSIToken(t *testing.T) {
 	t.Parallel()
 
+	// This test fails when running as root because the test case for checking
+	// the error condition when the file is unreadable fails (root can read the
+	// file even though the permissions are set to 0200).
 	if unix.Geteuid() == 0 {
 		t.Skip("test only works as non-root")
 	}

client/allocrunner/taskrunner/sids_hook_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/nomad/client/consul"
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/testlog"
+	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/stretchr/testify/require"
 )
@@ -51,7 +52,7 @@ func TestSIDSHook_recoverToken(t *testing.T) {
 		logger: testlog.HCLogger(t),
 	})
 
-	expected := "12345678-1234-1234-1234-1234567890"
+	expected := uuid.Generate()
 	err := h.writeToken(secrets, expected)
 	r.NoError(err)
 

client/allocrunner/taskrunner/task_runner_hooks.go

@@ -116,15 +116,14 @@ func (tr *TaskRunner) initHooks() {
 			lifecycle:  tr,
 			logger:     hookLogger,
 		}))
+		// envoy bootstrap must execute after sidsHook maybe sets SI token
+		tr.runnerHooks = append(tr.runnerHooks, newEnvoyBootstrapHook(&envoyBootstrapHookConfig{
+			alloc:          alloc,
+			consulHTTPAddr: tr.clientConfig.ConsulConfig.Addr,
+			logger:         hookLogger,
+		}))
 	}
 
-	// envoy bootstrap must execute after sidsHook maybe sets SI token
-	tr.runnerHooks = append(tr.runnerHooks, newEnvoyBootstrapHook(&envoyBootstrapHookConfig{
-		alloc:          alloc,
-		consulHTTPAddr: tr.clientConfig.ConsulConfig.Addr,
-		logger:         hookLogger,
-	}))
-
 	// If there are any script checks, add the hook
 	scriptCheckHook := newScriptCheckHook(scriptCheckHookConfig{
 		alloc:  tr.Alloc(),

client/allocrunner/taskrunner/task_runner_test.go

@@ -28,6 +28,7 @@ import (
 	mockdriver "github.com/hashicorp/nomad/drivers/mock"
 	"github.com/hashicorp/nomad/drivers/rawexec"
 	"github.com/hashicorp/nomad/helper/testlog"
+	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/plugins/device"
@@ -1127,7 +1128,7 @@ func TestTaskRunner_BlockForSIDSToken(t *testing.T) {
 	defer cleanup()
 
 	// control when we get a Consul SI token
-	token := "12345678-1234-1234-1234-1234567890"
+	token := uuid.Generate()
 	waitCh := make(chan struct{})
 	deriveFn := func(*structs.Allocation, []string) (map[string]string, error) {
 		<-waitCh
@@ -1191,7 +1192,7 @@ func TestTaskRunner_DeriveSIToken_Retry(t *testing.T) {
 	defer cleanup()
 
 	// control when we get a Consul SI token
-	token := "12345678-1234-1234-1234-1234567890"
+	token := uuid.Generate()
 	deriveCount := 0
 	deriveFn := func(*structs.Allocation, []string) (map[string]string, error) {
 		if deriveCount > 0 {

nomad/job_endpoint.go

@@ -234,13 +234,10 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
 	}
 
 	// Enforce that the operator has necessary Consul ACL permissions
-	connectTasks := args.Job.ConnectTasks()
-	if len(connectTasks) > 0 {
-		for _, tg := range connectTasks {
-			for _, task := range tg {
-				if err := checkOperatorToken(task); err != nil {
-					return err
-				}
+	for _, tg := range args.Job.ConnectTasks() {
+		for _, task := range tg {
+			if err := checkOperatorToken(task); err != nil {
+				return err
 			}
 		}
 	}

nomad/leader.go

@@ -441,7 +441,7 @@ func (s *Server) revokeSITokenAccessorsOnRestore() error {
 
 	if len(toRevoke) > 0 {
 		ctx := context.Background()
-		_ = s.consulACLs.RevokeTokens(ctx, toRevoke, true)
+		s.consulACLs.RevokeTokens(ctx, toRevoke, true)
 	}
 
 	return nil