config: fix identity config for Consul service (#18363)

Rename the agent configuraion for workload identity to
`WorkloadIdentityConfig` to make its use more explicit and remove the
`ServiceName` field since it is never expected to be defined in a
configuration file.

Also update the job mutation to inject a service identity following
these rules:

1. Don't inject identity if `consul.use_identity` is false.
2. Don't inject identity if `consul.service_identity` is not specified.
3. Don't inject identity if service provider is not `consul`.
4. Set name and service name if the service specifies an identity.
5. Inject `consul.service_identity` if service does not specify an
   identity.

api/services.go

@@ -243,7 +243,7 @@ type Service struct {
 	TaggedAddresses   map[string]string `hcl:"tagged_addresses,block"`
 	TaskName          string            `mapstructure:"task" hcl:"task,optional"`
 	OnUpdate          string            `mapstructure:"on_update" hcl:"on_update,optional"`
-	Identity          *WorkloadIdentity `hcl:"identity,optional"`
+	Identity          *WorkloadIdentity `hcl:"identity,block"`
 
 	// Provider defines which backend system provides the service registration,
 	// either "consul" (default) or "nomad".

command/agent/config_parse.go

@@ -57,8 +57,8 @@ func ParseConfigFile(path string) (*Config, error) {
 		ACL:   &ACLConfig{},
 		Audit: &config.AuditConfig{},
 		Consul: &config.ConsulConfig{
-			ServiceIdentity:  &config.WorkloadIdentity{},
-			TemplateIdentity: &config.WorkloadIdentity{},
+			ServiceIdentity:  &config.WorkloadIdentityConfig{},
+			TemplateIdentity: &config.WorkloadIdentityConfig{},
 		},
 		Consuls:   map[string]*config.ConsulConfig{},
 		Autopilot: &config.AutopilotConfig{},
@@ -418,7 +418,7 @@ func parseConsuls(c *Config, list *ast.ObjectList) error {
 				return err
 			}
 
-			var serviceIdentity config.WorkloadIdentity
+			var serviceIdentity config.WorkloadIdentityConfig
 			if err := mapstructure.WeakDecode(m, &serviceIdentity); err != nil {
 				return err
 			}
@@ -432,7 +432,7 @@ func parseConsuls(c *Config, list *ast.ObjectList) error {
 				return err
 			}
 
-			var templateIdentity config.WorkloadIdentity
+			var templateIdentity config.WorkloadIdentityConfig
 			if err := mapstructure.WeakDecode(m, &templateIdentity); err != nil {
 				return err
 			}

command/agent/config_parse_test.go

@@ -236,15 +236,15 @@ var basicConfig = &Config{
 		Timeout:              5 * time.Second,
 		TimeoutHCL:           "5s",
 		UseIdentity:          &trueValue,
-		ServiceIdentity: &config.WorkloadIdentity{
+		ServiceIdentity: &config.WorkloadIdentityConfig{
 			Audience: []string{"consul.io", "nomad.dev"},
-			Env:      false,
-			File:     true,
+			Env:      pointer.Of(false),
+			File:     pointer.Of(true),
 		},
-		TemplateIdentity: &config.WorkloadIdentity{
+		TemplateIdentity: &config.WorkloadIdentityConfig{
 			Audience: []string{"consul.io"},
-			Env:      true,
-			File:     false,
+			Env:      pointer.Of(true),
+			File:     pointer.Of(false),
 		},
 	},
 	Consuls: map[string]*config.ConsulConfig{
@@ -272,15 +272,15 @@ var basicConfig = &Config{
 			Timeout:              5 * time.Second,
 			TimeoutHCL:           "5s",
 			UseIdentity:          &trueValue,
-			ServiceIdentity: &config.WorkloadIdentity{
+			ServiceIdentity: &config.WorkloadIdentityConfig{
 				Audience: []string{"consul.io", "nomad.dev"},
-				Env:      false,
-				File:     true,
+				Env:      pointer.Of(false),
+				File:     pointer.Of(true),
 			},
-			TemplateIdentity: &config.WorkloadIdentity{
+			TemplateIdentity: &config.WorkloadIdentityConfig{
 				Audience: []string{"consul.io"},
-				Env:      true,
-				File:     false,
+				Env:      pointer.Of(true),
+				File:     pointer.Of(false),
 			},
 		},
 	},

nomad/config.go

@@ -463,6 +463,55 @@ func (c *Config) Copy() *Config {
 	return &nc
 }
 
+// ConsulServiceIdentity returns the workload identity to be used for accessing
+// the Consul API to register and manage Consul services.
+func (c *Config) ConsulServiceIdentity() *structs.WorkloadIdentity {
+	if c.ConsulConfig == nil {
+		return nil
+	}
+
+	return workloadIdentityFromConfig(c.ConsulConfig.ServiceIdentity)
+}
+
+// ConsulTemplateIdentity returns the workload identity to be used for
+// accessing the Consul API from templates.
+func (c *Config) ConsulTemplateIdentity() *structs.WorkloadIdentity {
+	if c.ConsulConfig == nil {
+		return nil
+	}
+
+	return workloadIdentityFromConfig(c.ConsulConfig.TemplateIdentity)
+}
+
+// UseConsulIdentity returns true when Consul workload identity is enabled.
+func (c *Config) UseConsulIdentity() bool {
+	return c.ConsulConfig != nil &&
+		c.ConsulConfig.UseIdentity != nil &&
+		*c.ConsulConfig.UseIdentity
+}
+
+// workloadIdentityFromConfig returns a structs.WorkloadIdentity to be used in
+// a job from a config.WorkloadIdentityConfig parsed from an agent config file.
+func workloadIdentityFromConfig(widConfig *config.WorkloadIdentityConfig) *structs.WorkloadIdentity {
+	if widConfig == nil {
+		return nil
+	}
+
+	wid := &structs.WorkloadIdentity{}
+
+	if len(widConfig.Audience) > 0 {
+		wid.Audience = widConfig.Audience
+	}
+	if widConfig.Env != nil {
+		wid.Env = *widConfig.Env
+	}
+	if widConfig.File != nil {
+		wid.File = *widConfig.File
+	}
+
+	return wid
+}
+
 // DefaultConfig returns the default configuration. Only used as the basis for
 // merging agent or test parameters.
 func DefaultConfig() *Config {

nomad/job_endpoint.go

@@ -73,7 +73,7 @@ func NewJobEndpoints(s *Server, ctx *RPCContext) *Job {
 			jobExposeCheckHook{},
 			jobImpliedConstraints{},
 			jobNodePoolMutatingHook{srv: s},
-			jobIdentityCreator{srv: s},
+			jobImplicitIdentitiesHook{srv: s},
 		},
 		validators: []jobValidator{
 			jobConnectHook{},

nomad/job_endpoint_hook_connect_test.go

@@ -407,7 +407,7 @@ func TestJobEndpointConnect_groupConnectHook_MeshGateway(t *testing.T) {
 func TestJobEndpointConnect_ConnectInterpolation(t *testing.T) {
 	ci.Parallel(t)
 
-	server := &Server{logger: testlog.HCLogger(t)}
+	server := &Server{logger: testlog.HCLogger(t), config: DefaultConfig()}
 	jobEndpoint := NewJobEndpoints(server, nil)
 
 	j := mock.ConnectJob()

nomad/job_endpoint_hook_implicit_identities.go

@@ -0,0 +1,76 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package nomad
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/nomad/nomad/structs"
+)
+
+var (
+	consulServiceIdentityNamePrefix = "consul-service"
+)
+
+// jobImplicitIdentitiesHook adds implicit `identity` blocks for external
+// services, like Consul and Vault.
+type jobImplicitIdentitiesHook struct {
+	srv *Server
+}
+
+func (jobImplicitIdentitiesHook) Name() string {
+	return "implicit-identities"
+}
+
+func (h jobImplicitIdentitiesHook) Mutate(job *structs.Job) (*structs.Job, []error, error) {
+	for _, tg := range job.TaskGroups {
+		for _, s := range tg.Services {
+			h.handleConsulService(s)
+		}
+
+		for _, t := range tg.Tasks {
+			for _, s := range t.Services {
+				h.handleConsulService(s)
+			}
+		}
+	}
+
+	return job, nil, nil
+}
+
+// handleConsulService injects a workload identity to the service if:
+//  1. The service uses the Consul provider.
+//  2. The server is configured with `consul.use_identity = true` and a
+//     `consul.service_identity` is provided.
+//
+// If the service already has an identity it sets the identity name and service
+// name values.
+func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service) {
+	if !h.srv.config.UseConsulIdentity() {
+		return
+	}
+
+	if s.Provider != "" && s.Provider != "consul" {
+		return
+	}
+
+	// Use the identity specified in the service.
+	serviceWID := s.Identity
+	if serviceWID == nil {
+		// If the service doesn't specify an identity, fallback to the service
+		// identity defined in the server configuration.
+		serviceWID = h.srv.config.ConsulServiceIdentity()
+		if serviceWID == nil {
+			// If no identity is found, skip injecting the implicit identity
+			// and fallback to the legacy flow.
+			return
+		}
+	}
+
+	// Set the expected identity name and service name.
+	serviceWID.Name = fmt.Sprintf("%s/%s", consulServiceIdentityNamePrefix, s.Name)
+	serviceWID.ServiceName = s.Name
+
+	s.Identity = serviceWID
+}

nomad/job_endpoint_hook_implicit_identities_test.go

@@ -0,0 +1,240 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package nomad
+
+import (
+	"testing"
+
+	"github.com/hashicorp/nomad/ci"
+	"github.com/hashicorp/nomad/helper/pointer"
+	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/hashicorp/nomad/nomad/structs/config"
+	"github.com/shoenig/test/must"
+)
+
+func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) {
+	ci.Parallel(t)
+
+	testCases := []struct {
+		name              string
+		inputJob          *structs.Job
+		inputConfig       *Config
+		expectedOutputJob *structs.Job
+	}{
+		{
+			name: "no mutation when identity is disabled",
+			inputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "consul",
+					}},
+				}},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(false),
+				},
+			},
+			expectedOutputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "consul",
+					}},
+				}},
+			},
+		},
+		{
+			name: "no mutation when identity is enabled but no service identity is configured",
+			inputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "consul",
+					}},
+				}},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+				},
+			},
+			expectedOutputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "consul",
+					}},
+				}},
+			},
+		},
+		{
+			name: "no mutation when nomad service",
+			inputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "nomad",
+					}},
+				}},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+					ServiceIdentity: &config.WorkloadIdentityConfig{
+						Audience: []string{"consul.io"},
+					},
+				},
+			},
+			expectedOutputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "nomad",
+					}},
+				}},
+			},
+		},
+		{
+			name: "mutate identity name and service name when custom identity is provided",
+			inputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{
+						{
+							Provider: "consul",
+							Name:     "web",
+							Identity: &structs.WorkloadIdentity{
+								Audience: []string{"consul.io", "nomad.dev"},
+								File:     true,
+								Env:      false,
+							},
+						},
+						{
+							Name: "web",
+							Identity: &structs.WorkloadIdentity{
+								Audience: []string{"consul.io", "nomad.dev"},
+								File:     true,
+								Env:      false,
+							},
+						},
+					},
+					Tasks: []*structs.Task{{
+						Services: []*structs.Service{{
+							Provider: "consul",
+							Name:     "web-task",
+							Identity: &structs.WorkloadIdentity{
+								Audience: []string{"consul.io", "nomad.dev"},
+								File:     true,
+								Env:      false,
+							},
+						}},
+					}},
+				}},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+					ServiceIdentity: &config.WorkloadIdentityConfig{
+						Audience: []string{"consul.io"},
+					},
+				},
+			},
+			expectedOutputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{
+						{
+							Provider: "consul",
+							Name:     "web",
+							Identity: &structs.WorkloadIdentity{
+								Name:        "consul-service/web",
+								Audience:    []string{"consul.io", "nomad.dev"},
+								File:        true,
+								Env:         false,
+								ServiceName: "web",
+							},
+						},
+						{
+							Name: "web",
+							Identity: &structs.WorkloadIdentity{
+								Name:        "consul-service/web",
+								Audience:    []string{"consul.io", "nomad.dev"},
+								File:        true,
+								Env:         false,
+								ServiceName: "web",
+							},
+						},
+					},
+					Tasks: []*structs.Task{{
+						Services: []*structs.Service{{
+							Provider: "consul",
+							Name:     "web-task",
+							Identity: &structs.WorkloadIdentity{
+								Name:        "consul-service/web-task",
+								Audience:    []string{"consul.io", "nomad.dev"},
+								File:        true,
+								Env:         false,
+								ServiceName: "web-task",
+							},
+						}},
+					}},
+				}},
+			},
+		},
+		{
+			name: "mutate service to inject identity",
+			inputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "consul",
+						Name:     "web",
+					}},
+					Tasks: []*structs.Task{{
+						Services: []*structs.Service{{
+							Provider: "consul",
+							Name:     "web-task",
+						}},
+					}},
+				}},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+					ServiceIdentity: &config.WorkloadIdentityConfig{
+						Audience: []string{"consul.io"},
+					},
+				},
+			},
+			expectedOutputJob: &structs.Job{
+				TaskGroups: []*structs.TaskGroup{{
+					Services: []*structs.Service{{
+						Provider: "consul",
+						Name:     "web",
+						Identity: &structs.WorkloadIdentity{
+							Name:        "consul-service/web",
+							Audience:    []string{"consul.io"},
+							ServiceName: "web",
+						},
+					}},
+					Tasks: []*structs.Task{{
+						Services: []*structs.Service{{
+							Provider: "consul",
+							Name:     "web-task",
+							Identity: &structs.WorkloadIdentity{
+								Name:        "consul-service/web-task",
+								Audience:    []string{"consul.io"},
+								ServiceName: "web-task",
+							},
+						}},
+					}},
+				}},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			impl := jobImplicitIdentitiesHook{srv: &Server{
+				config: tc.inputConfig,
+			}}
+			actualJob, actualWarnings, actualError := impl.Mutate(tc.inputJob)
+			must.Eq(t, tc.expectedOutputJob, actualJob)
+			must.NoError(t, actualError)
+			must.Nil(t, actualWarnings)
+		})
+	}
+}

nomad/job_endpoint_hooks.go

@@ -307,8 +307,24 @@ func (v *jobValidate) Validate(job *structs.Job) (warnings []error, err error) {
 
 	for _, tg := range job.TaskGroups {
 		for _, s := range tg.Services {
-			if s.Identity != nil && s.Identity.Name == "" {
-				multierror.Append(validationErrors, fmt.Errorf("service identity name cannot be empty"))
+			serviceWarn, serviceErr := v.validateServiceIdentity(s)
+			if serviceErr != nil {
+				multierror.Append(validationErrors, serviceErr)
+			}
+			if len(serviceWarn) > 0 {
+				warnings = append(warnings, serviceWarn...)
+			}
+		}
+
+		for _, t := range tg.Tasks {
+			for _, s := range t.Services {
+				serviceWarn, serviceErr := v.validateServiceIdentity(s)
+				if serviceErr != nil {
+					multierror.Append(validationErrors, serviceErr)
+				}
+				if len(serviceWarn) > 0 {
+					warnings = append(warnings, serviceWarn...)
+				}
 			}
 		}
 	}
@@ -316,6 +332,22 @@ func (v *jobValidate) Validate(job *structs.Job) (warnings []error, err error) {
 	return warnings, validationErrors.ErrorOrNil()
 }
 
+func (v *jobValidate) validateServiceIdentity(s *structs.Service) (warnings []error, err error) {
+	if s.Identity != nil {
+		if !v.srv.config.UseConsulIdentity() {
+			return nil, fmt.Errorf("service %s defines an identity but server configuration for consul.use_identity is not true", s.Name)
+		}
+
+		if s.Identity.Name == "" {
+			return nil, fmt.Errorf("identity for service %s has an empty name", s.Name)
+		}
+	} else if v.srv.config.UseConsulIdentity() && v.srv.config.ConsulServiceIdentity() == nil {
+		return nil, fmt.Errorf("service %s does not have an identity and no default service identity is provided", s.Name)
+	}
+
+	return nil, nil
+}
+
 type memoryOversubscriptionValidate struct {
 	srv *Server
 }
@@ -377,42 +409,3 @@ func (j *Job) submissionController(args *structs.JobRegisterRequest) error {
 	}
 	return nil
 }
-
-// jobIdentityCreator adds implicit `identity` blocks for external services,
-// like Consul and Vault.
-type jobIdentityCreator struct {
-	srv *Server
-}
-
-func (jobIdentityCreator) Name() string {
-	return "identityBlockCreator"
-}
-
-func (jobIdentityCreator) Mutate(job *structs.Job) (*structs.Job, []error, error) {
-	for _, tg := range job.TaskGroups {
-		for _, s := range tg.Services {
-			if s.Provider != "consul" {
-				continue
-			}
-
-			identityName := fmt.Sprintf("consul-service/%s", s.Name)
-
-			// if there's an identity block present, overwrite its name and ServiceName, but
-			// keep the rest. Users can set custom aud, file and env properties to account
-			// for the specifics of their environments, but Name and ServiceName must always
-			// conform to a convention.
-			if s.Identity != nil {
-				s.Identity.Name = identityName
-				s.Identity.ServiceName = s.Name
-			} else {
-				s.Identity = &structs.WorkloadIdentity{
-					Name:        identityName,
-					Audience:    []string{"consul.io"},
-					ServiceName: s.Name,
-				}
-			}
-		}
-	}
-
-	return job, nil, nil
-}

nomad/job_endpoint_hooks_test.go

@@ -7,11 +7,127 @@ import (
 	"testing"
 
 	"github.com/hashicorp/nomad/ci"
+	"github.com/hashicorp/nomad/helper/pointer"
+	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
 
+func Test_jobValidate_Validate_consul_service(t *testing.T) {
+	ci.Parallel(t)
+
+	testCases := []struct {
+		name         string
+		inputService *structs.Service
+		inputConfig  *Config
+		expectedWarn []error
+		expectedErr  string
+	}{
+		{
+			name: "no error when consul identity not enabled and services does not have an identity",
+			inputService: &structs.Service{
+				Provider: "consul",
+				Name:     "web",
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(false),
+				},
+			},
+		},
+		{
+			name: "no error when consul identity is enabled and default service identity is provided",
+			inputService: &structs.Service{
+				Provider: "consul",
+				Name:     "web",
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+					ServiceIdentity: &config.WorkloadIdentityConfig{
+						Audience: []string{"consul.io"},
+					},
+				},
+			},
+		},
+		{
+			name: "no error when consul identity is enabled and service has a proper identity",
+			inputService: &structs.Service{
+				Provider: "consul",
+				Name:     "web",
+				Identity: &structs.WorkloadIdentity{
+					Name:        "consul-service/web",
+					Audience:    []string{"consul.io", "nomad.dev"},
+					File:        true,
+					Env:         false,
+					ServiceName: "web",
+				},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+				},
+			},
+		},
+		{
+			name: "error when service defines identity but consul identity is disabled",
+			inputService: &structs.Service{
+				Provider: "consul",
+				Name:     "web",
+				Identity: &structs.WorkloadIdentity{
+					Audience: []string{"consul.io", "nomad.dev"},
+					File:     true,
+					Env:      false,
+				},
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(false),
+				},
+			},
+			expectedErr: "server configuration for consul.use_identity is not true",
+		},
+		{
+			name: "error when service does not define identity and consul identity is enabled but no default is provided",
+			inputService: &structs.Service{
+				Provider: "consul",
+				Name:     "web",
+			},
+			inputConfig: &Config{
+				ConsulConfig: &config.ConsulConfig{
+					UseIdentity: pointer.Of(true),
+				},
+			},
+			expectedErr: "no default service identity is provided",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.inputConfig.JobMaxPriority = 100
+			impl := jobValidate{srv: &Server{
+				config: tc.inputConfig,
+			}}
+
+			job := mock.Job()
+			job.TaskGroups[0].Services = []*structs.Service{tc.inputService}
+			job.TaskGroups[0].Tasks[0].Services = []*structs.Service{tc.inputService}
+
+			warn, err := impl.Validate(job)
+			must.Eq(t, tc.expectedWarn, warn)
+
+			if len(tc.expectedErr) == 0 {
+				must.NoError(t, err)
+			} else {
+				must.Error(t, err)
+				must.ErrorContains(t, err, tc.expectedErr)
+			}
+		})
+	}
+}
+
 func Test_jobImpliedConstraints_Mutate(t *testing.T) {
 	ci.Parallel(t)
 
@@ -806,96 +922,3 @@ func TestJob_submissionController(t *testing.T) {
 		must.Nil(t, args.Submission)
 	})
 }
-
-func Test_jobIdentityCreator_Mutate(t *testing.T) {
-	ci.Parallel(t)
-
-	testCases := []struct {
-		name              string
-		inputJob          *structs.Job
-		expectedOutputJob *structs.Job
-	}{
-		{
-			name: "no mutation",
-			inputJob: &structs.Job{
-				TaskGroups: []*structs.TaskGroup{{
-					Services: []*structs.Service{{
-						Provider: "nomad",
-					}},
-				}},
-			},
-			expectedOutputJob: &structs.Job{
-				TaskGroups: []*structs.TaskGroup{{
-					Services: []*structs.Service{{
-						Provider: "nomad",
-					}},
-				}},
-			},
-		},
-		{
-			name: "custom set identity, merge",
-			inputJob: &structs.Job{
-				TaskGroups: []*structs.TaskGroup{{
-					Services: []*structs.Service{{
-						Provider: "consul",
-						Name:     "web",
-						Identity: &structs.WorkloadIdentity{
-							Name:     "test",
-							Audience: []string{"consul.io", "nomad.dev"},
-							File:     true,
-							Env:      false,
-						},
-					}},
-				}},
-			},
-			expectedOutputJob: &structs.Job{
-				TaskGroups: []*structs.TaskGroup{{
-					Services: []*structs.Service{{
-						Provider: "consul",
-						Name:     "web",
-						Identity: &structs.WorkloadIdentity{
-							Name:        "consul-service/web",
-							Audience:    []string{"consul.io", "nomad.dev"},
-							ServiceName: "web",
-							File:        true,
-							Env:         false,
-						},
-					}},
-				}},
-			},
-		},
-		{
-			name: "mutate",
-			inputJob: &structs.Job{
-				TaskGroups: []*structs.TaskGroup{{
-					Services: []*structs.Service{{
-						Provider: "consul",
-						Name:     "web",
-					}},
-				}},
-			},
-			expectedOutputJob: &structs.Job{
-				TaskGroups: []*structs.TaskGroup{{
-					Services: []*structs.Service{{
-						Provider: "consul",
-						Name:     "web",
-						Identity: &structs.WorkloadIdentity{
-							Name:        "consul-service/web",
-							Audience:    []string{"consul.io"},
-							ServiceName: "web",
-						},
-					}},
-				}},
-			},
-		},
-	}
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			impl := jobIdentityCreator{}
-			actualJob, actualWarnings, actualError := impl.Mutate(tc.inputJob)
-			must.Eq(t, tc.expectedOutputJob, actualJob)
-			must.NoError(t, actualError)
-			must.Nil(t, actualWarnings)
-		})
-	}
-}

nomad/structs/config/consul.go

@@ -154,7 +154,7 @@ type ConsulConfig struct {
 	// "consul-service/${service_name}-${service_port}".
 	//
 	// ServiceIdentity is set on the server.
-	ServiceIdentity *WorkloadIdentity `mapstructure:"service_identity"`
+	ServiceIdentity *WorkloadIdentityConfig `mapstructure:"service_identity"`
 
 	// TemplateIdentity is intended to reduce overhead for jobspec authors and make
 	// for graceful upgrades without forcing rewrite of all jobspecs. If set, when a
@@ -165,7 +165,7 @@ type ConsulConfig struct {
 	// The name field of the identity is always set to "consul".
 	//
 	// TemplateIdentity is set on the server.
-	TemplateIdentity *WorkloadIdentity `mapstructure:"template_identity"`
+	TemplateIdentity *WorkloadIdentityConfig `mapstructure:"template_identity"`
 
 	// ExtraKeysHCL is used by hcl to surface unexpected keys
 	ExtraKeysHCL []string `mapstructure:",unusedKeys" json:"-"`
@@ -296,12 +296,21 @@ func (c *ConsulConfig) Merge(b *ConsulConfig) *ConsulConfig {
 	if b.UseIdentity != nil {
 		result.UseIdentity = pointer.Of(*b.UseIdentity)
 	}
-	if b.ServiceIdentity != nil {
-		result.ServiceIdentity = pointer.Of(*b.ServiceIdentity)
+
+	if result.ServiceIdentity == nil && b.ServiceIdentity != nil {
+		sID := *b.ServiceIdentity
+		result.ServiceIdentity = &sID
+	} else if b.ServiceIdentity != nil {
+		result.ServiceIdentity = result.ServiceIdentity.Merge(b.ServiceIdentity)
 	}
-	if b.TemplateIdentity != nil {
-		result.TemplateIdentity = pointer.Of(*b.TemplateIdentity)
+
+	if result.TemplateIdentity == nil && b.TemplateIdentity != nil {
+		tID := *b.TemplateIdentity
+		result.TemplateIdentity = &tID
+	} else if b.TemplateIdentity != nil {
+		result.TemplateIdentity = result.TemplateIdentity.Merge(b.TemplateIdentity)
 	}
+
 	return result
 }
 
@@ -407,68 +416,3 @@ func (c *ConsulConfig) Copy() *ConsulConfig {
 		ExtraKeysHCL:         slices.Clone(c.ExtraKeysHCL),
 	}
 }
-
-// WorkloadIdentity is the jobspec block which determines if and how a workload
-// identity is exposed to tasks similar to the Vault block.
-//
-// This is a copy of WorkloadIdentity from nomad/structs package in order to
-// avoid import cycles.
-type WorkloadIdentity struct {
-	Name string `mapstructure:"name"`
-
-	// Audience is the valid recipients for this identity (the "aud" JWT claim)
-	// and defaults to the identity's name.
-	Audience []string `mapstructure:"aud"`
-
-	// Env injects the Workload Identity into the Task's environment if
-	// set.
-	Env bool `mapstructure:"env"`
-
-	// File writes the Workload Identity into the Task's secrets directory
-	// if set.
-	File bool `mapstructure:"file"`
-
-	// ServiceName is used to bind the identity to a correct Consul service.
-	ServiceName string `mapstructure:"-" json:"-"`
-}
-
-func (wi *WorkloadIdentity) Copy() *WorkloadIdentity {
-	if wi == nil {
-		return nil
-	}
-	return &WorkloadIdentity{
-		Name:        wi.Name,
-		Audience:    slices.Clone(wi.Audience),
-		Env:         wi.Env,
-		File:        wi.File,
-		ServiceName: wi.ServiceName,
-	}
-}
-
-func (wi *WorkloadIdentity) Equal(other *WorkloadIdentity) bool {
-	if wi == nil || other == nil {
-		return wi == other
-	}
-
-	if wi.Name != other.Name {
-		return false
-	}
-
-	if !slices.Equal(wi.Audience, other.Audience) {
-		return false
-	}
-
-	if wi.Env != other.Env {
-		return false
-	}
-
-	if wi.File != other.File {
-		return false
-	}
-
-	if wi.ServiceName != other.ServiceName {
-		return false
-	}
-
-	return true
-}

nomad/structs/config/consul_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/nomad/ci"
+	"github.com/hashicorp/nomad/helper/pointer"
 )
 
 func TestMain(m *testing.M) {
@@ -93,12 +94,11 @@ func TestConsulConfig_Merge(t *testing.T) {
 		ServerAutoJoin:       &yes,
 		ClientAutoJoin:       &yes,
 		UseIdentity:          &yes,
-		ServiceIdentity: &WorkloadIdentity{
-			Name:        "test",
-			Audience:    []string{"consul.io", "nomad.dev"},
-			Env:         false,
-			File:        true,
-			ServiceName: "test",
+		ServiceIdentity: &WorkloadIdentityConfig{
+			Name:     "test",
+			Audience: []string{"consul.io", "nomad.dev"},
+			Env:      pointer.Of(false),
+			File:     pointer.Of(true),
 		},
 		ExtraKeysHCL: []string{"b", "2"},
 	}
@@ -129,12 +129,11 @@ func TestConsulConfig_Merge(t *testing.T) {
 		ServerAutoJoin:       &yes,
 		ClientAutoJoin:       &yes,
 		UseIdentity:          &yes,
-		ServiceIdentity: &WorkloadIdentity{
-			Name:        "test",
-			Audience:    []string{"consul.io", "nomad.dev"},
-			Env:         false,
-			File:        true,
-			ServiceName: "test",
+		ServiceIdentity: &WorkloadIdentityConfig{
+			Name:     "test",
+			Audience: []string{"consul.io", "nomad.dev"},
+			Env:      pointer.Of(false),
+			File:     pointer.Of(true),
 		},
 		ExtraKeysHCL: []string{"a", "1"}, // not merged
 	}

nomad/structs/config/workload_id.go

@@ -0,0 +1,99 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package config
+
+import (
+	"slices"
+
+	"github.com/hashicorp/go-set"
+	"github.com/hashicorp/nomad/helper/pointer"
+)
+
+// WorkloadIdentityConfig is the agent configuraion block used to define
+// default workload identities.
+//
+// This based on the WorkloadIdentity struct from nomad/structs/workload_id.go
+// and may need to be kept in sync.
+type WorkloadIdentityConfig struct {
+	// Name is used to identity the workload identity. It is not expected to be
+	// set by users, but may have a default value.
+	Name string `mapstructure:"-" json:"-"`
+
+	// Audience is the valid recipients for this identity (the "aud" JWT claim)
+	// and defaults to the identity's name.
+	Audience []string `mapstructure:"aud"`
+
+	// Env injects the Workload Identity into the Task's environment if
+	// set.
+	Env *bool `mapstructure:"env"`
+
+	// File writes the Workload Identity into the Task's secrets directory
+	// if set.
+	File *bool `mapstructure:"file"`
+}
+
+func (wi *WorkloadIdentityConfig) Copy() *WorkloadIdentityConfig {
+	if wi == nil {
+		return nil
+	}
+	nwi := new(WorkloadIdentityConfig)
+	*nwi = *wi
+	nwi.Audience = slices.Clone(wi.Audience)
+
+	if wi.Env != nil {
+		nwi.Env = pointer.Of(*wi.Env)
+	}
+	if wi.File != nil {
+		nwi.File = pointer.Of(*wi.File)
+	}
+
+	return nwi
+}
+
+func (wi *WorkloadIdentityConfig) Equal(other *WorkloadIdentityConfig) bool {
+	if wi == nil || other == nil {
+		return wi == other
+	}
+
+	if wi.Name != other.Name {
+		return false
+	}
+	if !slices.Equal(wi.Audience, other.Audience) {
+		return false
+	}
+	if !pointer.Eq(wi.Env, other.Env) {
+		return false
+	}
+	if !pointer.Eq(wi.File, other.File) {
+		return false
+	}
+
+	return true
+}
+
+func (wi *WorkloadIdentityConfig) Merge(other *WorkloadIdentityConfig) *WorkloadIdentityConfig {
+	result := wi.Copy()
+
+	if other.Name != "" {
+		result.Name = other.Name
+	}
+
+	if len(result.Audience) == 0 {
+		result.Audience = slices.Clone(other.Audience)
+	} else if len(other.Audience) > 0 {
+		// Append incoming audiences avoiding duplicates.
+		audSet := set.From(result.Audience)
+		for _, aud := range other.Audience {
+			if !audSet.Contains(aud) {
+				audSet.Insert(aud)
+				result.Audience = append(result.Audience, aud)
+			}
+		}
+	}
+
+	result.Env = pointer.Merge(result.Env, other.Env)
+	result.File = pointer.Merge(result.File, other.File)
+
+	return result
+}

nomad/structs/config/workload_id_test.go

@@ -0,0 +1,242 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package config
+
+import (
+	"testing"
+
+	"github.com/hashicorp/nomad/ci"
+	"github.com/hashicorp/nomad/helper/pointer"
+	"github.com/shoenig/test/must"
+)
+
+func TestWorkloadIdentityConfig_Copy(t *testing.T) {
+	ci.Parallel(t)
+
+	original := &WorkloadIdentityConfig{
+		Name:     "test",
+		Audience: []string{"aud"},
+		Env:      pointer.Of(true),
+		File:     pointer.Of(false),
+	}
+
+	// Verify Copy() returns the same values but different pointer.
+	clone := original.Copy()
+	must.Eq(t, original, clone)
+	must.NotEqOp(t, original, clone)
+
+	// Verify returned struct does not mutate original.
+	clone.Name = "clone"
+	clone.Audience = []string{"aud", "clone"}
+	clone.Env = pointer.Of(false)
+	clone.File = pointer.Of(true)
+
+	must.NotEq(t, original, clone)
+	must.NotEqOp(t, original, clone)
+}
+
+func TestWorkloadIdentityConfig_Equal(t *testing.T) {
+	ci.Parallel(t)
+
+	testCases := []struct {
+		name     string
+		a        *WorkloadIdentityConfig
+		b        *WorkloadIdentityConfig
+		expectEq bool
+	}{
+		{
+			name: "equal",
+			a: &WorkloadIdentityConfig{
+				Name:     "test",
+				Audience: []string{"aud"},
+				Env:      pointer.Of(true),
+				File:     pointer.Of(false),
+			},
+			b: &WorkloadIdentityConfig{
+				Name:     "test",
+				Audience: []string{"aud"},
+				Env:      pointer.Of(true),
+				File:     pointer.Of(false),
+			},
+			expectEq: true,
+		},
+		{
+			name: "different name",
+			a: &WorkloadIdentityConfig{
+				Name: "a",
+			},
+			b: &WorkloadIdentityConfig{
+				Name: "b",
+			},
+			expectEq: false,
+		},
+		{
+			name: "different audience",
+			a: &WorkloadIdentityConfig{
+				Audience: []string{"a"},
+			},
+			b: &WorkloadIdentityConfig{
+				Audience: []string{"b"},
+			},
+			expectEq: false,
+		},
+		{
+			name: "different env",
+			a: &WorkloadIdentityConfig{
+				Env: pointer.Of(true),
+			},
+			b: &WorkloadIdentityConfig{
+				Env: pointer.Of(false),
+			},
+			expectEq: false,
+		},
+		{
+			name: "different env nil",
+			a: &WorkloadIdentityConfig{
+				Env: pointer.Of(true),
+			},
+			b: &WorkloadIdentityConfig{
+				Env: nil,
+			},
+			expectEq: false,
+		},
+		{
+			name: "different file",
+			a: &WorkloadIdentityConfig{
+				File: pointer.Of(true),
+			},
+			b: &WorkloadIdentityConfig{
+				File: pointer.Of(false),
+			},
+			expectEq: false,
+		},
+		{
+			name: "different file nil",
+			a: &WorkloadIdentityConfig{
+				File: pointer.Of(true),
+			},
+			b: &WorkloadIdentityConfig{
+				File: nil,
+			},
+			expectEq: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.expectEq {
+				must.True(t, tc.a.Equal(tc.b))
+			} else {
+				must.False(t, tc.a.Equal(tc.b))
+			}
+		})
+	}
+}
+
+func TestWorkloadIdentityConfig_Merge(t *testing.T) {
+	ci.Parallel(t)
+
+	testCases := []struct {
+		name     string
+		other    *WorkloadIdentityConfig
+		expected *WorkloadIdentityConfig
+	}{
+		{
+			name: "merge name",
+			other: &WorkloadIdentityConfig{
+				Name: "other",
+			},
+			expected: &WorkloadIdentityConfig{
+				Name:     "other",
+				Audience: []string{"aud"},
+				Env:      pointer.Of(true),
+				File:     pointer.Of(false),
+			},
+		},
+		{
+			name: "merge audience",
+			other: &WorkloadIdentityConfig{
+				Audience: []string{"aud", "aud", "other"},
+			},
+			expected: &WorkloadIdentityConfig{
+				Name:     "test",
+				Audience: []string{"aud", "other"},
+				Env:      pointer.Of(true),
+				File:     pointer.Of(false),
+			},
+		},
+		{
+			name: "merge env",
+			other: &WorkloadIdentityConfig{
+				Env: pointer.Of(false),
+			},
+			expected: &WorkloadIdentityConfig{
+				Name:     "test",
+				Audience: []string{"aud"},
+				Env:      pointer.Of(false),
+				File:     pointer.Of(false),
+			},
+		},
+		{
+			name: "merge file",
+			other: &WorkloadIdentityConfig{
+				File: pointer.Of(true),
+			},
+			expected: &WorkloadIdentityConfig{
+				Name:     "test",
+				Audience: []string{"aud"},
+				Env:      pointer.Of(true),
+				File:     pointer.Of(true),
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			original := &WorkloadIdentityConfig{
+				Name:     "test",
+				Audience: []string{"aud"},
+				Env:      pointer.Of(true),
+				File:     pointer.Of(false),
+			}
+			got := original.Merge(tc.other)
+			must.Eq(t, tc.expected, got)
+		})
+	}
+}
+
+func TestWorkloadIdentityConfig_Merge_multiple(t *testing.T) {
+	widConfig1 := &WorkloadIdentityConfig{
+		Name:     "wid1",
+		Audience: []string{"aud1"},
+		Env:      pointer.Of(true),
+	}
+
+	widConfig2 := &WorkloadIdentityConfig{
+		Name:     "wid2",
+		Audience: []string{"aud2"},
+		Env:      pointer.Of(false),
+	}
+
+	got12 := widConfig1.Merge(widConfig2)
+	must.Eq(t, &WorkloadIdentityConfig{
+		Name:     "wid2",
+		Audience: []string{"aud1", "aud2"},
+		Env:      pointer.Of(false),
+	}, got12)
+
+	widConfig3 := &WorkloadIdentityConfig{
+		Name:     "wid3",
+		Audience: []string{"aud1", "aud2", "aud3"},
+		File:     pointer.Of(false),
+	}
+
+	got123 := got12.Merge(widConfig3)
+	must.Eq(t, &WorkloadIdentityConfig{
+		Name:     "wid3",
+		Audience: []string{"aud1", "aud2", "aud3"},
+		Env:      pointer.Of(false),
+		File:     pointer.Of(false),
+	}, got123)
+}

nomad/structs/workload_id.go

@@ -43,9 +43,9 @@ var (
 // WorkloadIdentity is the jobspec block which determines if and how a workload
 // identity is exposed to tasks similar to the Vault block.
 //
-// CAUTION: a copy of this struct definition lives in config/consul.go in order
-// to avoid import cycles. If updating WorkloadIdentity, please remember to update
-// its copy as well.
+// CAUTION: a similar struct called WorkloadIdentityConfig lives in
+// nomad/structs/config/workload_id.go and is used for agent configuration.
+// Updates here may need to be applied there as well.
 type WorkloadIdentity struct {
 	Name string