Skip to content

task: adds ability to interpret values from secrets hook #26261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions client/allocrunner/taskrunner/artifact_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions client/allocrunner/taskrunner/envoy_version_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"])
})

Expand Down
11 changes: 2 additions & 9 deletions client/allocrunner/taskrunner/secrets_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package taskrunner
import (
"context"
"fmt"
"maps"
"path/filepath"

log "github.com/hashicorp/go-hclog"
Expand Down Expand Up @@ -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 {
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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
Expand Down
24 changes: 12 additions & 12 deletions client/allocrunner/taskrunner/secrets_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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{
{
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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)
}
1 change: 1 addition & 0 deletions client/allocrunner/taskrunner/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
43 changes: 32 additions & 11 deletions client/taskenv/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,18 @@ 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

// 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

Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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),
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
24 changes: 20 additions & 4 deletions client/taskenv/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ const (
envOneVal = "127.0.0.1"
envTwoKey = "NOMAD_PORT_WEB"
envTwoVal = ":80"

// Secrets populated from secrets hook
testSecret = "foo"
)

var (
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions client/taskenv/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
Loading