Skip to content

Commit

Permalink
vault: validate tasks using non-default clusters (#18810)
Browse files Browse the repository at this point in the history 
Since Nomad servers only start a Vault client for the default cluster,
tasks using non-default clusters must provide an identity to be used for
token derivation, either in the task itself or in the agent
configuration.
  • Loading branch information
@lgfa29
lgfa29 authored Oct 26, 2023
1 parent 8f8265f commit 61d4ee7
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 31 deletions.
39 changes: 39 additions & 0 deletions nomad/job_endpoint_hook_implicit_identities_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,45 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) {
}},
},
},
{
name: "mutate when task does not have a vault identity for non-default cluster",
inputJob: &structs.Job{
TaskGroups: []*structs.TaskGroup{{
Tasks: []*structs.Task{{
Vault: &structs.Vault{
Cluster: "other",
},
}},
}},
},
inputConfig: &Config{
VaultConfigs: map[string]*config.VaultConfig{
structs.VaultDefaultCluster: {
DefaultIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"vault.io"},
},
},
"other": {
DefaultIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"vault-other.io"},
},
},
},
},
expectedOutputJob: &structs.Job{
TaskGroups: []*structs.TaskGroup{{
Tasks: []*structs.Task{{
Identities: []*structs.WorkloadIdentity{{
Name: "vault_other",
Audience: []string{"vault-other.io"},
}},
Vault: &structs.Vault{
Cluster: "other",
},
}},
}},
},
},
}

for _, tc := range testCases {
Expand Down
57 changes: 33 additions & 24 deletions nomad/job_endpoint_hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,44 +410,53 @@ func (v *jobValidate) validateServiceIdentity(s *structs.Service, parent string)
return nil
}

// validateVaultIdentity validates that a task is properly configured to access
// a Vault cluster.
//
// It assumes the jobImplicitIdentitiesHook mutator hook has been called to
// inject task identities if necessary.
func (v *jobValidate) validateVaultIdentity(t *structs.Task) ([]error, error) {
var mErr *multierror.Error
var warnings []error

vaultWIDs := make([]string, 0, len(t.Identities))
for _, wid := range t.Identities {
if strings.HasPrefix(wid.Name, structs.WorkloadIdentityVaultPrefix) {
vaultWIDs = append(vaultWIDs, wid.Name)
}
}

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))
// Warn if task doesn't use Vault but has Vault identities.
for _, wid := range t.Identities {
if strings.HasPrefix(wid.Name, structs.WorkloadIdentityVaultPrefix) {
warnings = append(warnings, fmt.Errorf("Task %s has an identity called %s but no vault block", t.Name, wid.Name))
}
}
return warnings, nil
}

hasTaskWID := len(vaultWIDs) > 0
hasDefaultWID := v.srv.config.VaultIdentityConfig(t.Vault.Cluster) != nil
vaultWIDName := t.Vault.IdentityName()
vaultWID := t.GetIdentity(vaultWIDName)
if vaultWID == nil {
// Tasks using non-default clusters are required to have an identity.
if t.Vault.Cluster != structs.VaultDefaultCluster {
return warnings, fmt.Errorf(
"Task %s uses Vault cluster %s but does not have an identity named %s and no default identity is provided in agent configuration",
t.Name, t.Vault.Cluster, vaultWIDName,
)
}

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,
))
// Tasks using the default cluster but without a Vault identity will
// use the legacy flow.
if len(t.Vault.Policies) == 0 {
return warnings, fmt.Errorf("Task %s has a Vault block with an empty list of policies", t.Name)
}

return warnings, nil
}

// 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))
// Warn if tasks is using identity-based flow with the deprecated policies
// field.
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,
))
}

return warnings, mErr.ErrorOrNil()
return warnings, nil
}

type memoryOversubscriptionValidate struct {
Expand Down
71 changes: 64 additions & 7 deletions nomad/job_endpoint_hooks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,20 +153,70 @@ func Test_jobValidate_Validate_vault(t *testing.T) {
},
},
{
name: "no error when vault identity is provided via task",
inputTaskVault: &structs.Vault{},
name: "no error when vault identity is provided via config from non-default cluster",
inputTaskVault: &structs.Vault{
Cluster: "other",
},
inputTaskIdentities: nil,
inputConfig: map[string]*config.VaultConfig{
structs.VaultDefaultCluster: {},
"other": {
DefaultIdentity: &config.WorkloadIdentityConfig{
Audience: []string{"vault.io"},
TTL: pointer.Of(time.Hour),
},
},
},
},
{
name: "no error when vault identity is provided via task",
inputTaskVault: &structs.Vault{
Cluster: structs.VaultDefaultCluster,
},
inputTaskIdentities: []*structs.WorkloadIdentity{{
Name: "vault_default",
Audience: []string{"vault.io"},
TTL: time.Hour,
}},
},
{
name: "error when not using vault identity and vault block is missing policies",
inputTaskVault: &structs.Vault{},
name: "no error when vault identity is provided via task for non-default cluster",
inputTaskVault: &structs.Vault{
Cluster: "other",
},
inputTaskIdentities: []*structs.WorkloadIdentity{{
Name: "vault_other",
Audience: []string{"vault.io"},
TTL: time.Hour,
}},
},
{
name: "no error when task uses legacy flow with default cluster",
inputTaskVault: &structs.Vault{
Cluster: structs.VaultDefaultCluster,
Policies: []string{"nomad-workload"},
},
},
{
name: "error when not using vault identity and vault block is missing policies",
inputTaskVault: &structs.Vault{
Cluster: structs.VaultDefaultCluster,
},
inputTaskIdentities: nil,
expectedErr: "Vault block with an empty list of policies",
},
{
name: "error when no identity is available for non-default cluster",
inputTaskVault: &structs.Vault{
Cluster: "other",
},
inputTaskIdentities: nil,
inputConfig: map[string]*config.VaultConfig{
structs.VaultDefaultCluster: {},
"other": {},
},
expectedErr: "does not have an identity named vault_other",
},
{
name: "warn when using default vault identity but task has vault policies",
inputTaskVault: &structs.Vault{
Expand All @@ -187,6 +237,7 @@ func Test_jobValidate_Validate_vault(t *testing.T) {
{
name: "warn when using task vault identity but task has vault policies",
inputTaskVault: &structs.Vault{
Cluster: structs.VaultDefaultCluster,
Policies: []string{"nomad-workload"},
},
inputTaskIdentities: []*structs.WorkloadIdentity{{
Expand All @@ -212,12 +263,14 @@ func Test_jobValidate_Validate_vault(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
impl := jobValidate{srv: &Server{
srv := &Server{
config: &Config{
JobMaxPriority: 100,
VaultConfigs: tc.inputConfig,
},
}}
}
implicitIdentities := jobImplicitIdentitiesHook{srv: srv}
impl := jobValidate{srv: srv}

job := mock.Job()
task := job.TaskGroups[0].Tasks[0]
Expand All @@ -228,7 +281,11 @@ func Test_jobValidate_Validate_vault(t *testing.T) {
task.Vault.ChangeMode = structs.VaultChangeModeRestart
}

warns, err := impl.Validate(job)
mutatedJob, warn, err := implicitIdentities.Mutate(job)
must.NoError(t, err)
must.SliceEmpty(t, warn)

warns, err := impl.Validate(mutatedJob)

if len(tc.expectedErr) == 0 {
must.NoError(t, err)
Expand Down

0 comments on commit 61d4ee7

Please sign in to comment.