From fed1992cea5ec4f81294076805737c83fa2b0e74 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 27 Sep 2023 17:44:07 -0300 Subject: [PATCH] vault: remove `use_identity` agent config (#18592) The initial intention behind the `vault.use_identity` configuration was to indicate to Nomad servers that they would need to sign a workload identities for allocs with a `vault` block. But in order to support identity renewal, #18262 and #18431 moved the token signing logic to the alloc runner since a new token needs to be signed prior to the TTL expiring. So #18343 implemented `use_identity` as a flag to indicate that the workload identity JWT flow should be used when deriving Vault tokens for tasks. But this configuration value is set on servers so it is not available to clients at the time of token derivation, making its meaning not clear: a job may end up using the identity-based flow even when `use_identity` is `false`. The only reliable signal available to clients at token derivation time is the presence of an `identity` block for Vault, and this is already configured with the `vault.default_identity` configuration block, making `vault.use_identity` redundant. This commit removes the `vault.use_identity` configuration and simplifies the logic on when an implicit Vault identity is injected into tasks. --- command/agent/config_parse_test.go | 3 -- command/agent/testdata/basic.hcl | 1 - command/agent/testdata/basic.json | 3 +- nomad/config.go | 7 --- .../job_endpoint_hook_implicit_identities.go | 6 +-- ..._endpoint_hook_implicit_identities_test.go | 38 ++------------ nomad/job_endpoint_hooks.go | 38 +++++--------- nomad/job_endpoint_hooks_test.go | 51 ++++--------------- nomad/structs/config/vault.go | 13 ----- nomad/structs/config/vault_test.go | 7 --- website/content/docs/configuration/vault.mdx | 8 --- 11 files changed, 31 insertions(+), 144 deletions(-) diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index f649f37e4..313ea1a17 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -306,7 +306,6 @@ var basicConfig = &Config{ TLSSkipVerify: &trueValue, TaskTokenTTL: "1s", Token: "12345", - UseIdentity: pointer.Of(true), DefaultIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"vault.io", "nomad.io"}, Env: pointer.Of(false), @@ -331,7 +330,6 @@ var basicConfig = &Config{ TLSSkipVerify: &trueValue, TaskTokenTTL: "1s", Token: "12345", - UseIdentity: pointer.Of(true), DefaultIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"vault.io", "nomad.io"}, Env: pointer.Of(false), @@ -1097,7 +1095,6 @@ func TestConfig_MultipleVault(t *testing.T) { must.Eq(t, "abracadabra", cfg.Vault.Token) must.MapLen(t, 3, cfg.Vaults) - must.Equal(t, cfg.Vault, cfg.Vaults["default"]) must.Equal(t, cfg.Vault, cfg.Vaults[structs.VaultDefaultCluster]) must.Eq(t, "alternate", cfg.Vaults["alternate"].Name) diff --git a/command/agent/testdata/basic.hcl b/command/agent/testdata/basic.hcl index cde60006d..1e226a2ac 100644 --- a/command/agent/testdata/basic.hcl +++ b/command/agent/testdata/basic.hcl @@ -271,7 +271,6 @@ vault { tls_server_name = "foobar" tls_skip_verify = true create_from_role = "test_role" - use_identity = true default_identity { aud = ["vault.io", "nomad.io"] diff --git a/command/agent/testdata/basic.json b/command/agent/testdata/basic.json index 2f47eb3f0..bb9b2f737 100644 --- a/command/agent/testdata/basic.json +++ b/command/agent/testdata/basic.json @@ -400,8 +400,7 @@ "task_token_ttl": "1s", "tls_server_name": "foobar", "tls_skip_verify": true, - "token": "12345", - "use_identity": true + "token": "12345" } ], "reporting":{ diff --git a/nomad/config.go b/nomad/config.go index 9e7fd2e47..1d5318a4a 100644 --- a/nomad/config.go +++ b/nomad/config.go @@ -500,13 +500,6 @@ func (c *Config) UseConsulIdentity() bool { *c.ConsulConfig.UseIdentity } -// UseVaultIdentity returns true when Vault workload identity is enabled. -func (c *Config) UseVaultIdentity() bool { - return c.VaultConfig != nil && - c.VaultConfig.UseIdentity != nil && - *c.VaultConfig.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 { diff --git a/nomad/job_endpoint_hook_implicit_identities.go b/nomad/job_endpoint_hook_implicit_identities.go index 76e6978c1..0fe1482d8 100644 --- a/nomad/job_endpoint_hook_implicit_identities.go +++ b/nomad/job_endpoint_hook_implicit_identities.go @@ -108,10 +108,10 @@ func (h jobImplicitIdentitiesHook) handleConsulTasks(t *structs.Task, consul *st // 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 -// `vault.default_identity` is provided. +// 2. The task does not have an identity for the Vault cluster. +// 3. The server is configured with a `vault.default_identity`. func (h jobImplicitIdentitiesHook) handleVault(t *structs.Task) { - if !h.srv.config.UseVaultIdentity() || t.Vault == nil { + if t.Vault == nil { return } diff --git a/nomad/job_endpoint_hook_implicit_identities_test.go b/nomad/job_endpoint_hook_implicit_identities_test.go index 1fc80646e..8af7ab767 100644 --- a/nomad/job_endpoint_hook_implicit_identities_test.go +++ b/nomad/job_endpoint_hook_implicit_identities_test.go @@ -339,7 +339,6 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }, inputConfig: &Config{ VaultConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), DefaultIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"vault.io"}, }, @@ -352,7 +351,7 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }, }, { - name: "no mutation when vault identity is disabled", + name: "no mutation when no default identity is provided", inputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ Tasks: []*structs.Task{{ @@ -361,34 +360,7 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }}, }, inputConfig: &Config{ - VaultConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(false), - DefaultIdentity: &config.WorkloadIdentityConfig{ - Audience: []string{"vault.io"}, - }, - }, - }, - expectedOutputJob: &structs.Job{ - TaskGroups: []*structs.TaskGroup{{ - Tasks: []*structs.Task{{ - Vault: &structs.Vault{}, - }}, - }}, - }, - }, - { - name: "no mutation when vault identity is enabled but no default identity is configured", - inputJob: &structs.Job{ - TaskGroups: []*structs.TaskGroup{{ - Tasks: []*structs.Task{{ - Vault: &structs.Vault{}, - }}, - }}, - }, - inputConfig: &Config{ - VaultConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), - }, + VaultConfig: &config.VaultConfig{}, }, expectedOutputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ @@ -415,9 +387,8 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }, inputConfig: &Config{ VaultConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), DefaultIdentity: &config.WorkloadIdentityConfig{ - Audience: []string{"vault.io"}, + Audience: []string{"vault-from-config.io"}, }, }, }, @@ -436,7 +407,7 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }, }, { - name: "mutate when task does not have a vault idenity", + name: "mutate when task does not have a vault identity", inputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ Tasks: []*structs.Task{{ @@ -448,7 +419,6 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }, inputConfig: &Config{ VaultConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), DefaultIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"vault.io"}, }, diff --git a/nomad/job_endpoint_hooks.go b/nomad/job_endpoint_hooks.go index 3afc16ee3..76ec21f20 100644 --- a/nomad/job_endpoint_hooks.go +++ b/nomad/job_endpoint_hooks.go @@ -404,48 +404,36 @@ func (v *jobValidate) validateVaultIdentity(t *structs.Task) ([]error, error) { var mErr *multierror.Error var warnings []error - hasVault := t.Vault != nil - hasDefaultWID := v.srv.config.VaultDefaultIdentity() != nil - vaultWIDs := []string{} for _, wid := range t.Identities { if strings.HasPrefix(wid.Name, structs.WorkloadIdentityVaultPrefix) { vaultWIDs = append(vaultWIDs, wid.Name) } } - hasVaultWID := len(vaultWIDs) > 0 + hasTaskWID := len(vaultWIDs) > 0 + hasDefaultWID := v.srv.config.VaultDefaultIdentity() != nil - useIdentity := hasVault && v.srv.config.UseVaultIdentity() - hasWID := hasVaultWID || hasDefaultWID - - if useIdentity { - if !hasWID { - mErr = multierror.Append(mErr, fmt.Errorf( - "Task %s expected to have a Vault identity, add an identity block called %s or provide a default using the default_identity block in the server Vault configuration", - t.Name, t.Vault.IdentityName(), - )) + if t.Vault == nil { + for _, wid := range vaultWIDs { + warnings = append(warnings, fmt.Errorf("Task %s has an identity called %s but no vault block", t.Name, wid)) } + return warnings, nil + } + if hasTaskWID || hasDefaultWID { if len(t.Vault.Policies) > 0 { warnings = append(warnings, fmt.Errorf( "Task %s has a Vault block with policies but uses workload identity to authenticate with Vault, policies will be ignored", t.Name, )) } - } else if hasVault && len(t.Vault.Policies) == 0 { - mErr = multierror.Append(mErr, fmt.Errorf("Task %s has a Vault block with an empty list of policies", t.Name)) + return warnings, nil } - for _, wid := range vaultWIDs { - if !v.srv.config.UseVaultIdentity() { - warnings = append(warnings, fmt.Errorf( - "Task %s has an identity called %s but server is not configured to use Vault identities, set use_identity to true in the Vault server configuration", - t.Name, wid, - )) - } - if !hasVault { - warnings = append(warnings, fmt.Errorf("Task %s has an identity called %s but no vault block", t.Name, wid)) - } + // At this point Nomad will use the legacy token-based flow, so keep the + // existing validations. + if len(t.Vault.Policies) == 0 { + mErr = multierror.Append(mErr, fmt.Errorf("Task %s has a Vault block with an empty list of policies", t.Name)) } return warnings, mErr.ErrorOrNil() diff --git a/nomad/job_endpoint_hooks_test.go b/nomad/job_endpoint_hooks_test.go index 792995f7d..a679db8d6 100644 --- a/nomad/job_endpoint_hooks_test.go +++ b/nomad/job_endpoint_hooks_test.go @@ -179,21 +179,10 @@ func Test_jobValidate_Validate_vault(t *testing.T) { expectedErr string }{ { - name: "no error when vault identity is not enabled and task does not have a vault identity", - inputTaskVault: &structs.Vault{ - Policies: []string{"nomad-workload"}, - }, - inputTaskIdentities: nil, - inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(false), - }, - }, - { - name: "no error when vault identity is enabled and identity is provided via config", + name: "no error when vault identity is provided via config", inputTaskVault: &structs.Vault{}, inputTaskIdentities: nil, inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), DefaultIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"vault.io"}, TTL: pointer.Of(time.Hour), @@ -201,43 +190,29 @@ func Test_jobValidate_Validate_vault(t *testing.T) { }, }, { - name: "no error when vault identity is enabled and identity is provided via task", + name: "no error when vault identity is provided via task", inputTaskVault: &structs.Vault{}, inputTaskIdentities: []*structs.WorkloadIdentity{{ Name: "vault_default", Audience: []string{"vault.io"}, TTL: time.Hour, }}, - inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), - }, + inputConfig: &config.VaultConfig{}, }, { name: "error when not using vault identity and vault block is missing policies", inputTaskVault: &structs.Vault{}, inputTaskIdentities: nil, - inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(false), - }, - expectedErr: "Vault block with an empty list of policies", + inputConfig: &config.VaultConfig{}, + expectedErr: "Vault block with an empty list of policies", }, { - name: "error when vault identity is enabled but no identity is provided", - inputTaskVault: &structs.Vault{}, - inputTaskIdentities: nil, - inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), - }, - expectedErr: "expected to have a Vault identity", - }, - { - name: "warn when vault identity is enabled but task has vault policies", + name: "warn when using default vault identity but task has vault policies", inputTaskVault: &structs.Vault{ Policies: []string{"nomad-workload"}, }, inputTaskIdentities: nil, inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), DefaultIdentity: &config.WorkloadIdentityConfig{ Audience: []string{"vault.io"}, TTL: pointer.Of(time.Hour), @@ -246,7 +221,7 @@ func Test_jobValidate_Validate_vault(t *testing.T) { expectedWarns: []string{"policies will be ignored"}, }, { - name: "warn when vault identity is disabled but task has vault identity", + name: "warn when using task vault identity but task has vault policies", inputTaskVault: &structs.Vault{ Policies: []string{"nomad-workload"}, }, @@ -255,12 +230,8 @@ func Test_jobValidate_Validate_vault(t *testing.T) { Audience: []string{"vault.io"}, TTL: time.Hour, }}, - inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(false), - }, - expectedWarns: []string{ - "has an identity called vault_default but server is not configured to use Vault identities", - }, + inputConfig: &config.VaultConfig{}, + expectedWarns: []string{"policies will be ignored"}, }, { name: "warn when vault identity is provided but task does not have vault block", @@ -270,9 +241,7 @@ func Test_jobValidate_Validate_vault(t *testing.T) { Audience: []string{"vault.io"}, TTL: time.Hour, }}, - inputConfig: &config.VaultConfig{ - UseIdentity: pointer.Of(true), - }, + inputConfig: &config.VaultConfig{}, expectedWarns: []string{ "has an identity called vault_default but no vault block", }, diff --git a/nomad/structs/config/vault.go b/nomad/structs/config/vault.go index 53238cb87..afa891851 100644 --- a/nomad/structs/config/vault.go +++ b/nomad/structs/config/vault.go @@ -87,13 +87,6 @@ type VaultConfig struct { // Servers-only fields. - // UseIdentity defines if workload identities should be used to derive - // Vault tokens. - // - // It is a transitional field used only during the adoption period of - // workload identities and will be ignored and removed in future versions. - UseIdentity *bool `mapstructure:"use_identity"` - // DefaultIdentity is the default workload identity configuration used when // a job has a `vault` block but no `identity` named "vault_", where // matches this block `name` parameter. @@ -135,7 +128,6 @@ func DefaultVaultConfig() *VaultConfig { Addr: "https://vault.service.consul:8200", ConnectionRetryIntv: DefaultVaultConnectRetryIntv, AllowUnauthenticated: pointer.Of(true), - UseIdentity: pointer.Of(false), } } @@ -192,8 +184,6 @@ func (c *VaultConfig) Merge(b *VaultConfig) *VaultConfig { result.TLSServerName = b.TLSServerName } - result.UseIdentity = pointer.Merge(result.UseIdentity, b.UseIdentity) - if result.DefaultIdentity == nil && b.DefaultIdentity != nil { sID := *b.DefaultIdentity result.DefaultIdentity = &sID @@ -298,9 +288,6 @@ func (c *VaultConfig) Equal(b *VaultConfig) bool { return false } - if !pointer.Eq(b.UseIdentity, c.UseIdentity) { - return false - } if !c.DefaultIdentity.Equal(b.DefaultIdentity) { return false } diff --git a/nomad/structs/config/vault_test.go b/nomad/structs/config/vault_test.go index 5c0127413..c81d5fd8e 100644 --- a/nomad/structs/config/vault_test.go +++ b/nomad/structs/config/vault_test.go @@ -29,7 +29,6 @@ func TestVaultConfig_Merge(t *testing.T) { TLSKeyFile: "1", TLSSkipVerify: pointer.Of(true), TLSServerName: "1", - UseIdentity: pointer.Of(false), DefaultIdentity: nil, } @@ -46,7 +45,6 @@ func TestVaultConfig_Merge(t *testing.T) { TLSKeyFile: "2", TLSSkipVerify: nil, TLSServerName: "2", - UseIdentity: pointer.Of(true), DefaultIdentity: &WorkloadIdentityConfig{ Audience: []string{"vault.dev"}, Env: pointer.Of(true), @@ -67,7 +65,6 @@ func TestVaultConfig_Merge(t *testing.T) { TLSKeyFile: "2", TLSSkipVerify: pointer.Of(true), TLSServerName: "2", - UseIdentity: pointer.Of(true), DefaultIdentity: &WorkloadIdentityConfig{ Audience: []string{"vault.dev"}, Env: pointer.Of(true), @@ -99,7 +96,6 @@ func TestVaultConfig_Equals(t *testing.T) { TLSKeyFile: "1", TLSSkipVerify: pointer.Of(true), TLSServerName: "1", - UseIdentity: pointer.Of(true), DefaultIdentity: &WorkloadIdentityConfig{ Audience: []string{"vault.dev"}, Env: pointer.Of(true), @@ -122,7 +118,6 @@ func TestVaultConfig_Equals(t *testing.T) { TLSKeyFile: "1", TLSSkipVerify: pointer.Of(true), TLSServerName: "1", - UseIdentity: pointer.Of(true), DefaultIdentity: &WorkloadIdentityConfig{ Audience: []string{"vault.dev"}, Env: pointer.Of(true), @@ -147,7 +142,6 @@ func TestVaultConfig_Equals(t *testing.T) { TLSKeyFile: "1", TLSSkipVerify: pointer.Of(true), TLSServerName: "1", - UseIdentity: pointer.Of(true), DefaultIdentity: &WorkloadIdentityConfig{ Audience: []string{"vault.dev"}, Env: pointer.Of(true), @@ -170,7 +164,6 @@ func TestVaultConfig_Equals(t *testing.T) { TLSKeyFile: "1", TLSSkipVerify: pointer.Of(true), TLSServerName: "1", - UseIdentity: pointer.Of(false), DefaultIdentity: &WorkloadIdentityConfig{ Audience: []string{"vault.io"}, Env: pointer.Of(false), diff --git a/website/content/docs/configuration/vault.mdx b/website/content/docs/configuration/vault.mdx index 832648131..59b3f2409 100644 --- a/website/content/docs/configuration/vault.mdx +++ b/website/content/docs/configuration/vault.mdx @@ -28,7 +28,6 @@ information about the Vault integration. ```hcl vault { enabled = true - use_identity = true default_identity { aud = ["vault.io"] @@ -133,10 +132,6 @@ agents with [`client.enabled`] set to `true`. These parameters should only be defined in the configuration file of Nomad agents with [`server.enabled`] set to `true`. -- `use_identity` `(bool: false)` - If set to `true`, Nomad uses workload - identities to generate Vault ACL tokens. If `false` the deprecated - token-based flow is used. - - `default_identity` ([Identity](#default_identity-parameters): nil) - Specifies the default workload identity configuration to use when a task with a `vault` block does not specify an [`identity`][jobspec_identity] block @@ -238,9 +233,6 @@ vault { # workload identities. create_from_role = "nomad-cluster" - # Generate Vault tokens using workload identities. - use_identity = true - # Provide a default workload identity configuration so jobs don't need to # specify one. default_identity {