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.
This commit is contained in:
Luiz Aoqui
2023-09-27 17:44:07 -03:00
committed by GitHub
parent 868aba57bb
commit fed1992cea
11 changed files with 31 additions and 144 deletions

View File

@@ -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)

View File

@@ -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"]

View File

@@ -400,8 +400,7 @@
"task_token_ttl": "1s",
"tls_server_name": "foobar",
"tls_skip_verify": true,
"token": "12345",
"use_identity": true
"token": "12345"
}
],
"reporting":{

View File

@@ -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 {

View File

@@ -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
}

View File

@@ -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"},
},

View File

@@ -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()

View File

@@ -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",
},

View File

@@ -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_<name>", where
// <name> 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
}

View File

@@ -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),

View File

@@ -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` <code>([Identity](#default_identity-parameters): nil)</code> -
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 {