From d7bd47d60f5eccec5b29544df26721356ffdd3d4 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 20 Sep 2023 15:43:08 -0400 Subject: [PATCH] config: remove `consul.template_identity` in lieu of `task_identity` (#18540) The original thinking for Workload Identity integration with Consul and Vault was that we'd allow `template` blocks to specify their own identity. But because the login to Consul/Vault to get tokens happens at the task level, this would involve making the `template` block a new WID watcher on its own rather than using the Consul and Vault hooks we're building at the group/task level. So it doesn't make sense to have separate identities for individual `template` blocks rather than at the level of tasks. Update the agent configuration to rename the `template_identity` to the more accurate `task_identity`, which will be used for any non-service hooks (just `template` today). Update the implicit identities job mutation hook to create the identity we'll need as well. --- command/agent/config_parse.go | 24 +++--- command/agent/config_parse_test.go | 4 +- command/agent/testdata/basic.hcl | 2 +- command/agent/testdata/basic.json | 2 +- nomad/config.go | 8 +- .../job_endpoint_hook_implicit_identities.go | 30 +++++++ ..._endpoint_hook_implicit_identities_test.go | 82 +++++++++++++++++++ nomad/structs/config/consul.go | 18 ++-- 8 files changed, 141 insertions(+), 29 deletions(-) diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index b14777ba1..d197fdab1 100644 --- a/command/agent/config_parse.go +++ b/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.WorkloadIdentityConfig{}, - TemplateIdentity: &config.WorkloadIdentityConfig{}, + ServiceIdentity: &config.WorkloadIdentityConfig{}, + TaskIdentity: &config.WorkloadIdentityConfig{}, }, Consuls: map[string]*config.ConsulConfig{}, Autopilot: &config.AutopilotConfig{}, @@ -189,11 +189,11 @@ func ParseConfigFile(path string) (*Config, error) { }) } - if consulConfig.TemplateIdentity != nil { + if consulConfig.TaskIdentity != nil { tds = append(tds, durationConversionMap{ - fmt.Sprintf("consuls.%s.template_identity.ttl", name), nil, &consulConfig.TemplateIdentity.TTLHCL, + fmt.Sprintf("consuls.%s.task_identity.ttl", name), nil, &consulConfig.TaskIdentity.TTLHCL, func(d *time.Duration) { - consulConfig.TemplateIdentity.TTL = d + consulConfig.TaskIdentity.TTL = d }, }) } @@ -440,7 +440,7 @@ func parseConsuls(c *Config, list *ast.ObjectList) error { } delete(m, "service_identity") - delete(m, "template_identity") + delete(m, "task_identity") cc := &config.ConsulConfig{} err := mapstructure.WeakDecode(m, cc) @@ -486,18 +486,18 @@ func parseConsuls(c *Config, list *ast.ObjectList) error { c.Consuls[cc.Name].ServiceIdentity = &serviceIdentity } - if o := listVal.Filter("template_identity"); len(o.Items) > 0 { + if o := listVal.Filter("task_identity"); len(o.Items) > 0 { var m map[string]interface{} - templateIdentityBlock := o.Items[0] - if err := hcl.DecodeObject(&m, templateIdentityBlock.Val); err != nil { + taskIdentityBlock := o.Items[0] + if err := hcl.DecodeObject(&m, taskIdentityBlock.Val); err != nil { return err } - var templateIdentity config.WorkloadIdentityConfig - if err := mapstructure.WeakDecode(m, &templateIdentity); err != nil { + var taskIdentity config.WorkloadIdentityConfig + if err := mapstructure.WeakDecode(m, &taskIdentity); err != nil { return err } - c.Consuls[cc.Name].TemplateIdentity = &templateIdentity + c.Consuls[cc.Name].TaskIdentity = &taskIdentity } } diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index 587d2255d..36fdee492 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -243,7 +243,7 @@ var basicConfig = &Config{ TTL: pointer.Of(1 * time.Hour), TTLHCL: "1h", }, - TemplateIdentity: &config.WorkloadIdentityConfig{ + TaskIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"consul.io"}, Env: pointer.Of(true), File: pointer.Of(false), @@ -283,7 +283,7 @@ var basicConfig = &Config{ TTL: pointer.Of(1 * time.Hour), TTLHCL: "1h", }, - TemplateIdentity: &config.WorkloadIdentityConfig{ + TaskIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"consul.io"}, Env: pointer.Of(true), File: pointer.Of(false), diff --git a/command/agent/testdata/basic.hcl b/command/agent/testdata/basic.hcl index e9c48a5e4..754671d88 100644 --- a/command/agent/testdata/basic.hcl +++ b/command/agent/testdata/basic.hcl @@ -250,7 +250,7 @@ consul { file = true ttl = "1h" } - template_identity { + task_identity { aud = ["consul.io"] env = true file = false diff --git a/command/agent/testdata/basic.json b/command/agent/testdata/basic.json index 2723bb427..6f1bc3d0a 100644 --- a/command/agent/testdata/basic.json +++ b/command/agent/testdata/basic.json @@ -177,7 +177,7 @@ "ttl": "1h" }, "ssl": true, - "template_identity": { + "task_identity": { "aud": [ "consul.io" ], diff --git a/nomad/config.go b/nomad/config.go index 1cca718f5..c0d8c0dac 100644 --- a/nomad/config.go +++ b/nomad/config.go @@ -473,14 +473,14 @@ func (c *Config) ConsulServiceIdentity() *structs.WorkloadIdentity { 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 { +// ConsulTaskIdentity returns the workload identity to be used for accessing the +// Consul API from task hooks not supporting services (ex templates). +func (c *Config) ConsulTaskIdentity() *structs.WorkloadIdentity { if c.ConsulConfig == nil { return nil } - return workloadIdentityFromConfig(c.ConsulConfig.TemplateIdentity) + return workloadIdentityFromConfig(c.ConsulConfig.TaskIdentity) } // VaultDefaultIdentity returns the workload identity to be used for accessing diff --git a/nomad/job_endpoint_hook_implicit_identities.go b/nomad/job_endpoint_hook_implicit_identities.go index ba938ff37..35f7ba6c6 100644 --- a/nomad/job_endpoint_hook_implicit_identities.go +++ b/nomad/job_endpoint_hook_implicit_identities.go @@ -11,6 +11,7 @@ import ( const ( consulServiceIdentityNamePrefix = "consul-service" + consulTaskIdentityNamePrefix = "consul" vaultIdentityName = "vault" ) @@ -34,6 +35,9 @@ func (h jobImplicitIdentitiesHook) Mutate(job *structs.Job) (*structs.Job, []err for _, s := range t.Services { h.handleConsulService(s) } + if len(t.Templates) > 0 { + h.handleConsulTasks(t, tg.Consul) + } h.handleVault(t) } } @@ -77,6 +81,32 @@ func (h jobImplicitIdentitiesHook) handleConsulService(s *structs.Service) { s.Identity = serviceWID } +func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task, consul *structs.Consul) { + if !h.srv.config.UseConsulIdentity() { + return + } + + widName := fmt.Sprintf("%s/%s", consulTaskIdentityNamePrefix, t.Name) + + // Use the Consul identity specified in the task if present + for _, wid := range t.Identities { + if wid.Name == widName { + return + } + } + + // If task doesn't specify an identity for Consul, fallback to the + // default identity defined in the server configuration. + taskWID := h.srv.config.ConsulTaskIdentity() + if taskWID == nil { + // If no identity is found skip inject the implicit identity and + // fallback to the legacy flow. + return + } + taskWID.Name = widName + t.Identities = append(t.Identities, taskWID) +} + // handleVault injects a workload identity to the task if: // 1. The task has a Vault block. // 2. The server is configured with `vault.use_identity = true` and a diff --git a/nomad/job_endpoint_hook_implicit_identities_test.go b/nomad/job_endpoint_hook_implicit_identities_test.go index c6b8f35a6..88e791194 100644 --- a/nomad/job_endpoint_hook_implicit_identities_test.go +++ b/nomad/job_endpoint_hook_implicit_identities_test.go @@ -225,6 +225,88 @@ func Test_jobImplicitIndentitiesHook_Mutate_consul_service(t *testing.T) { }}, }, }, + { + name: "mutate task to inject identity for templates", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Templates: []*structs.Template{{}}, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfig: &config.ConsulConfig{ + UseIdentity: pointer.Of(true), + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Templates: []*structs.Template{{}}, + Identities: []*structs.WorkloadIdentity{{ + Name: "consul/web-task", + Audience: []string{"consul.io"}, + }}, + }}, + }}, + }, + }, + { + name: "no mutation for templates when identity is enabled but no task identity is configured", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Templates: []*structs.Template{{}}, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfig: &config.ConsulConfig{ + UseIdentity: pointer.Of(true), + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Templates: []*structs.Template{{}}, + }}, + }}, + }, + }, + { + name: "no task mutation for templates when identity is disabled", + inputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Templates: []*structs.Template{{}}, + }}, + }}, + }, + inputConfig: &Config{ + ConsulConfig: &config.ConsulConfig{ + UseIdentity: pointer.Of(false), + TaskIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"consul.io"}, + }, + }, + }, + expectedOutputJob: &structs.Job{ + TaskGroups: []*structs.TaskGroup{{ + Tasks: []*structs.Task{{ + Name: "web-task", + Templates: []*structs.Template{{}}, + }}, + }}, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/nomad/structs/config/consul.go b/nomad/structs/config/consul.go index 5c338329a..3060d3fce 100644 --- a/nomad/structs/config/consul.go +++ b/nomad/structs/config/consul.go @@ -156,7 +156,7 @@ type ConsulConfig struct { // ServiceIdentity is set on the server. ServiceIdentity *WorkloadIdentityConfig `mapstructure:"service_identity"` - // TemplateIdentity is intended to reduce overhead for jobspec authors and make + // TaskIdentity is intended to reduce overhead for jobspec authors and make // for graceful upgrades without forcing rewrite of all jobspecs. If set, when a // job has both a template block and a consul block, the Nomad server will sign a // Workload Identity for that task. The client will use this identity rather than @@ -164,8 +164,8 @@ type ConsulConfig struct { // // The name field of the identity is always set to "consul". // - // TemplateIdentity is set on the server. - TemplateIdentity *WorkloadIdentityConfig `mapstructure:"template_identity"` + // TaskIdentity is set on the server. + TaskIdentity *WorkloadIdentityConfig `mapstructure:"task_identity"` // ExtraKeysHCL is used by hcl to surface unexpected keys ExtraKeysHCL []string `mapstructure:",unusedKeys" json:"-"` @@ -304,11 +304,11 @@ func (c *ConsulConfig) Merge(b *ConsulConfig) *ConsulConfig { result.ServiceIdentity = result.ServiceIdentity.Merge(b.ServiceIdentity) } - 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) + if result.TaskIdentity == nil && b.TaskIdentity != nil { + tID := *b.TaskIdentity + result.TaskIdentity = &tID + } else if b.TaskIdentity != nil { + result.TaskIdentity = result.TaskIdentity.Merge(b.TaskIdentity) } return result @@ -412,7 +412,7 @@ func (c *ConsulConfig) Copy() *ConsulConfig { Namespace: c.Namespace, UseIdentity: c.UseIdentity, ServiceIdentity: c.ServiceIdentity.Copy(), - TemplateIdentity: c.TemplateIdentity.Copy(), + TaskIdentity: c.TaskIdentity.Copy(), ExtraKeysHCL: slices.Clone(c.ExtraKeysHCL), } }