diff --git a/.changelog/25298.txt b/.changelog/25298.txt new file mode 100644 index 000000000..ca87352e3 --- /dev/null +++ b/.changelog/25298.txt @@ -0,0 +1,5 @@ +```release-note:breaking-change +consul: Identities are no longer added to tasks by default when they include a template block. +Please see [Nomad's upgrade guide](https://developer.hashicorp.com/nomad/docs/upgrade/upgrade-specific) +for more detail. +``` diff --git a/api/jobs_test.go b/api/jobs_test.go index 3e179e4e8..7068e1545 100644 --- a/api/jobs_test.go +++ b/api/jobs_test.go @@ -332,10 +332,6 @@ func TestJobs_Canonicalize(t *testing.T) { MaxDelay: pointerOf(1 * time.Hour), Unlimited: pointerOf(true), }, - Consul: &Consul{ - Namespace: "", - Cluster: "default", - }, Update: &UpdateStrategy{ Stagger: pointerOf(30 * time.Second), MaxParallel: pointerOf(1), @@ -417,10 +413,6 @@ func TestJobs_Canonicalize(t *testing.T) { MaxDelay: pointerOf(time.Duration(0)), Unlimited: pointerOf(false), }, - Consul: &Consul{ - Namespace: "", - Cluster: "default", - }, Tasks: []*Task{ { KillTimeout: pointerOf(5 * time.Second), @@ -507,10 +499,6 @@ func TestJobs_Canonicalize(t *testing.T) { MaxDelay: pointerOf(1 * time.Hour), Unlimited: pointerOf(true), }, - Consul: &Consul{ - Namespace: "", - Cluster: "default", - }, Update: &UpdateStrategy{ Stagger: pointerOf(30 * time.Second), MaxParallel: pointerOf(1), @@ -680,10 +668,6 @@ func TestJobs_Canonicalize(t *testing.T) { Migrate: pointerOf(false), SizeMB: pointerOf(300), }, - Consul: &Consul{ - Namespace: "", - Cluster: "default", - }, Update: &UpdateStrategy{ Stagger: pointerOf(30 * time.Second), MaxParallel: pointerOf(1), @@ -988,10 +972,6 @@ func TestJobs_Canonicalize(t *testing.T) { MaxDelay: pointerOf(1 * time.Hour), Unlimited: pointerOf(true), }, - Consul: &Consul{ - Namespace: "", - Cluster: "default", - }, Update: &UpdateStrategy{ Stagger: pointerOf(1 * time.Second), MaxParallel: pointerOf(1), @@ -1118,10 +1098,6 @@ func TestJobs_Canonicalize(t *testing.T) { MaxDelay: pointerOf(1 * time.Hour), Unlimited: pointerOf(true), }, - Consul: &Consul{ - Namespace: "", - Cluster: "default", - }, Update: &UpdateStrategy{ Stagger: pointerOf(30 * time.Second), MaxParallel: pointerOf(1), diff --git a/api/tasks.go b/api/tasks.go index 58c83cbfa..a59b0a391 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -553,11 +553,10 @@ func (g *TaskGroup) Canonicalize(job *Job) { } // Merge job.consul onto group.consul - if g.Consul == nil { - g.Consul = new(Consul) + if g.Consul != nil { + g.Consul.MergeNamespace(job.ConsulNamespace) + g.Consul.Canonicalize() } - g.Consul.MergeNamespace(job.ConsulNamespace) - g.Consul.Canonicalize() // Merge the update policy from the job if ju, tu := job.Update != nil, g.Update != nil; ju && tu { diff --git a/api/tasks_test.go b/api/tasks_test.go index f860fa16d..38f70c718 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -892,23 +892,6 @@ func TestTaskGroup_Canonicalize_Consul(t *testing.T) { must.Eq(t, "ns2", tg.Consul.Namespace) }) - t.Run("inherit job consul in group", func(t *testing.T) { - job := &Job{ - ID: pointerOf("job"), - ConsulNamespace: pointerOf("ns1"), - } - job.Canonicalize() - - tg := &TaskGroup{ - Name: pointerOf("group"), - Consul: nil, // not set, inherit from job - } - tg.Canonicalize(job) - - must.Eq(t, "ns1", *job.ConsulNamespace) - must.Eq(t, "ns1", tg.Consul.Namespace) - }) - t.Run("set in group only", func(t *testing.T) { job := &Job{ ID: pointerOf("job"), diff --git a/nomad/job_endpoint_hook_implicit_identities.go b/nomad/job_endpoint_hook_implicit_identities.go index a55f50f16..2126ee961 100644 --- a/nomad/job_endpoint_hook_implicit_identities.go +++ b/nomad/job_endpoint_hook_implicit_identities.go @@ -31,9 +31,10 @@ func (h jobImplicitIdentitiesHook) Mutate(job *structs.Job) (*structs.Job, []err h.handleConsulService(s, tg) hasIdentity = hasIdentity || s.Identity != nil } - if len(t.Templates) > 0 { - h.handleConsulTasks(t, tg) - } + + h.handleConsulTask(t, tg) + hasIdentity = hasIdentity || (len(t.Identities) > 0) + h.handleVault(t) hasIdentity = hasIdentity || (len(t.Identities) > 0) } @@ -90,8 +91,34 @@ func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service, tg *s s.Identity = serviceWID } -func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task, tg *structs.TaskGroup) { - widName := t.Consul.IdentityName() +// handleConsulTask injects a workload identity into the task for Consul if the +// task or task group includes a Consul block. The identity is generated in the +// following priority list: +// +// 1. A Consul identity configured in the task by an identity block. +// 2. Generated using the Consul block at the task level. +// 3. Generated using the Consul block at the task group level. +func (h jobImplicitIdentitiesHook) handleConsulTask(t *structs.Task, tg *structs.TaskGroup) { + + // If neither the task nor task group includes a Consul block, exit as we + // do not need to generate an identity. Operators can still specify + // identity blocks for Consul tasks which will allow workload access to the + // Consul API. + if t.Consul == nil && tg.Consul == nil { + return + } + + // The task or task group have a Consul block, now identify the workload + // identity name. The priority order is task followed by task group. It is + // important to be careful with the IdentityName() function as it returns a + // default non-empty value. + var widName string + + if t.Consul != nil { + widName = t.Consul.IdentityName() + } else if tg.Consul != nil { + widName = tg.Consul.IdentityName() + } // 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 index 7d3510302..4edc31abc 100644 --- a/nomad/job_endpoint_hook_implicit_identities_test.go +++ b/nomad/job_endpoint_hook_implicit_identities_test.go @@ -226,14 +226,35 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) { }}, }, }, + } + 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) + }) + } +} + +func Test_jobImplicitIdentitiesHook_Mutate_consulTask(t *testing.T) { + ci.Parallel(t) + + testCases := []struct { + name string + inputJob *structs.Job + inputConfig *Config + expectedOutputJob *structs.Job + }{ { - name: "mutate task to inject identity for templates", + name: "no consul block in task or task group", inputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ - Name: "group", Tasks: []*structs.Task{{ - Name: "web-task", - Templates: []*structs.Template{{}}, + Name: "web-task", }}, }}, }, @@ -248,27 +269,283 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) { }, expectedOutputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ - Name: "group", - Constraints: []*structs.Constraint{ - implicitIdentityClientVersionConstraint()}, Tasks: []*structs.Task{{ - Name: "web-task", - Templates: []*structs.Template{{}}, - Identities: []*structs.WorkloadIdentity{{ - Name: "consul_default", - Audience: []string{"consul.io"}, - }}, + Name: "web-task", }}, }}, }, }, { - name: "no mutation for templates when no task identity is configured", + name: "consul block in task without identity block", inputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ Tasks: []*structs.Task{{ - Name: "web-task", - Templates: []*structs.Template{{}}, + Name: "web-task", + Consul: &structs.Consul{}, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfigs: map[string]*config.ConsulConfig{ + structs.ConsulDefaultCluster: { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Constraints: []*structs.Constraint{ + implicitIdentityClientVersionConstraint(), + }, + Tasks: []*structs.Task{{ + Name: "web-task", + Consul: &structs.Consul{}, + Identities: []*structs.WorkloadIdentity{ + { + Name: "consul_default", + Audience: []string{"consul.io"}, + }, + }, + }}, + }}, + }, + }, + { + name: "consul block in task group without identity block", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Consul: &structs.Consul{}, + Tasks: []*structs.Task{{ + Name: "web-task", + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfigs: map[string]*config.ConsulConfig{ + structs.ConsulDefaultCluster: { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Consul: &structs.Consul{}, + Constraints: []*structs.Constraint{ + implicitIdentityClientVersionConstraint(), + }, + Tasks: []*structs.Task{{ + Name: "web-task", + Identities: []*structs.WorkloadIdentity{ + { + Name: "consul_default", + Audience: []string{"consul.io"}, + }, + }, + }}, + }}, + }, + }, + { + name: "consul block in task and task consul identity block", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Consul: &structs.Consul{Cluster: "alternative"}, + Identities: []*structs.WorkloadIdentity{ + { + Name: "consul_alternative", + Audience: []string{"consul_alternative.io"}, + }, + }, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfigs: map[string]*config.ConsulConfig{ + structs.ConsulDefaultCluster: { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + "alternative": { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul_alternative.io"}, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Constraints: []*structs.Constraint{ + implicitIdentityClientVersionConstraint(), + }, + Tasks: []*structs.Task{{ + Name: "web-task", + Consul: &structs.Consul{Cluster: "alternative"}, + Identities: []*structs.WorkloadIdentity{ + { + Name: "consul_alternative", + Audience: []string{"consul_alternative.io"}, + }, + }, + }}, + }}, + }, + }, + { + name: "consul block in task group and task consul identity block", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Consul: &structs.Consul{Cluster: "alternative"}, + Tasks: []*structs.Task{{ + Name: "web-task", + Identities: []*structs.WorkloadIdentity{ + { + Name: "consul_alternative", + Audience: []string{"consul_alternative.io"}, + }, + }, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfigs: map[string]*config.ConsulConfig{ + structs.ConsulDefaultCluster: { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + "alternative": { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul_alternative.io"}, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Consul: &structs.Consul{Cluster: "alternative"}, + Constraints: []*structs.Constraint{ + implicitIdentityClientVersionConstraint(), + }, + Tasks: []*structs.Task{{ + Name: "web-task", + Identities: []*structs.WorkloadIdentity{ + { + Name: "consul_alternative", + Audience: []string{"consul_alternative.io"}, + }, + }, + }}, + }}, + }, + }, + { + name: "consul block in task with existing non-consul identity", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Consul: &structs.Consul{}, + Identities: []*structs.WorkloadIdentity{ + { + Name: "vault_default", + Audience: []string{"vault.io"}, + }, + }, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfigs: map[string]*config.ConsulConfig{ + structs.ConsulDefaultCluster: { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Constraints: []*structs.Constraint{ + implicitIdentityClientVersionConstraint(), + }, + Tasks: []*structs.Task{{ + Name: "web-task", + Consul: &structs.Consul{}, + Identities: []*structs.WorkloadIdentity{ + { + Name: "vault_default", + Audience: []string{"vault.io"}, + }, + { + Name: "consul_default", + Audience: []string{"consul.io"}, + }, + }, + }}, + }}, + }, + }, + { + name: "consul block in task group with existing non-consul identity", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Consul: &structs.Consul{}, + Tasks: []*structs.Task{{ + Name: "web-task", + Identities: []*structs.WorkloadIdentity{ + { + Name: "vault_default", + Audience: []string{"vault.io"}, + }, + }, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfigs: map[string]*config.ConsulConfig{ + structs.ConsulDefaultCluster: { + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Consul: &structs.Consul{}, + Constraints: []*structs.Constraint{ + implicitIdentityClientVersionConstraint(), + }, + Tasks: []*structs.Task{{ + Name: "web-task", + Identities: []*structs.WorkloadIdentity{ + { + Name: "vault_default", + Audience: []string{"vault.io"}, + }, + { + Name: "consul_default", + Audience: []string{"consul.io"}, + }, + }, + }}, + }}, + }, + }, + { + name: "no mutation for task when no task identity is configured", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Consul: &structs.Consul{}, }}, }}, }, @@ -278,18 +555,17 @@ func Test_jobImplicitIdentitiesHook_Mutate_consul_service(t *testing.T) { expectedOutputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ Tasks: []*structs.Task{{ - Name: "web-task", - Templates: []*structs.Template{{}}, + Name: "web-task", + Consul: &structs.Consul{}, }}, }}, }, }, } + for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - impl := jobImplicitIdentitiesHook{srv: &Server{ - config: tc.inputConfig, - }} + impl := jobImplicitIdentitiesHook{srv: &Server{config: tc.inputConfig}} actualJob, actualWarnings, actualError := impl.Mutate(tc.inputJob) must.Eq(t, tc.expectedOutputJob, actualJob) must.NoError(t, actualError) diff --git a/nomad/structs/consul.go b/nomad/structs/consul.go index 4843a631d..e49bca1a2 100644 --- a/nomad/structs/consul.go +++ b/nomad/structs/consul.go @@ -102,72 +102,3 @@ func ValidateConsulClusterName(cluster string) error { return nil } - -// ConsulUsage is provides meta information about how Consul is used by a job, -// noting which connect services and normal services will be registered, and -// whether the keystore will be read via template. -type ConsulUsage struct { - Services []string - KV bool -} - -// Used returns true if Consul is used for registering services or reading from -// the keystore. -func (cu *ConsulUsage) Used() bool { - switch { - case cu.KV: - return true - case len(cu.Services) > 0: - return true - } - return false -} - -// ConsulUsages returns a map from Consul namespace to things that will use Consul, -// including ConsulConnect TaskKinds, Consul Services from groups and tasks, and -// a boolean indicating if Consul KV is in use. -func (j *Job) ConsulUsages() map[string]*ConsulUsage { - m := make(map[string]*ConsulUsage) - - for _, tg := range j.TaskGroups { - namespace := j.ConsulNamespace - if tgNamespace := tg.Consul.GetNamespace(); tgNamespace != "" { - namespace = tgNamespace - } - if _, exists := m[namespace]; !exists { - m[namespace] = new(ConsulUsage) - } - - // Gather group services - for _, service := range tg.Services { - if service.IsConsul() { - m[namespace].Services = append(m[namespace].Services, service.Name) - } - } - - // Gather task services and KV usage - for _, task := range tg.Tasks { - taskNamespace := namespace - if task.Consul != nil && task.Consul.Namespace != "" { - taskNamespace = task.Consul.Namespace - } - - for _, service := range task.Services { - if service.IsConsul() { - if _, exists := m[taskNamespace]; !exists { - m[taskNamespace] = new(ConsulUsage) - } - m[taskNamespace].Services = append(m[taskNamespace].Services, service.Name) - } - } - if len(task.Templates) > 0 { - if _, exists := m[taskNamespace]; !exists { - m[taskNamespace] = new(ConsulUsage) - } - m[taskNamespace].KV = true - } - } - } - - return m -} diff --git a/website/content/docs/integrations/consul/acl.mdx b/website/content/docs/integrations/consul/acl.mdx index 2469cde3a..b9ee16c5f 100644 --- a/website/content/docs/integrations/consul/acl.mdx +++ b/website/content/docs/integrations/consul/acl.mdx @@ -82,8 +82,14 @@ blocks. To avoid having to add these additional identities to every job, you can configure Nomad servers with the [`consul.service_identity`][] and [`consul.task_identity`][] agent configuration. Upon job registration, the -Nomad servers update tasks that have a [`consul`][] or [`template`][] block and -services that use the Consul service provider with these default identities. +Nomad servers update tasks that have a [`consul`][] block and services that use +the Consul service provider with these default identities. + +Job specifications that include [`template`][] blocks are not provided with +default identities because Nomad is unable to decipher the contents of the +template data. You must specify the identities required for Consul in the job +specification. Refer to the [Workload Identities for Consul][jobspec_identity_consul] +section of the `identity` block documentation for more information. You can also specify identities for Consul directly in the job. When provided, they override the Nomad server configuration. Refer to the [Workload Identities diff --git a/website/content/docs/job-specification/identity.mdx b/website/content/docs/job-specification/identity.mdx index a02f809c4..518d8358a 100644 --- a/website/content/docs/job-specification/identity.mdx +++ b/website/content/docs/job-specification/identity.mdx @@ -83,7 +83,7 @@ job "docs" { readable by that user. Otherwise the file is readable by everyone but is protected by parent directory permissions. - `filepath` `(string: "")` - If not empty and file is `true`, the workload - identity will be available at the specified location relative to the + identity is available at the specified location relative to the [task working directory][] instead of the `NOMAD_SECRETS_DIR`. - `ttl` `(string: "")` - The lifetime of the identity before it expires. The client will renew the identity at roughly half the TTL. This is specified @@ -104,8 +104,13 @@ inside the task or service that will access Consul. You can configure Nomad servers to automatically add default identities for Consul using the [`consul.service_identity`][] and [`consul.task_identity`][] agent configuration. Upon job registration, the Nomad server updates tasks that -have a [`consul`][] or [`template`][] block and services that use the Consul -service provider to include the default identities. +have a [`consul`][] block and services that use the Consul service provider to +include the default identities. + +Job specifications that include [`template`][] blocks are not provided with +default identities because Nomad is unable to decipher the contents of the +template data. You must specify the identities required for Consul in the job +specification. You can also specify these identities directly in the job. When provided, they override the default identities configured in the Nomad servers. Identities for diff --git a/website/content/docs/upgrade/upgrade-specific.mdx b/website/content/docs/upgrade/upgrade-specific.mdx index 8bac0d772..c7818b2a6 100644 --- a/website/content/docs/upgrade/upgrade-specific.mdx +++ b/website/content/docs/upgrade/upgrade-specific.mdx @@ -82,6 +82,11 @@ Before upgrading to Nomad 1.10, perform the following tasks: Refer to [Migrating to Using Workload Identity with Vault][] for more details. +#### Consul template implicit workload identity removal +Nomad no longer creates an implicit Consul identity for workloads that don't +register services with Consul. Tasks that require Consul tokens for template +rendering must include a [`consul` block][] or specify an [`identity`][]. + ## Nomad 1.9.5 #### CNI plugins @@ -2339,3 +2344,5 @@ deleted and then Nomad 0.3.0 can be launched. [Process Isolation]: https://learn.microsoft.com/en-us/virtualization/windowscontainers/manage-containers/hyperv-container#process-isolation [reschedule]: /nomad/docs/jobs-specification/reschedule [`infra_image`]: /nomad/docs/drivers/docker#infra_image +[`identity`]: /nomad/docs/job-specification/identity#identity-block +[`consul` block]: /nomad/docs/job-specification/consul