consul: make service and task identity names unique (#18634)

Ref: hashicorp/team-nomad#404
hashicorp · Oct 2, 2023 · 62a0768 · 62a0768
1 parent e7b70ad
commit 62a0768
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 28 deletions.
diff --git a/nomad/job_endpoint_hook_implicit_identities.go b/nomad/job_endpoint_hook_implicit_identities.go
@@ -35,7 +35,7 @@ func (h jobImplicitIdentitiesHook) Mutate(job *structs.Job) (*structs.Job, []err
 				h.handleConsulService(s)
 			}
 			if len(t.Templates) > 0 {
-				h.handleConsulTasks(t, tg.Consul)
+				h.handleConsulTasks(t, tg.Name)
 			}
 			h.handleVault(t)
 		}
@@ -74,18 +74,20 @@ func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service) {
 	}
 
 	// Set the expected identity name and service name.
-	serviceWID.Name = fmt.Sprintf("%s/%s", consulServiceIdentityNamePrefix, s.Name)
+	name := s.MakeUniqueIdentityName()
+	serviceWID.Name = fmt.Sprintf("%s/%s", consulServiceIdentityNamePrefix, name)
 	serviceWID.ServiceName = s.Name
 
 	s.Identity = serviceWID
 }
 
-func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task, consul *structs.Consul) {
+func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task, taskGroup string) {
 	if !h.srv.config.UseConsulIdentity() {
 		return
 	}
 
-	widName := fmt.Sprintf("%s/%s", consulTaskIdentityNamePrefix, t.Name)
+	name := t.MakeUniqueIdentityName(taskGroup)
+	widName := fmt.Sprintf("%s/%s", consulTaskIdentityNamePrefix, name)
 
 	// Use the Consul identity specified in the task if present
 	for _, wid := range t.Identities {

diff --git a/nomad/job_endpoint_hook_implicit_identities_test.go b/nomad/job_endpoint_hook_implicit_identities_test.go
@@ -97,16 +97,20 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 				TaskGroups: []*structs.TaskGroup{{
 					Services: []*structs.Service{
 						{
-							Provider: "consul",
-							Name:     "web",
+							Provider:  "consul",
+							Name:      "web",
+							TaskName:  "task",
+							PortLabel: "80",
 							Identity: &structs.WorkloadIdentity{
 								Audience: []string{"consul.io", "nomad.dev"},
 								File:     true,
 								Env:      false,
 							},
 						},
 						{
-							Name: "web",
+							Name:      "web",
+							TaskName:  "task",
+							PortLabel: "80",
 							Identity: &structs.WorkloadIdentity{
 								Audience: []string{"consul.io", "nomad.dev"},
 								File:     true,
@@ -116,8 +120,10 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 					},
 					Tasks: []*structs.Task{{
 						Services: []*structs.Service{{
-							Provider: "consul",
-							Name:     "web-task",
+							Provider:  "consul",
+							Name:      "web-task",
+							TaskName:  "task",
+							PortLabel: "80",
 							Identity: &structs.WorkloadIdentity{
 								Audience: []string{"consul.io", "nomad.dev"},
 								File:     true,
@@ -139,20 +145,24 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 				TaskGroups: []*structs.TaskGroup{{
 					Services: []*structs.Service{
 						{
-							Provider: "consul",
-							Name:     "web",
+							Provider:  "consul",
+							Name:      "web",
+							TaskName:  "task",
+							PortLabel: "80",
 							Identity: &structs.WorkloadIdentity{
-								Name:        "consul-service/web",
+								Name:        "consul-service/task-web-80",
 								Audience:    []string{"consul.io", "nomad.dev"},
 								File:        true,
 								Env:         false,
 								ServiceName: "web",
 							},
 						},
 						{
-							Name: "web",
+							Name:      "web",
+							TaskName:  "task",
+							PortLabel: "80",
 							Identity: &structs.WorkloadIdentity{
-								Name:        "consul-service/web",
+								Name:        "consul-service/task-web-80",
 								Audience:    []string{"consul.io", "nomad.dev"},
 								File:        true,
 								Env:         false,
@@ -162,10 +172,12 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 					},
 					Tasks: []*structs.Task{{
 						Services: []*structs.Service{{
-							Provider: "consul",
-							Name:     "web-task",
+							Provider:  "consul",
+							Name:      "web-task",
+							TaskName:  "task",
+							PortLabel: "80",
 							Identity: &structs.WorkloadIdentity{
-								Name:        "consul-service/web-task",
+								Name:        "consul-service/task-web-task-80",
 								Audience:    []string{"consul.io", "nomad.dev"},
 								File:        true,
 								Env:         false,
@@ -181,13 +193,17 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 			inputJob: &structs.Job{
 				TaskGroups: []*structs.TaskGroup{{
 					Services: []*structs.Service{{
-						Provider: "consul",
-						Name:     "web",
+						Provider:  "consul",
+						TaskName:  "task",
+						Name:      "web",
+						PortLabel: "80",
 					}},
 					Tasks: []*structs.Task{{
 						Services: []*structs.Service{{
-							Provider: "consul",
-							Name:     "web-task",
+							Provider:  "consul",
+							TaskName:  "task",
+							Name:      "web-task",
+							PortLabel: "80",
 						}},
 					}},
 				}},
@@ -203,20 +219,24 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 			expectedOutputJob: &structs.Job{
 				TaskGroups: []*structs.TaskGroup{{
 					Services: []*structs.Service{{
-						Provider: "consul",
-						Name:     "web",
+						Provider:  "consul",
+						PortLabel: "80",
+						Name:      "web",
+						TaskName:  "task",
 						Identity: &structs.WorkloadIdentity{
-							Name:        "consul-service/web",
+							Name:        "consul-service/task-web-80",
 							Audience:    []string{"consul.io"},
 							ServiceName: "web",
 						},
 					}},
 					Tasks: []*structs.Task{{
 						Services: []*structs.Service{{
-							Provider: "consul",
-							Name:     "web-task",
+							Provider:  "consul",
+							PortLabel: "80",
+							Name:      "web-task",
+							TaskName:  "task",
 							Identity: &structs.WorkloadIdentity{
-								Name:        "consul-service/web-task",
+								Name:        "consul-service/task-web-task-80",
 								Audience:    []string{"consul.io"},
 								ServiceName: "web-task",
 							},
@@ -229,6 +249,7 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 			name: "mutate task to inject identity for templates",
 			inputJob: &structs.Job{
 				TaskGroups: []*structs.TaskGroup{{
+					Name: "group",
 					Tasks: []*structs.Task{{
 						Name:      "web-task",
 						Templates: []*structs.Template{{}},
@@ -245,11 +266,12 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
 			},
 			expectedOutputJob: &structs.Job{
 				TaskGroups: []*structs.TaskGroup{{
+					Name: "group",
 					Tasks: []*structs.Task{{
 						Name:      "web-task",
 						Templates: []*structs.Template{{}},
 						Identities: []*structs.WorkloadIdentity{{
-							Name:     "consul/web-task",
+							Name:     "consul/group-web-task",
 							Audience: []string{"consul.io"},
 						}},
 					}},

diff --git a/nomad/structs/services.go b/nomad/structs/services.go
@@ -772,6 +772,12 @@ func (s *Service) Validate() error {
 	return mErr.ErrorOrNil()
 }
 
+// MakeUniqueIdentityName returns a service identity name consisting of: task
+// name, service name and service port label.
+func (s *Service) MakeUniqueIdentityName() string {
+	return fmt.Sprintf("%v-%v-%v", s.TaskName, s.Name, s.PortLabel)
+}
+
 func (s *Service) validateCheckPort(c *ServiceCheck) error {
 	if s.PortLabel == "" && c.PortLabel == "" && c.RequiresPort() {
 		return fmt.Errorf("Check %s invalid: check requires a port but neither check nor service %+q have a port", c.Name, s.Name)

diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
@@ -7666,6 +7666,12 @@ func (t *Task) GetIdentity(name string) *WorkloadIdentity {
 	return nil
 }
 
+// MakeUniqueIdentityName returns a task identity name consisting of: task
+// group name and task name.
+func (t *Task) MakeUniqueIdentityName(taskGroup string) string {
+	return fmt.Sprintf("%v-%v", taskGroup, t.Name)
+}
+
 func (t *Task) Copy() *Task {
 	if t == nil {
 		return nil