Merge pull request #8761 from hashicorp/b-consul-op-token-check

consul/connect: make use of task kind to determine service name in consul token checks

nomad/job_endpoint.go

@@ -252,15 +252,16 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
 
 	// helper function that checks if the "operator token" supplied with the
 	// job has sufficient ACL permissions for establishing consul connect services
-	checkOperatorToken := func(task string) error {
+	checkOperatorToken := func(kind structs.TaskKind) error {
 		if j.srv.config.ConsulConfig.AllowsUnauthenticated() {
 			// if consul.allow_unauthenticated is enabled (which is the default)
 			// just let the Job through without checking anything.
 			return nil
 		}
-		proxiedTask := strings.TrimPrefix(task, structs.ConnectProxyPrefix+"-")
+
+		service := kind.Value()
 		ctx := context.Background()
-		if err := j.srv.consulACLs.CheckSIPolicy(ctx, proxiedTask, args.Job.ConsulToken); err != nil {
+		if err := j.srv.consulACLs.CheckSIPolicy(ctx, service, args.Job.ConsulToken); err != nil {
 			// not much in the way of exported error types, we could parse
 			// the content, but all errors are going to be failures anyway
 			return errors.Wrap(err, "operator token denied")
@@ -269,11 +270,9 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
 	}
 
 	// Enforce that the operator has necessary Consul ACL permissions
-	for _, tg := range args.Job.ConnectTasks() {
-		for _, task := range tg {
-			if err := checkOperatorToken(task); err != nil {
-				return err
-			}
+	for _, taskKind := range args.Job.ConnectTasks() {
+		if err := checkOperatorToken(taskKind); err != nil {
+			return err
 		}
 	}
 

nomad/structs/structs.go

@@ -4230,25 +4230,24 @@ func (j *Job) VaultPolicies() map[string]map[string]*Vault {
 	return policies
 }
 
-// Connect tasks returns the set of Consul Connect enabled tasks that will
-// require a Service Identity token, if Consul ACLs are enabled.
+// ConnectTasks returns the set of Consul Connect enabled tasks defined on the
+// job that will require a Service Identity token in the case that Consul ACLs
+// are enabled. The TaskKind.Value is the name of the Consul service.
 //
 // This method is meaningful only after the Job has passed through the job
 // submission Mutator functions.
-//
-// task group -> []task
-func (j *Job) ConnectTasks() map[string][]string {
-	m := make(map[string][]string)
+func (j *Job) ConnectTasks() []TaskKind {
+	var kinds []TaskKind
 	for _, tg := range j.TaskGroups {
 		for _, task := range tg.Tasks {
 			if task.Kind.IsConnectProxy() ||
 				task.Kind.IsConnectNative() ||
 				task.Kind.IsAnyConnectGateway() {
-				m[tg.Name] = append(m[tg.Name], task.Name)
+				kinds = append(kinds, task.Kind)
 			}
 		}
 	}
-	return m
+	return kinds
 }
 
 // ConfigEntries accumulates the Consul Configuration Entries defined in task groups

nomad/structs/structs_test.go

@@ -608,9 +608,6 @@ func TestJob_ConnectTasks(t *testing.T) {
 	t.Parallel()
 	r := require.New(t)
 
-	// todo(shoenig): this will need some updates when we support connect native
-	//  tasks, which will have a different Kind format, probably.
-
 	j0 := &Job{
 		TaskGroups: []*TaskGroup{{
 			Name: "tg1",
@@ -633,15 +630,35 @@ func TestJob_ConnectTasks(t *testing.T) {
 				Name: "connect-proxy-task2",
 				Kind: "connect-proxy:task2",
 			}},
+		}, {
+			Name: "tg3",
+			Tasks: []*Task{{
+				Name: "ingress",
+				Kind: "connect-ingress:ingress",
+			}},
+		}, {
+			Name: "tg4",
+			Tasks: []*Task{{
+				Name: "frontend",
+				Kind: "connect-native:uuid-fe",
+			}, {
+				Name: "generator",
+				Kind: "connect-native:uuid-api",
+			}},
 		}},
 	}
 
 	connectTasks := j0.ConnectTasks()
 
-	exp := map[string][]string{
-		"tg1": {"connect-proxy-task1", "connect-proxy-task3"},
-		"tg2": {"connect-proxy-task2"},
+	exp := []TaskKind{
+		NewTaskKind(ConnectProxyPrefix, "task1"),
+		NewTaskKind(ConnectProxyPrefix, "task3"),
+		NewTaskKind(ConnectProxyPrefix, "task2"),
+		NewTaskKind(ConnectIngressPrefix, "ingress"),
+		NewTaskKind(ConnectNativePrefix, "uuid-fe"),
+		NewTaskKind(ConnectNativePrefix, "uuid-api"),
 	}
+
 	r.Equal(exp, connectTasks)
 }