diff --git a/client/allocrunner/taskrunner/artifact_hook_test.go b/client/allocrunner/taskrunner/artifact_hook_test.go index 3e2ee1891a6..f65fc78b009 100644 --- a/client/allocrunner/taskrunner/artifact_hook_test.go +++ b/client/allocrunner/taskrunner/artifact_hook_test.go @@ -88,7 +88,7 @@ func TestTaskRunner_ArtifactHook_PartialDone(t *testing.T) { _, destdir := getter.SetupDir(t) req := &interfaces.TaskPrestartRequest{ - TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""), + TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, nil, destdir, ""), TaskDir: &allocdir.TaskDir{Dir: destdir}, Task: &structs.Task{ Artifacts: []*structs.TaskArtifact{ @@ -180,7 +180,7 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadSuccess(t *testing.T) { _, destdir := getter.SetupDir(t) req := &interfaces.TaskPrestartRequest{ - TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""), + TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, nil, destdir, ""), TaskDir: &allocdir.TaskDir{Dir: destdir}, Task: &structs.Task{ Artifacts: []*structs.TaskArtifact{ @@ -271,7 +271,7 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadFailure(t *testing.T) { _, destdir := getter.SetupDir(t) req := &interfaces.TaskPrestartRequest{ - TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""), + TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, nil, destdir, ""), TaskDir: &allocdir.TaskDir{Dir: destdir}, Task: &structs.Task{ Artifacts: []*structs.TaskArtifact{ diff --git a/client/allocrunner/taskrunner/envoy_version_hook_test.go b/client/allocrunner/taskrunner/envoy_version_hook_test.go index 871f65ded3a..2b0c8b49aff 100644 --- a/client/allocrunner/taskrunner/envoy_version_hook_test.go +++ b/client/allocrunner/taskrunner/envoy_version_hook_test.go @@ -26,7 +26,7 @@ var ( taskEnvDefault = taskenv.NewTaskEnv(nil, nil, nil, map[string]string{ "meta.connect.sidecar_image": envoy.ImageFormat, "meta.connect.gateway_image": envoy.ImageFormat, - }, "", "") + }, nil, "", "") ) func TestEnvoyVersionHook_semver(t *testing.T) { @@ -147,7 +147,7 @@ func TestEnvoyVersionHook_interpolateImage(t *testing.T) { "MY_ENVOY": "my/envoy", }, map[string]string{ "MY_ENVOY": "my/envoy", - }, nil, nil, "", "")) + }, nil, nil, nil, "", "")) must.Eq(t, "my/envoy", task.Config["image"]) }) diff --git a/client/allocrunner/taskrunner/secrets_hook.go b/client/allocrunner/taskrunner/secrets_hook.go index 3979c023cf0..565c88be65e 100644 --- a/client/allocrunner/taskrunner/secrets_hook.go +++ b/client/allocrunner/taskrunner/secrets_hook.go @@ -6,7 +6,6 @@ package taskrunner import ( "context" "fmt" - "maps" "path/filepath" log "github.com/hashicorp/go-hclog" @@ -73,9 +72,6 @@ type secretsHook struct { // secrets to be fetched and populated for interpolation secrets []*structs.Secret - - // taskrunner secrets map - taskSecrets map[string]string } func newSecretsHook(conf *secretsHookConfig, secrets []*structs.Secret) *secretsHook { @@ -87,9 +83,6 @@ func newSecretsHook(conf *secretsHookConfig, secrets []*structs.Secret) *secrets envBuilder: conf.envBuilder, nomadNamespace: conf.nomadNamespace, secrets: secrets, - // Future work will inject taskSecrets from the taskRunner, so that the taskrunner - // can make these secrets available to other hooks. - taskSecrets: make(map[string]string), } } @@ -146,13 +139,13 @@ func (h *secretsHook) Prestart(ctx context.Context, req *interfaces.TaskPrestart case <-unblock: } - // parse and copy variables to taskSecrets + // parse and copy variables to envBuilder secrets for _, p := range providers { vars, err := p.Parse() if err != nil { return err } - maps.Copy(h.taskSecrets, vars) + h.envBuilder.SetSecrets(vars) } return nil diff --git a/client/allocrunner/taskrunner/secrets_hook_test.go b/client/allocrunner/taskrunner/secrets_hook_test.go index 98ec9b3b505..1e8acb0d8f3 100644 --- a/client/allocrunner/taskrunner/secrets_hook_test.go +++ b/client/allocrunner/taskrunner/secrets_hook_test.go @@ -66,12 +66,13 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) { alloc := mock.MinAlloc() task := alloc.Job.TaskGroups[0].Tasks[0] + taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region) conf := &secretsHookConfig{ logger: testlog.HCLogger(t), lifecycle: trtesting.NewMockTaskHooks(), events: &trtesting.MockEmitter{}, clientConfig: clientConfig, - envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region), + envBuilder: taskEnv, } secretHook := newSecretsHook(conf, []*structs.Secret{ { @@ -100,7 +101,7 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) { "secret.test_secret.key1": "value1", "secret.test_secret.key2": "value2", } - must.Eq(t, expected, secretHook.taskSecrets) + must.Eq(t, expected, taskEnv.Build().TaskSecrets) }) t.Run("returns early if context is cancelled", func(t *testing.T) { @@ -140,13 +141,13 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) { alloc := mock.MinAlloc() task := alloc.Job.TaskGroups[0].Tasks[0] + taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region) conf := &secretsHookConfig{ - logger: testlog.HCLogger(t), lifecycle: trtesting.NewMockTaskHooks(), events: &trtesting.MockEmitter{}, clientConfig: clientConfig, - envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region), + envBuilder: taskEnv, } secretHook := newSecretsHook(conf, []*structs.Secret{ { @@ -172,7 +173,7 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) { must.NoError(t, err) expected := map[string]string{} - must.Eq(t, expected, secretHook.taskSecrets) + must.Eq(t, expected, taskEnv.Build().TaskSecrets) }) t.Run("errors when failure building secret providers", func(t *testing.T) { @@ -182,13 +183,13 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) { alloc := mock.MinAlloc() task := alloc.Job.TaskGroups[0].Tasks[0] + taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region) conf := &secretsHookConfig{ - logger: testlog.HCLogger(t), lifecycle: trtesting.NewMockTaskHooks(), events: &trtesting.MockEmitter{}, clientConfig: clientConfig, - envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region), + envBuilder: taskEnv, } // give an invalid secret, in this case a nomad secret with bad namespace @@ -214,7 +215,7 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) { must.Error(t, err) expected := map[string]string{} - must.Eq(t, expected, secretHook.taskSecrets) + must.Eq(t, expected, taskEnv.Build().TaskSecrets) }) } @@ -259,14 +260,13 @@ func TestSecretsHook_Prestart_Vault(t *testing.T) { alloc := mock.MinAlloc() task := alloc.Job.TaskGroups[0].Tasks[0] + taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region) conf := &secretsHookConfig{ - - // alloc: alloc, logger: testlog.HCLogger(t), lifecycle: trtesting.NewMockTaskHooks(), events: &trtesting.MockEmitter{}, clientConfig: clientConfig, - envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region), + envBuilder: taskEnv, } secretHook := newSecretsHook(conf, []*structs.Secret{ { @@ -296,5 +296,5 @@ func TestSecretsHook_Prestart_Vault(t *testing.T) { "secret.test_secret.secret": "secret", } - must.Eq(t, exp, secretHook.taskSecrets) + must.Eq(t, exp, taskEnv.Build().TaskSecrets) } diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go index 05188811daf..55ace68c6b5 100644 --- a/client/allocrunner/taskrunner/template/template_test.go +++ b/client/allocrunner/taskrunner/template/template_test.go @@ -1699,6 +1699,7 @@ func TestTaskTemplateManager_Env_InterpolatedDest(t *testing.T) { map[string]string{"NOMAD_META_path": "exists"}, map[string]string{}, map[string]string{}, + map[string]string{}, d, "") vars, err := loadTemplateEnv(templates, taskEnv) diff --git a/client/taskenv/env.go b/client/taskenv/env.go index 05cb284803c..55466c4d5ed 100644 --- a/client/taskenv/env.go +++ b/client/taskenv/env.go @@ -154,8 +154,8 @@ const ( nodeMetaPrefix = "meta." ) -// TaskEnv is a task's environment as well as node attribute's for -// interpolation. +// TaskEnv is a task's environment as well as node attribute's and +// task secrets for interpolation. type TaskEnv struct { // NodeAttrs is the map of node attributes for interpolation NodeAttrs map[string]string @@ -163,6 +163,9 @@ type TaskEnv struct { // EnvMap is the map of environment variables EnvMap map[string]string + // TaskSecrets is the map of secrets populated from the secrets hook + TaskSecrets map[string]string + // deviceEnv is the environment variables populated from the device hooks. deviceEnv map[string]string @@ -185,11 +188,12 @@ type TaskEnv struct { // NewTaskEnv creates a new task environment with the given environment, device // environment and node attribute maps. -func NewTaskEnv(env, envClient, deviceEnv, node map[string]string, clientTaskDir, clientAllocDir string) *TaskEnv { +func NewTaskEnv(env, envClient, deviceEnv, node map[string]string, secrets map[string]string, clientTaskDir, clientAllocDir string) *TaskEnv { return &TaskEnv{ NodeAttrs: node, deviceEnv: deviceEnv, EnvMap: env, + TaskSecrets: secrets, EnvMapClient: envClient, clientTaskDir: clientTaskDir, clientSharedAllocDir: clientAllocDir, @@ -311,6 +315,13 @@ func (t *TaskEnv) AllValues() (map[string]cty.Value, map[string]error, error) { } } + // Prepare task-based secrets for use in interpolation + for k, v := range t.TaskSecrets { + if err := addNestedKey(allMap, k, v); err != nil { + errs[k] = err + } + } + // Add flat envMap as a Map to allMap so users can access any key via // HCL2's indexing syntax: ${env["foo...bar"]} allMap["env"] = cty.MapVal(envMap) @@ -359,10 +370,10 @@ func (t *TaskEnv) ParseAndReplace(args []string) []string { } // ReplaceEnv takes an arg and replaces all occurrences of environment variables -// and Nomad variables. If the variable is found in the passed map it is -// replaced, otherwise the original string is returned. +// and Node attributes, and task secrets. If the variable is found in the passed map +// it is replaced, otherwise the original string is returned. func (t *TaskEnv) ReplaceEnv(arg string) string { - return hargs.ReplaceEnv(arg, t.EnvMap, t.NodeAttrs) + return hargs.ReplaceEnv(arg, t.EnvMap, t.NodeAttrs, t.TaskSecrets) } // replaceEnvClient takes an arg and replaces all occurrences of client-specific @@ -425,6 +436,9 @@ type Builder struct { // nodeAttrs are Node attributes and metadata nodeAttrs map[string]string + // taskSecrets are secrets populated from the secrets hook + taskSecrets map[string]string + // taskMeta are the meta attributes on the task taskMeta map[string]string @@ -516,9 +530,10 @@ func NewBuilder(node *structs.Node, alloc *structs.Allocation, task *structs.Tas // NewEmptyBuilder creates a new environment builder. func NewEmptyBuilder() *Builder { return &Builder{ - mu: &sync.RWMutex{}, - hookEnvs: map[string]map[string]string{}, - envvars: make(map[string]string), + mu: &sync.RWMutex{}, + hookEnvs: map[string]map[string]string{}, + envvars: make(map[string]string), + taskSecrets: make(map[string]string), } } @@ -649,7 +664,7 @@ func (b *Builder) buildEnv(allocDir, localDir, secretsDir string, // Copy interpolated task env vars second as they override host env vars for k, v := range b.envvars { - envMap[k] = hargs.ReplaceEnv(v, nodeAttrs, envMap) + envMap[k] = hargs.ReplaceEnv(v, nodeAttrs, envMap, b.taskSecrets) } // Copy hook env vars in the order the hooks were run @@ -709,7 +724,13 @@ func (b *Builder) Build() *TaskEnv { envMap, deviceEnvs := b.buildEnv(b.allocDir, b.localDir, b.secretsDir, nodeAttrs) envMapClient, _ := b.buildEnv(b.clientSharedAllocDir, b.clientTaskLocalDir, b.clientTaskSecretsDir, nodeAttrs) - return NewTaskEnv(envMap, envMapClient, deviceEnvs, nodeAttrs, b.clientTaskRoot, b.clientSharedAllocDir) + return NewTaskEnv(envMap, envMapClient, deviceEnvs, nodeAttrs, b.taskSecrets, b.clientTaskRoot, b.clientSharedAllocDir) +} + +func (b *Builder) SetSecrets(secrets map[string]string) { + b.mu.Lock() + defer b.mu.Unlock() + maps.Copy(b.taskSecrets, secrets) } // SetHookEnv sets environment variables from a hook. Variables are diff --git a/client/taskenv/env_test.go b/client/taskenv/env_test.go index 540be233c62..81c967dcfdc 100644 --- a/client/taskenv/env_test.go +++ b/client/taskenv/env_test.go @@ -39,6 +39,9 @@ const ( envOneVal = "127.0.0.1" envTwoKey = "NOMAD_PORT_WEB" envTwoVal = ":80" + + // Secrets populated from secrets hook + testSecret = "foo" ) var ( @@ -65,7 +68,13 @@ func testEnvBuilder() *Builder { envOneKey: envOneVal, envTwoKey: envTwoVal, } - return NewBuilder(n, mock.Alloc(), task, "global") + + b := NewBuilder(n, mock.Alloc(), task, "global") + b.taskSecrets = map[string]string{ + testSecret: testSecret, + } + + return b } func TestEnvironment_ParseAndReplace_Env(t *testing.T) { @@ -145,13 +154,13 @@ func TestEnvironment_ParseAndReplace_Mixed(t *testing.T) { func TestEnvironment_ReplaceEnv_Mixed(t *testing.T) { ci.Parallel(t) - input := fmt.Sprintf("${%v}${%v%v}", nodeNameKey, nodeAttributePrefix, attrKey) - exp := fmt.Sprintf("%v%v", nodeName, attrVal) + input := fmt.Sprintf("${%v}${%v%v}${%v}", nodeNameKey, nodeAttributePrefix, attrKey, testSecret) + exp := fmt.Sprintf("%v%v%v", nodeName, attrVal, testSecret) env := testEnvBuilder() act := env.Build().ReplaceEnv(input) if act != exp { - t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp) + t.Fatalf("ReplaceEnv(%v) returned %#v; want %#v", input, act, exp) } } @@ -364,6 +373,10 @@ func TestEnvironment_AllValues(t *testing.T) { } env.mu.Unlock() + env.taskSecrets = map[string]string{ + "secret.testsecret.test": "foo", + } + values, errs, err := env.Build().AllValues() require.NoError(t, err) @@ -397,6 +410,9 @@ func TestEnvironment_AllValues(t *testing.T) { "node.attr.kernel.name": "linux", "node.attr.nomad.version": "0.5.0", + // Task Secrets for interpreting task config + `secret.testsecret.test`: "foo", + // Env "taskEnvKey": "taskEnvVal", "NOMAD_ADDR_http": "127.0.0.1:80", diff --git a/client/taskenv/services_test.go b/client/taskenv/services_test.go index 2995e9ff8e2..0899161fe9b 100644 --- a/client/taskenv/services_test.go +++ b/client/taskenv/services_test.go @@ -122,7 +122,7 @@ func TestInterpolateServices(t *testing.T) { var testEnv = NewTaskEnv( map[string]string{"foo": "bar", "baz": "blah"}, map[string]string{"foo": "bar", "baz": "blah"}, - nil, nil, "", "") + nil, nil, nil, "", "") func TestInterpolate_interpolateMapStringSliceString(t *testing.T) { ci.Parallel(t) @@ -219,7 +219,7 @@ func TestInterpolate_interpolateConnect(t *testing.T) { "service1": "_service1", "host1": "_host1", } - env := NewTaskEnv(e, e, nil, nil, "", "") + env := NewTaskEnv(e, e, nil, nil, nil, "", "") connect := &structs.ConsulConnect{ Native: false,