From c43e30a3876450cba0385a461f26932c49b662d9 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 3 Sep 2024 13:56:36 -0400 Subject: [PATCH] WI: interpolate parent job ID in `vault.default_identity.extra_claims` (#23817) When we interpolate job fields for the `vault.default_identity.extra_claims` block, we forgot to use the parent job ID when that's available (as we do for all other claims). This changeset fixes the bug and adds a helper method that'll hopefully remind us to do this going forward. Also added a missing changelog entry for #23675 where we implemented the `extra_claims` block originally, which shipped in Nomad 1.8.3. Fixes: https://github.com/hashicorp/nomad/issues/23798 --- .changelog/23675.txt | 3 ++ .changelog/23817.txt | 3 ++ CHANGELOG.md | 1 + nomad/auth/auth.go | 5 +- nomad/structs/structs.go | 9 ++++ nomad/structs/workload_id.go | 10 ++-- nomad/structs/workload_id_test.go | 49 ++++++++++---------- website/content/docs/configuration/vault.mdx | 1 + 8 files changed, 47 insertions(+), 34 deletions(-) create mode 100644 .changelog/23675.txt create mode 100644 .changelog/23817.txt diff --git a/.changelog/23675.txt b/.changelog/23675.txt new file mode 100644 index 000000000..2beb31189 --- /dev/null +++ b/.changelog/23675.txt @@ -0,0 +1,3 @@ +```release-note:improvement +identity: Added support for server-configured additional claims on the Vault default_identity block +``` diff --git a/.changelog/23817.txt b/.changelog/23817.txt new file mode 100644 index 000000000..18b061573 --- /dev/null +++ b/.changelog/23817.txt @@ -0,0 +1,3 @@ +```release-note:bug +identity: Fixed a bug where dispatch and periodic jobs would have their job ID and not parent job ID used when interpolating vault.default_identity.extra_claims +``` diff --git a/CHANGELOG.md b/CHANGELOG.md index 167ef62e7..003db437a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ IMPROVEMENTS: * cli: `acl token create` will now emit a warning if the token has a policy that does not yet exist [[GH-16437](https://github.com/hashicorp/nomad/issues/16437)] * keyring: Added support for encrypting the keyring via Vault transit or external KMS [[GH-23580](https://github.com/hashicorp/nomad/issues/23580)] * keyring: Added support for prepublishing keys [[GH-23577](https://github.com/hashicorp/nomad/issues/23577)] +* identity: Added support for server-configured additional claims on the Vault default_identity block [[GH-23675](https://github.com/hashicorp/nomad/issues/23675)] * metrics: Added `client.tasks` metrics to track task states [[GH-23773](https://github.com/hashicorp/nomad/issues/23773)] * resources: Added `resources.secrets` field to configure size of secrets directory on Linux [[GH-23696](https://github.com/hashicorp/nomad/issues/23696)] * tls: Allow setting the `tls_min_version` field to `"tls13"` [[GH-23713](https://github.com/hashicorp/nomad/issues/23713)] diff --git a/nomad/auth/auth.go b/nomad/auth/auth.go index 940d17583..2189a6274 100644 --- a/nomad/auth/auth.go +++ b/nomad/auth/auth.go @@ -641,10 +641,7 @@ func (s *Authenticator) ResolvePoliciesForClaims(claims *structs.IdentityClaims) } // Find any policies attached to the job - jobId := alloc.Job.ID - if alloc.Job.ParentID != "" { - jobId = alloc.Job.ParentID - } + jobId := alloc.Job.GetIDforWorkloadIdentity() iter, err := snap.ACLPolicyByJob(nil, alloc.Namespace, jobId) if err != nil { return nil, err diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 5b8b57f46..10f020dde 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -4604,6 +4604,15 @@ func (j *Job) GetNamespace() string { return j.Namespace } +// GetIDforWorkloadIdentity is used when we want the job ID for identity; here we +// always want the parent ID if there is one and then fallback to the ID +func (j *Job) GetIDforWorkloadIdentity() string { + if j.ParentID != "" { + return j.ParentID + } + return j.ID +} + // GetCreateIndex implements the CreateIndexGetter interface, required for // pagination. func (j *Job) GetCreateIndex() uint64 { diff --git a/nomad/structs/workload_id.go b/nomad/structs/workload_id.go index c66c4bbb3..0a61be9a7 100644 --- a/nomad/structs/workload_id.go +++ b/nomad/structs/workload_id.go @@ -191,7 +191,7 @@ func (b *IdentityClaimsBuilder) Build(now time.Time) *IdentityClaims { jwtnow := jwt.NewNumericDate(now.UTC()) claims := &IdentityClaims{ Namespace: b.alloc.Namespace, - JobID: b.job.ID, + JobID: b.job.GetIDforWorkloadIdentity(), AllocationID: b.alloc.ID, ServiceName: b.serviceName, Claims: jwt.Claims{ @@ -200,10 +200,6 @@ func (b *IdentityClaimsBuilder) Build(now time.Time) *IdentityClaims { }, ExtraClaims: b.extras, } - // If this is a child job, use the parent's ID - if b.job.ParentID != "" { - claims.JobID = b.job.ParentID - } if b.task != nil && b.wihandle.WorkloadType != WorkloadTypeService { claims.TaskName = b.task.Name } @@ -235,13 +231,15 @@ func (b *IdentityClaimsBuilder) interpolate() { if len(b.extras) == 0 { return } + r := strings.NewReplacer( // attributes that always exist "${job.region}", b.job.Region, "${job.namespace}", b.job.Namespace, - "${job.id}", b.job.ID, + "${job.id}", b.job.GetIDforWorkloadIdentity(), "${job.node_pool}", b.job.NodePool, "${group.name}", b.tg.Name, + "${alloc.id}", b.alloc.ID, // attributes that conditionally exist "${node.id}", strAttrGet(b.node, func(n *Node) string { return n.ID }), diff --git a/nomad/structs/workload_id_test.go b/nomad/structs/workload_id_test.go index c355cf8c7..2e69f8a78 100644 --- a/nomad/structs/workload_id_test.go +++ b/nomad/structs/workload_id_test.go @@ -20,6 +20,7 @@ func TestNewIdentityClaims(t *testing.T) { job := &Job{ ID: "job", + ParentID: "parentJob", Name: "job", Namespace: "default", Region: "global", @@ -178,7 +179,7 @@ func TestNewIdentityClaims(t *testing.T) { // group: no consul. "job/group/services/group-service": { Namespace: "default", - JobID: "job", + JobID: "parentJob", ServiceName: "group-service", Claims: jwt.Claims{ Subject: "global:default:job:group:group-service:consul-service_group-service-http", @@ -190,7 +191,7 @@ func TestNewIdentityClaims(t *testing.T) { // task: no consul, no vault. "job/group/task/default-identity": { Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", Claims: jwt.Claims{ Subject: "global:default:job:group:task:default-identity", @@ -200,7 +201,7 @@ func TestNewIdentityClaims(t *testing.T) { }, "job/group/task/alt-identity": { Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", Claims: jwt.Claims{ Subject: "global:default:job:group:task:alt-identity", @@ -213,7 +214,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/group/task/consul_default": { ConsulNamespace: "", Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", Claims: jwt.Claims{ Subject: "global:default:job:group:task:consul_default", @@ -226,7 +227,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/group/task/vault_default": { VaultNamespace: "", Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", VaultRole: "", // not specified in jobspec Claims: jwt.Claims{ @@ -234,12 +235,12 @@ func TestNewIdentityClaims(t *testing.T) { Audience: jwt.Audience{"vault.io"}, }, ExtraClaims: map[string]string{ - "nomad_workload_id": "global:default:job", + "nomad_workload_id": "global:default:parentJob", }, }, "job/group/task/services/task-service": { Namespace: "default", - JobID: "job", + JobID: "parentJob", ServiceName: "task-service", Claims: jwt.Claims{ Subject: "global:default:job:group:task-service:consul-service_task-task-service-http", @@ -251,7 +252,7 @@ func TestNewIdentityClaims(t *testing.T) { // task: with consul, with vault. "job/group/consul-vault-task/default-identity": { Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "consul-vault-task", Claims: jwt.Claims{ Subject: "global:default:job:group:consul-vault-task:default-identity", @@ -263,7 +264,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/group/consul-vault-task/consul_default": { ConsulNamespace: "task-consul-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "consul-vault-task", Claims: jwt.Claims{ Subject: "global:default:job:group:consul-vault-task:consul_default", @@ -275,7 +276,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/group/consul-vault-task/vault_default": { VaultNamespace: "vault-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "consul-vault-task", VaultRole: "role-from-spec-group", Claims: jwt.Claims{ @@ -283,14 +284,14 @@ func TestNewIdentityClaims(t *testing.T) { Audience: jwt.Audience{"vault.io"}, }, ExtraClaims: map[string]string{ - "nomad_workload_id": "global:default:job", + "nomad_workload_id": "global:default:parentJob", }, }, // Use task-level Consul namespace for task services. "job/group/consul-vault-task/services/consul-vault-task-service": { ConsulNamespace: "task-consul-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", ServiceName: "consul-vault-task-service", Claims: jwt.Claims{ Subject: "global:default:job:group:consul-vault-task-service:consul-service_consul-vault-task-service-http", @@ -303,7 +304,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/consul-group/services/group-service": { ConsulNamespace: "group-consul-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", ServiceName: "group-service", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:group-service:consul-service_group-service-http", @@ -315,7 +316,7 @@ func TestNewIdentityClaims(t *testing.T) { // task: no consul, no vault. "job/consul-group/task/default-identity": { Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:task:default-identity", @@ -325,7 +326,7 @@ func TestNewIdentityClaims(t *testing.T) { }, "job/consul-group/task/alt-identity": { Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:task:alt-identity", @@ -338,7 +339,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/consul-group/task/consul_default": { ConsulNamespace: "group-consul-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:task:consul_default", @@ -348,7 +349,7 @@ func TestNewIdentityClaims(t *testing.T) { }, "job/consul-group/task/vault_default": { Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "task", VaultRole: "", // not specified in jobspec Claims: jwt.Claims{ @@ -356,7 +357,7 @@ func TestNewIdentityClaims(t *testing.T) { Audience: jwt.Audience{"vault.io"}, }, ExtraClaims: map[string]string{ - "nomad_workload_id": "global:default:job", + "nomad_workload_id": "global:default:parentJob", }, }, // Use group-level Consul namespace for task service because task @@ -364,7 +365,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/consul-group/task/services/task-service": { ConsulNamespace: "group-consul-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", ServiceName: "task-service", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:task-service:consul-service_task-task-service-http", @@ -376,7 +377,7 @@ func TestNewIdentityClaims(t *testing.T) { // task: with consul, with vault. "job/consul-group/consul-vault-task/default-identity": { Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "consul-vault-task", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:consul-vault-task:default-identity", @@ -388,7 +389,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/consul-group/consul-vault-task/consul_default": { ConsulNamespace: "task-consul-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "consul-vault-task", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:consul-vault-task:consul_default", @@ -399,7 +400,7 @@ func TestNewIdentityClaims(t *testing.T) { "job/consul-group/consul-vault-task/vault_default": { VaultNamespace: "vault-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", TaskName: "consul-vault-task", VaultRole: "role-from-spec-consul-group", Claims: jwt.Claims{ @@ -407,14 +408,14 @@ func TestNewIdentityClaims(t *testing.T) { Audience: jwt.Audience{"vault.io"}, }, ExtraClaims: map[string]string{ - "nomad_workload_id": "global:default:job", + "nomad_workload_id": "global:default:parentJob", }, }, // Use task-level Consul namespace for task services. "job/consul-group/consul-vault-task/services/consul-task-service": { ConsulNamespace: "task-consul-namespace", Namespace: "default", - JobID: "job", + JobID: "parentJob", ServiceName: "consul-task-service", Claims: jwt.Claims{ Subject: "global:default:job:consul-group:consul-task-service:consul-service_consul-vault-task-consul-task-service-http", diff --git a/website/content/docs/configuration/vault.mdx b/website/content/docs/configuration/vault.mdx index 7b3a6b345..60a25739d 100644 --- a/website/content/docs/configuration/vault.mdx +++ b/website/content/docs/configuration/vault.mdx @@ -203,6 +203,7 @@ will be removed in a future release. - `${job.id}` - The job's ID. - `${job.node_pool}` - The node pool where the allocation is running. - `${group.name}` - The task group name of the task using Vault. + - `${alloc.id}` - The allocation's ID. - `${task.name}` - The name of the task using Vault. - `${node.id}` - The ID of the node where the allocation is running. - `${node.datacenter}` - The datacenter of the node where the allocation is running.