From 311a2bd38d38921e8e0afd97c435082be48e171b Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 11:26:46 +0100 Subject: [PATCH 01/10] task-level consul hook must be called for templates --- client/allocrunner/taskrunner/consul_hook.go | 4 ++-- client/allocrunner/taskrunner/task_runner_hooks.go | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/client/allocrunner/taskrunner/consul_hook.go b/client/allocrunner/taskrunner/consul_hook.go index 8ffac91cd95..ee09407e6a3 100644 --- a/client/allocrunner/taskrunner/consul_hook.go +++ b/client/allocrunner/taskrunner/consul_hook.go @@ -35,11 +35,11 @@ type consulHook struct { logger log.Logger } -func newConsulHook(logger log.Logger, tr *TaskRunner, hookResources *cstructs.AllocHookResources) *consulHook { +func newConsulHook(logger log.Logger, tr *TaskRunner) *consulHook { h := &consulHook{ task: tr.Task(), tokenDir: tr.taskDir.SecretsDir, - hookResources: hookResources, + hookResources: tr.allocHookResources, } h.logger = logger.Named(h.Name()) return h diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 9cf6009aaa6..844c9dec729 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -122,6 +122,8 @@ func (tr *TaskRunner) initHooks() { nomadNamespace: tr.alloc.Job.Namespace, renderOnTaskRestart: task.RestartPolicy.RenderTemplates, })) + + tr.runnerHooks = append(tr.runnerHooks, newConsulHook(hookLogger, tr)) } // Always add the service hook. A task with no services on initial registration From cbaa44cba3a4cb5e2b2fc66f71eeb2b87dbd8d16 Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 11:27:16 +0100 Subject: [PATCH 02/10] docs: change documentation about Consul token file names --- website/content/docs/job-specification/consul.mdx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/website/content/docs/job-specification/consul.mdx b/website/content/docs/job-specification/consul.mdx index 06ea169fbe5..e24301df6fb 100644 --- a/website/content/docs/job-specification/consul.mdx +++ b/website/content/docs/job-specification/consul.mdx @@ -74,11 +74,12 @@ to Using Workload Identity with Consul ### Access to Token The Nomad client will make the Consul token available to the task by writing it -to the secret directory at `secrets/consul_token` and by injecting a -`CONSUL_TOKEN` environment variable in the task. If the Nomad cluster is +to the secret directory at +`secrets/nomad_consul_{$consul_cluster}_{$task_identity_name}` and by injecting +a `CONSUL_TOKEN` environment variable in the task. If the Nomad cluster is [configured][config_consul_namespace] to use [Consul Namespaces][], a -`CONSUL_NAMESPACE` environment variable will be injected whenever `CONSUL_TOKEN` -is set. +`CONSUL_NAMESPACE` environment variable will be injected whenever +`CONSUL_TOKEN` is set. The [`template`][template] block can use the Consul token as well. From ebd5ade139271d62e9d2d5eceeb9f2a472fa68fb Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 14:44:23 +0100 Subject: [PATCH 03/10] set env --- client/allocrunner/taskrunner/consul_hook.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/allocrunner/taskrunner/consul_hook.go b/client/allocrunner/taskrunner/consul_hook.go index ee09407e6a3..218d63cab2a 100644 --- a/client/allocrunner/taskrunner/consul_hook.go +++ b/client/allocrunner/taskrunner/consul_hook.go @@ -32,6 +32,7 @@ type consulHook struct { task *structs.Task tokenDir string hookResources *cstructs.AllocHookResources + taskEnv map[string]string logger log.Logger } @@ -41,6 +42,7 @@ func newConsulHook(logger log.Logger, tr *TaskRunner) *consulHook { tokenDir: tr.taskDir.SecretsDir, hookResources: tr.allocHookResources, } + h.taskEnv = tr.envBuilder.Build().Map() h.logger = logger.Named(h.Name()) return h } @@ -71,6 +73,8 @@ func (h *consulHook) Prestart(context.Context, *interfaces.TaskPrestartRequest, if err := os.WriteFile(tokenPath, []byte(token.SecretID), consulTokenFilePerms); err != nil { mErr.Errors = append(mErr.Errors, fmt.Errorf("failed to write Consul SI token: %w", err)) } + + h.taskEnv["CONSUL_TOKEN"] = token.SecretID } } From 18cd6919cb53de0abd8b98aa6da42bbd94da6878 Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 14:51:51 +0100 Subject: [PATCH 04/10] changelog --- .changelog/19490.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/19490.txt diff --git a/.changelog/19490.txt b/.changelog/19490.txt new file mode 100644 index 00000000000..a94303e6300 --- /dev/null +++ b/.changelog/19490.txt @@ -0,0 +1,3 @@ +```release-note:bug +client: fix a bug where task level Consul hook never gets called +``` From cb19ea61b6980294d125f7fe5f20cdaa45343a9a Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 15:08:17 +0100 Subject: [PATCH 05/10] revert documentation changes --- website/content/docs/job-specification/consul.mdx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/website/content/docs/job-specification/consul.mdx b/website/content/docs/job-specification/consul.mdx index e24301df6fb..06ea169fbe5 100644 --- a/website/content/docs/job-specification/consul.mdx +++ b/website/content/docs/job-specification/consul.mdx @@ -74,12 +74,11 @@ to Using Workload Identity with Consul ### Access to Token The Nomad client will make the Consul token available to the task by writing it -to the secret directory at -`secrets/nomad_consul_{$consul_cluster}_{$task_identity_name}` and by injecting -a `CONSUL_TOKEN` environment variable in the task. If the Nomad cluster is +to the secret directory at `secrets/consul_token` and by injecting a +`CONSUL_TOKEN` environment variable in the task. If the Nomad cluster is [configured][config_consul_namespace] to use [Consul Namespaces][], a -`CONSUL_NAMESPACE` environment variable will be injected whenever -`CONSUL_TOKEN` is set. +`CONSUL_NAMESPACE` environment variable will be injected whenever `CONSUL_TOKEN` +is set. The [`template`][template] block can use the Consul token as well. From ac4a05a709f755734e82335f32feb1164793a9aa Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 15:15:21 +0100 Subject: [PATCH 06/10] tasks can only ever have 1 token --- client/allocrunner/taskrunner/consul_hook.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/client/allocrunner/taskrunner/consul_hook.go b/client/allocrunner/taskrunner/consul_hook.go index 218d63cab2a..86dd59dc302 100644 --- a/client/allocrunner/taskrunner/consul_hook.go +++ b/client/allocrunner/taskrunner/consul_hook.go @@ -18,10 +18,9 @@ import ( ) const ( - // consulTokenFilePrefix is the begging of the name of the file holding the - // Consul SI token inside the task's secret directory. Full name of the file is - // always consulTokenFilePrefix_identityName - consulTokenFilePrefix = "nomad_consul" + // consulTokenFilename is the name of the file holding the Consul SI token + // inside the task's secret directory. + consulTokenFilename = "consul_token" // consulTokenFilePerms is the level of file permissions granted on the file in // the secrets directory for the task @@ -57,7 +56,7 @@ func (h *consulHook) Prestart(context.Context, *interfaces.TaskPrestartRequest, tokens := h.hookResources.GetConsulTokens() // Write tokens to tasks' secret dirs - for cluster, t := range tokens { + for _, t := range tokens { for identity, token := range t { // do not write tokens that do not belong to any of this task's // identities @@ -68,8 +67,7 @@ func (h *consulHook) Prestart(context.Context, *interfaces.TaskPrestartRequest, continue } - filename := fmt.Sprintf("%s_%s_%s", consulTokenFilePrefix, cluster, identity) - tokenPath := filepath.Join(h.tokenDir, filename) + tokenPath := filepath.Join(h.tokenDir, consulTokenFilename) if err := os.WriteFile(tokenPath, []byte(token.SecretID), consulTokenFilePerms); err != nil { mErr.Errors = append(mErr.Errors, fmt.Errorf("failed to write Consul SI token: %w", err)) } From 7b859650409e41f393da325becaa685cbe7f339b Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 16:03:06 +0100 Subject: [PATCH 07/10] publish CONSUL_TOKEN in the prestart response --- client/allocrunner/taskrunner/consul_hook.go | 21 ++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/client/allocrunner/taskrunner/consul_hook.go b/client/allocrunner/taskrunner/consul_hook.go index 86dd59dc302..855f1b353dd 100644 --- a/client/allocrunner/taskrunner/consul_hook.go +++ b/client/allocrunner/taskrunner/consul_hook.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/client/allocrunner/interfaces" cstructs "github.com/hashicorp/nomad/client/structs" + "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/nomad/structs" ) @@ -28,11 +29,12 @@ const ( ) type consulHook struct { - task *structs.Task - tokenDir string - hookResources *cstructs.AllocHookResources - taskEnv map[string]string - logger log.Logger + task *structs.Task + tokenDir string + hookResources *cstructs.AllocHookResources + taskEnvBuilder *taskenv.Builder + + logger log.Logger } func newConsulHook(logger log.Logger, tr *TaskRunner) *consulHook { @@ -41,7 +43,7 @@ func newConsulHook(logger log.Logger, tr *TaskRunner) *consulHook { tokenDir: tr.taskDir.SecretsDir, hookResources: tr.allocHookResources, } - h.taskEnv = tr.envBuilder.Build().Map() + h.taskEnvBuilder = tr.envBuilder h.logger = logger.Named(h.Name()) return h } @@ -50,7 +52,7 @@ func (*consulHook) Name() string { return "consul_task" } -func (h *consulHook) Prestart(context.Context, *interfaces.TaskPrestartRequest, *interfaces.TaskPrestartResponse) error { +func (h *consulHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRequest, resp *interfaces.TaskPrestartResponse) error { mErr := multierror.Error{} tokens := h.hookResources.GetConsulTokens() @@ -72,7 +74,10 @@ func (h *consulHook) Prestart(context.Context, *interfaces.TaskPrestartRequest, mErr.Errors = append(mErr.Errors, fmt.Errorf("failed to write Consul SI token: %w", err)) } - h.taskEnv["CONSUL_TOKEN"] = token.SecretID + envs := h.taskEnvBuilder.Build().Map() + envs["CONSUL_TOKEN"] = token.SecretID + + resp.Env = envs } } From ad83aff166ef2be0b4ceb617450fa4415280471f Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 16:35:21 +0100 Subject: [PATCH 08/10] no need for a taskenv.Builder afterall, and reshuffled the hook order --- client/allocrunner/taskrunner/consul_hook.go | 16 +++++++--------- .../allocrunner/taskrunner/task_runner_hooks.go | 4 ++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/client/allocrunner/taskrunner/consul_hook.go b/client/allocrunner/taskrunner/consul_hook.go index 855f1b353dd..e34929573b8 100644 --- a/client/allocrunner/taskrunner/consul_hook.go +++ b/client/allocrunner/taskrunner/consul_hook.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/client/allocrunner/interfaces" cstructs "github.com/hashicorp/nomad/client/structs" - "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/nomad/structs" ) @@ -29,10 +28,9 @@ const ( ) type consulHook struct { - task *structs.Task - tokenDir string - hookResources *cstructs.AllocHookResources - taskEnvBuilder *taskenv.Builder + task *structs.Task + tokenDir string + hookResources *cstructs.AllocHookResources logger log.Logger } @@ -43,7 +41,6 @@ func newConsulHook(logger log.Logger, tr *TaskRunner) *consulHook { tokenDir: tr.taskDir.SecretsDir, hookResources: tr.allocHookResources, } - h.taskEnvBuilder = tr.envBuilder h.logger = logger.Named(h.Name()) return h } @@ -74,10 +71,11 @@ func (h *consulHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartR mErr.Errors = append(mErr.Errors, fmt.Errorf("failed to write Consul SI token: %w", err)) } - envs := h.taskEnvBuilder.Build().Map() - envs["CONSUL_TOKEN"] = token.SecretID + env := map[string]string{ + "CONSUL_TOKEN": token.SecretID, + } - resp.Env = envs + resp.Env = env } } diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 844c9dec729..2f07a92f290 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -109,6 +109,8 @@ func (tr *TaskRunner) initHooks() { // If there are templates is enabled, add the hook if len(task.Templates) != 0 { + tr.runnerHooks = append(tr.runnerHooks, newConsulHook(hookLogger, tr)) + tr.runnerHooks = append(tr.runnerHooks, newTemplateHook(&templateHookConfig{ alloc: tr.Alloc(), logger: hookLogger, @@ -122,8 +124,6 @@ func (tr *TaskRunner) initHooks() { nomadNamespace: tr.alloc.Job.Namespace, renderOnTaskRestart: task.RestartPolicy.RenderTemplates, })) - - tr.runnerHooks = append(tr.runnerHooks, newConsulHook(hookLogger, tr)) } // Always add the service hook. A task with no services on initial registration From 2fbde1e9d0af0812fdaca059ca4d417c60de1757 Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 16:40:16 +0100 Subject: [PATCH 09/10] rephrase changelog entry --- .changelog/19490.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/19490.txt b/.changelog/19490.txt index a94303e6300..c2d5e988de5 100644 --- a/.changelog/19490.txt +++ b/.changelog/19490.txt @@ -1,3 +1,3 @@ ```release-note:bug -client: fix a bug where task level Consul hook never gets called +client: Fixed a bug where where the environment variable / file for the Consul token weren't written. ``` From 805c1f82feb4731bae281b16ce423eede5004e73 Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 15 Dec 2023 16:57:24 +0100 Subject: [PATCH 10/10] adjust the taskrunner hooks --- client/allocrunner/taskrunner/task_runner_hooks.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 2f07a92f290..5282f66978f 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -107,10 +107,12 @@ func (tr *TaskRunner) initHooks() { // Get the consul namespace for the TG of the allocation. consulNamespace := tr.alloc.ConsulNamespaceForTask(tr.taskName) + // Add the consul hook (populates task secret dirs and sets the environment if + // consul tokens are present for the task). + tr.runnerHooks = append(tr.runnerHooks, newConsulHook(hookLogger, tr)) + // If there are templates is enabled, add the hook if len(task.Templates) != 0 { - tr.runnerHooks = append(tr.runnerHooks, newConsulHook(hookLogger, tr)) - tr.runnerHooks = append(tr.runnerHooks, newTemplateHook(&templateHookConfig{ alloc: tr.Alloc(), logger: hookLogger,