From 8b9a5fde4e103a9c5347ae95a65f745520feb110 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 18 Oct 2023 20:45:01 -0400 Subject: [PATCH] vault: add multi-cluster support on templates (#18790) In Nomad Enterprise, a task may connect to a non-default Vault cluster, requiring `consul-template` to be configured with a specific client `vault` block. --- .../taskrunner/task_runner_test.go | 12 +++- .../taskrunner/template/template.go | 27 +++++---- .../taskrunner/template/template_test.go | 59 +++++++++++++------ .../allocrunner/taskrunner/template_hook.go | 19 +++++- nomad/config.go | 11 ++-- .../job_endpoint_hook_implicit_identities.go | 2 +- ..._endpoint_hook_implicit_identities_test.go | 28 +++++---- nomad/job_endpoint_hooks.go | 7 ++- nomad/job_endpoint_hooks_test.go | 35 ++++++----- 9 files changed, 132 insertions(+), 68 deletions(-) diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index ddf724395..9cc269499 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -118,10 +118,13 @@ func testTaskRunnerConfig(t *testing.T, alloc *structs.Allocation, taskName stri nomadRegMock := regMock.NewServiceRegistrationHandler(logger) wrapperMock := wrapper.NewHandlerWrapper(logger, consulRegMock, nomadRegMock) - task := alloc.LookupTask(taskName) - widsigner := widmgr.NewMockWIDSigner(task.Identities) + widsigner := widmgr.NewMockWIDSigner(thisTask.Identities) db := cstate.NewMemDB(logger) + if thisTask.Vault != nil { + clientConf.VaultConfigs[structs.VaultDefaultCluster].Enabled = pointer.Of(true) + } + var vaultFunc vaultclient.VaultClientFunc if vault != nil { vaultFunc = func(_ string) (vaultclient.VaultClient, error) { return vault, nil } @@ -2294,7 +2297,10 @@ func TestTaskRunner_Template_BlockingPreStart(t *testing.T) { }, } - task.Vault = &structs.Vault{Policies: []string{"default"}} + task.Vault = &structs.Vault{ + Cluster: structs.VaultDefaultCluster, + Policies: []string{"default"}, + } conf, cleanup := testTaskRunnerConfig(t, alloc, task.Name, nil) defer cleanup() diff --git a/client/allocrunner/taskrunner/template/template.go b/client/allocrunner/taskrunner/template/template.go index 4261037f3..c2a6543c0 100644 --- a/client/allocrunner/taskrunner/template/template.go +++ b/client/allocrunner/taskrunner/template/template.go @@ -25,6 +25,7 @@ import ( "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/helper/pointer" "github.com/hashicorp/nomad/nomad/structs" + structsc "github.com/hashicorp/nomad/nomad/structs/config" ) const ( @@ -102,6 +103,10 @@ type TaskTemplateManagerConfig struct { // VaultToken is the Vault token for the task. VaultToken string + // VaultConfig is the Vault configuration to use for this template. It may + // be nil if the task does not use Vault. + VaultConfig *structsc.VaultConfig + // VaultNamespace is the Vault namespace for the task VaultNamespace string @@ -881,30 +886,30 @@ func newRunnerConfig(config *TaskTemplateManagerConfig, emptyStr := "" conf.Vault.RenewToken = pointer.Of(false) conf.Vault.Token = &emptyStr - if cc.VaultConfig != nil && cc.VaultConfig.IsEnabled() { - conf.Vault.Address = &cc.VaultConfig.Addr + if config.VaultConfig != nil && config.VaultConfig.IsEnabled() { + conf.Vault.Address = &config.VaultConfig.Addr conf.Vault.Token = &config.VaultToken // Set the Vault Namespace. Passed in Task config has // highest precedence. - if config.ClientConfig.VaultConfig.Namespace != "" { - conf.Vault.Namespace = &config.ClientConfig.VaultConfig.Namespace + if config.VaultConfig.Namespace != "" { + conf.Vault.Namespace = &config.VaultConfig.Namespace } if config.VaultNamespace != "" { conf.Vault.Namespace = &config.VaultNamespace } - if strings.HasPrefix(cc.VaultConfig.Addr, "https") || cc.VaultConfig.TLSCertFile != "" { - skipVerify := cc.VaultConfig.TLSSkipVerify != nil && *cc.VaultConfig.TLSSkipVerify + if strings.HasPrefix(config.VaultConfig.Addr, "https") || config.VaultConfig.TLSCertFile != "" { + skipVerify := config.VaultConfig.TLSSkipVerify != nil && *config.VaultConfig.TLSSkipVerify verify := !skipVerify conf.Vault.SSL = &ctconf.SSLConfig{ Enabled: pointer.Of(true), Verify: &verify, - Cert: &cc.VaultConfig.TLSCertFile, - Key: &cc.VaultConfig.TLSKeyFile, - CaCert: &cc.VaultConfig.TLSCaFile, - CaPath: &cc.VaultConfig.TLSCaPath, - ServerName: &cc.VaultConfig.TLSServerName, + Cert: &config.VaultConfig.TLSCertFile, + Key: &config.VaultConfig.TLSKeyFile, + CaCert: &config.VaultConfig.TLSCaFile, + CaPath: &config.VaultConfig.TLSCaPath, + ServerName: &config.VaultConfig.TLSServerName, } } else { conf.Vault.SSL = &ctconf.SSLConfig{ diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go index 7e1daeca8..dcb1f378b 100644 --- a/client/allocrunner/taskrunner/template/template_test.go +++ b/client/allocrunner/taskrunner/template/template_test.go @@ -121,7 +121,10 @@ func newTestHarness(t *testing.T, templates []*structs.Template, consul, vault b if vault { harness.vault = testutil.NewTestVault(t) - harness.config.VaultConfig = harness.vault.Config + harness.config.VaultConfigs = map[string]*sconfig.VaultConfig{ + structs.VaultDefaultCluster: harness.vault.Config, + } + harness.config.VaultConfig = harness.config.VaultConfigs[structs.VaultDefaultCluster] harness.vaultToken = harness.vault.RootToken } @@ -143,6 +146,7 @@ func (h *testHarness) startWithErr() error { Templates: h.templates, ClientConfig: h.config, VaultToken: h.vaultToken, + VaultConfig: h.config.VaultConfigs[structs.VaultDefaultCluster], TaskDir: h.taskDir, EnvBuilder: h.envBuilder, MaxTemplateEventRate: h.emitRate, @@ -1705,14 +1709,19 @@ func TestTaskTemplateManager_Config_ServerName(t *testing.T) { ci.Parallel(t) c := config.DefaultConfig() c.Node = mock.Node() - c.VaultConfig = &sconfig.VaultConfig{ - Enabled: pointer.Of(true), - Addr: "https://localhost/", - TLSServerName: "notlocalhost", + c.VaultConfigs = map[string]*sconfig.VaultConfig{ + structs.VaultDefaultCluster: { + Enabled: pointer.Of(true), + Addr: "https://localhost/", + TLSServerName: "notlocalhost", + }, } + c.VaultConfig = c.VaultConfigs[structs.VaultDefaultCluster] + config := &TaskTemplateManagerConfig{ ClientConfig: c, VaultToken: "token", + VaultConfig: c.VaultConfigs[structs.VaultDefaultCluster], } ctconf, err := newRunnerConfig(config, nil) if err != nil { @@ -1733,17 +1742,21 @@ func TestTaskTemplateManager_Config_VaultNamespace(t *testing.T) { testNS := "test-namespace" c := config.DefaultConfig() c.Node = mock.Node() - c.VaultConfig = &sconfig.VaultConfig{ - Enabled: pointer.Of(true), - Addr: "https://localhost/", - TLSServerName: "notlocalhost", - Namespace: testNS, + c.VaultConfigs = map[string]*sconfig.VaultConfig{ + structs.VaultDefaultCluster: { + Enabled: pointer.Of(true), + Addr: "https://localhost/", + TLSServerName: "notlocalhost", + Namespace: testNS, + }, } + c.VaultConfig = c.VaultConfigs[structs.VaultDefaultCluster] alloc := mock.Alloc() config := &TaskTemplateManagerConfig{ ClientConfig: c, VaultToken: "token", + VaultConfig: c.VaultConfigs[structs.VaultDefaultCluster], EnvBuilder: taskenv.NewBuilder(c.Node, alloc, alloc.Job.TaskGroups[0].Tasks[0], c.Region), } @@ -1764,12 +1777,15 @@ func TestTaskTemplateManager_Config_VaultNamespace_TaskOverride(t *testing.T) { testNS := "test-namespace" c := config.DefaultConfig() c.Node = mock.Node() - c.VaultConfig = &sconfig.VaultConfig{ - Enabled: pointer.Of(true), - Addr: "https://localhost/", - TLSServerName: "notlocalhost", - Namespace: testNS, + c.VaultConfigs = map[string]*sconfig.VaultConfig{ + structs.VaultDefaultCluster: { + Enabled: pointer.Of(true), + Addr: "https://localhost/", + TLSServerName: "notlocalhost", + Namespace: testNS, + }, } + c.VaultConfig = c.VaultConfigs[structs.VaultDefaultCluster] alloc := mock.Alloc() overriddenNS := "new-namespace" @@ -1778,6 +1794,7 @@ func TestTaskTemplateManager_Config_VaultNamespace_TaskOverride(t *testing.T) { config := &TaskTemplateManagerConfig{ ClientConfig: c, VaultToken: "token", + VaultConfig: c.VaultConfigs[structs.VaultDefaultCluster], VaultNamespace: overriddenNS, EnvBuilder: taskenv.NewBuilder(c.Node, alloc, alloc.Job.TaskGroups[0].Tasks[0], c.Region), } @@ -2150,10 +2167,13 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) { clientConfig := config.DefaultConfig() clientConfig.Node = mock.Node() - clientConfig.VaultConfig = &sconfig.VaultConfig{ - Enabled: pointer.Of(true), - Namespace: testNS, + clientConfig.VaultConfigs = map[string]*sconfig.VaultConfig{ + structs.VaultDefaultCluster: { + Enabled: pointer.Of(true), + Namespace: testNS, + }, } + clientConfig.VaultConfig = clientConfig.VaultConfigs[structs.VaultDefaultCluster] clientConfig.ConsulConfig = &sconfig.ConsulConfig{ Namespace: testNS, @@ -2209,6 +2229,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) { &TaskTemplateManagerConfig{ ClientConfig: clientConfig, VaultToken: "token", + VaultConfig: clientConfig.VaultConfigs[structs.VaultDefaultCluster], EnvBuilder: taskenv.NewBuilder(clientConfig.Node, alloc, alloc.Job.TaskGroups[0].Tasks[0], clientConfig.Region), }, &config.Config{ @@ -2242,6 +2263,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) { &TaskTemplateManagerConfig{ ClientConfig: clientConfig, VaultToken: "token", + VaultConfig: clientConfig.VaultConfigs[structs.VaultDefaultCluster], EnvBuilder: taskenv.NewBuilder(clientConfig.Node, allocWithOverride, allocWithOverride.Job.TaskGroups[0].Tasks[0], clientConfig.Region), }, &config.Config{ @@ -2279,6 +2301,7 @@ func TestTaskTemplateManager_ClientTemplateConfig_Set(t *testing.T) { &TaskTemplateManagerConfig{ ClientConfig: clientConfig, VaultToken: "token", + VaultConfig: clientConfig.VaultConfigs[structs.VaultDefaultCluster], EnvBuilder: taskenv.NewBuilder(clientConfig.Node, allocWithOverride, allocWithOverride.Job.TaskGroups[0].Tasks[0], clientConfig.Region), Templates: []*structs.Template{ { diff --git a/client/allocrunner/taskrunner/template_hook.go b/client/allocrunner/taskrunner/template_hook.go index 63e102100..37c5c72a5 100644 --- a/client/allocrunner/taskrunner/template_hook.go +++ b/client/allocrunner/taskrunner/template_hook.go @@ -16,6 +16,7 @@ import ( cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/nomad/structs" + structsc "github.com/hashicorp/nomad/nomad/structs/config" ) const ( @@ -86,6 +87,9 @@ type templateHook struct { // workload identity consulToken string + // task is the task that defines these templates + task *structs.Task + // taskDir is the task directory taskDir string } @@ -116,7 +120,8 @@ func (h *templateHook) Prestart(ctx context.Context, req *interfaces.TaskPrestar h.templateManager = nil } - // Store the current Vault token and the task directory + // Store request information so they can be used in other hooks. + h.task = req.Task h.taskDir = req.TaskDir.Dir h.vaultToken = req.VaultToken h.nomadToken = req.NomadToken @@ -184,6 +189,17 @@ func (h *templateHook) Poststart(ctx context.Context, req *interfaces.TaskPostst func (h *templateHook) newManager() (unblock chan struct{}, err error) { unblock = make(chan struct{}) + + var vaultConfig *structsc.VaultConfig + if h.task.Vault != nil { + vaultCluster := h.task.Vault.Cluster + vaultConfig = h.config.clientConfig.GetVaultConfigs(h.logger)[vaultCluster] + + if vaultConfig == nil { + return nil, fmt.Errorf("Vault cluster %q is disabled or not configured", vaultCluster) + } + } + m, err := template.NewTaskTemplateManager(&template.TaskTemplateManagerConfig{ UnblockCh: unblock, Lifecycle: h.config.lifecycle, @@ -193,6 +209,7 @@ func (h *templateHook) newManager() (unblock chan struct{}, err error) { ConsulNamespace: h.config.consulNamespace, ConsulToken: h.consulToken, VaultToken: h.vaultToken, + VaultConfig: vaultConfig, VaultNamespace: h.vaultNamespace, TaskDir: h.taskDir, EnvBuilder: h.config.envBuilder, diff --git a/nomad/config.go b/nomad/config.go index b8577c0ed..b42de4b11 100644 --- a/nomad/config.go +++ b/nomad/config.go @@ -485,14 +485,15 @@ func (c *Config) ConsulTaskIdentity() *structs.WorkloadIdentity { return workloadIdentityFromConfig(c.ConsulConfig.TaskIdentity) } -// VaultDefaultIdentity returns the workload identity to be used for accessing -// the Vault API. -func (c *Config) VaultDefaultIdentity() *structs.WorkloadIdentity { - if c.VaultConfig == nil { +// VaultIdentityConfig returns the workload identity to be used for accessing +// the API of a given Vault cluster. +func (c *Config) VaultIdentityConfig(cluster string) *structs.WorkloadIdentity { + conf := c.VaultConfigs[cluster] + if conf == nil { return nil } - return workloadIdentityFromConfig(c.VaultConfig.DefaultIdentity) + return workloadIdentityFromConfig(conf.DefaultIdentity) } // workloadIdentityFromConfig returns a structs.WorkloadIdentity to be used in diff --git a/nomad/job_endpoint_hook_implicit_identities.go b/nomad/job_endpoint_hook_implicit_identities.go index d3a219c14..4533b9562 100644 --- a/nomad/job_endpoint_hook_implicit_identities.go +++ b/nomad/job_endpoint_hook_implicit_identities.go @@ -111,7 +111,7 @@ func (h jobImplicitIdentitiesHook) handleVault(t *structs.Task) { // If the task doesn't specify an identity for Vault, fallback to the // default identity defined in the server configuration. - vaultWID = h.srv.config.VaultDefaultIdentity() + vaultWID = h.srv.config.VaultIdentityConfig(t.Vault.Cluster) if vaultWID == nil { // If no identity is found skip inject the implicit identity and // fallback to the legacy flow. diff --git a/nomad/job_endpoint_hook_implicit_identities_test.go b/nomad/job_endpoint_hook_implicit_identities_test.go index 511203f76..19f3f368b 100644 --- a/nomad/job_endpoint_hook_implicit_identities_test.go +++ b/nomad/job_endpoint_hook_implicit_identities_test.go @@ -302,9 +302,11 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }}, }, inputConfig: &Config{ - VaultConfig: &config.VaultConfig{ - DefaultIdentity: &config.WorkloadIdentityConfig{ - Audience: []string{"vault.io"}, + VaultConfigs: map[string]*config.VaultConfig{ + structs.VaultDefaultCluster: { + DefaultIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"vault.io"}, + }, }, }, }, @@ -324,7 +326,9 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }}, }, inputConfig: &Config{ - VaultConfig: &config.VaultConfig{}, + VaultConfigs: map[string]*config.VaultConfig{ + structs.VaultDefaultCluster: {}, + }, }, expectedOutputJob: &structs.Job{ TaskGroups: []*structs.TaskGroup{{ @@ -350,9 +354,11 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }}, }, inputConfig: &Config{ - VaultConfig: &config.VaultConfig{ - DefaultIdentity: &config.WorkloadIdentityConfig{ - Audience: []string{"vault-from-config.io"}, + VaultConfigs: map[string]*config.VaultConfig{ + structs.VaultDefaultCluster: { + DefaultIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"vault-from-config.io"}, + }, }, }, }, @@ -382,9 +388,11 @@ func Test_jobImplicitIndentitiesHook_Mutate_vault(t *testing.T) { }}, }, inputConfig: &Config{ - VaultConfig: &config.VaultConfig{ - DefaultIdentity: &config.WorkloadIdentityConfig{ - Audience: []string{"vault.io"}, + VaultConfigs: map[string]*config.VaultConfig{ + structs.VaultDefaultCluster: { + DefaultIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"vault.io"}, + }, }, }, }, diff --git a/nomad/job_endpoint_hooks.go b/nomad/job_endpoint_hooks.go index 0477185f2..d374fa15c 100644 --- a/nomad/job_endpoint_hooks.go +++ b/nomad/job_endpoint_hooks.go @@ -388,14 +388,12 @@ func (v *jobValidate) validateVaultIdentity(t *structs.Task) ([]error, error) { var mErr *multierror.Error var warnings []error - vaultWIDs := []string{} + vaultWIDs := make([]string, 0, len(t.Identities)) for _, wid := range t.Identities { if strings.HasPrefix(wid.Name, structs.WorkloadIdentityVaultPrefix) { vaultWIDs = append(vaultWIDs, wid.Name) } } - hasTaskWID := len(vaultWIDs) > 0 - hasDefaultWID := v.srv.config.VaultDefaultIdentity() != nil if t.Vault == nil { for _, wid := range vaultWIDs { @@ -404,6 +402,9 @@ func (v *jobValidate) validateVaultIdentity(t *structs.Task) ([]error, error) { return warnings, nil } + hasTaskWID := len(vaultWIDs) > 0 + hasDefaultWID := v.srv.config.VaultIdentityConfig(t.Vault.Cluster) != nil + if hasTaskWID || hasDefaultWID { if len(t.Vault.Policies) > 0 { warnings = append(warnings, fmt.Errorf( diff --git a/nomad/job_endpoint_hooks_test.go b/nomad/job_endpoint_hooks_test.go index 59e0b8d1d..78520a057 100644 --- a/nomad/job_endpoint_hooks_test.go +++ b/nomad/job_endpoint_hooks_test.go @@ -133,18 +133,22 @@ func Test_jobValidate_Validate_vault(t *testing.T) { name string inputTaskVault *structs.Vault inputTaskIdentities []*structs.WorkloadIdentity - inputConfig *config.VaultConfig + inputConfig map[string]*config.VaultConfig expectedWarns []string expectedErr string }{ { - name: "no error when vault identity is provided via config", - inputTaskVault: &structs.Vault{}, + name: "no error when vault identity is provided via config", + inputTaskVault: &structs.Vault{ + Cluster: structs.VaultDefaultCluster, + }, inputTaskIdentities: nil, - inputConfig: &config.VaultConfig{ - DefaultIdentity: &config.WorkloadIdentityConfig{ - Audience: []string{"vault.io"}, - TTL: pointer.Of(time.Hour), + inputConfig: map[string]*config.VaultConfig{ + structs.VaultDefaultCluster: { + DefaultIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"vault.io"}, + TTL: pointer.Of(time.Hour), + }, }, }, }, @@ -156,25 +160,26 @@ func Test_jobValidate_Validate_vault(t *testing.T) { Audience: []string{"vault.io"}, TTL: time.Hour, }}, - inputConfig: &config.VaultConfig{}, }, { name: "error when not using vault identity and vault block is missing policies", inputTaskVault: &structs.Vault{}, inputTaskIdentities: nil, - inputConfig: &config.VaultConfig{}, expectedErr: "Vault block with an empty list of policies", }, { name: "warn when using default vault identity but task has vault policies", inputTaskVault: &structs.Vault{ + Cluster: structs.VaultDefaultCluster, Policies: []string{"nomad-workload"}, }, inputTaskIdentities: nil, - inputConfig: &config.VaultConfig{ - DefaultIdentity: &config.WorkloadIdentityConfig{ - Audience: []string{"vault.io"}, - TTL: pointer.Of(time.Hour), + inputConfig: map[string]*config.VaultConfig{ + structs.VaultDefaultCluster: { + DefaultIdentity: &config.WorkloadIdentityConfig{ + Audience: []string{"vault.io"}, + TTL: pointer.Of(time.Hour), + }, }, }, expectedWarns: []string{"policies will be ignored"}, @@ -189,7 +194,6 @@ func Test_jobValidate_Validate_vault(t *testing.T) { Audience: []string{"vault.io"}, TTL: time.Hour, }}, - inputConfig: &config.VaultConfig{}, expectedWarns: []string{"policies will be ignored"}, }, { @@ -200,7 +204,6 @@ func Test_jobValidate_Validate_vault(t *testing.T) { Audience: []string{"vault.io"}, TTL: time.Hour, }}, - inputConfig: &config.VaultConfig{}, expectedWarns: []string{ "has an identity called vault_default but no vault block", }, @@ -212,7 +215,7 @@ func Test_jobValidate_Validate_vault(t *testing.T) { impl := jobValidate{srv: &Server{ config: &Config{ JobMaxPriority: 100, - VaultConfig: tc.inputConfig, + VaultConfigs: tc.inputConfig, }, }}