Skip to content

Commit f5920c5

Browse files
mismithhislermismithhisler
authored
task: adds ability to interpret values from secrets hook (#26261)
1 parent 2a8cde4 commit f5920c5

File tree

8 files changed

+74
-43
lines changed

8 files changed

+74
-43
lines changed

‎client/allocrunner/taskrunner/artifact_hook_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func TestTaskRunner_ArtifactHook_PartialDone(t *testing.T) {
8888
_, destdir := getter.SetupDir(t)
8989

9090
req := &interfaces.TaskPrestartRequest{
91-
TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""),
91+
TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, nil, destdir, ""),
9292
TaskDir: &allocdir.TaskDir{Dir: destdir},
9393
Task: &structs.Task{
9494
Artifacts: []*structs.TaskArtifact{
@@ -180,7 +180,7 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadSuccess(t *testing.T) {
180180
_, destdir := getter.SetupDir(t)
181181

182182
req := &interfaces.TaskPrestartRequest{
183-
TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""),
183+
TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, nil, destdir, ""),
184184
TaskDir: &allocdir.TaskDir{Dir: destdir},
185185
Task: &structs.Task{
186186
Artifacts: []*structs.TaskArtifact{
@@ -271,7 +271,7 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadFailure(t *testing.T) {
271271
_, destdir := getter.SetupDir(t)
272272

273273
req := &interfaces.TaskPrestartRequest{
274-
TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""),
274+
TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, nil, destdir, ""),
275275
TaskDir: &allocdir.TaskDir{Dir: destdir},
276276
Task: &structs.Task{
277277
Artifacts: []*structs.TaskArtifact{

‎client/allocrunner/taskrunner/envoy_version_hook_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ var (
2626
taskEnvDefault = taskenv.NewTaskEnv(nil, nil, nil, map[string]string{
2727
"meta.connect.sidecar_image": envoy.ImageFormat,
2828
"meta.connect.gateway_image": envoy.ImageFormat,
29-
}, "", "")
29+
}, nil, "", "")
3030
)
3131

3232
func TestEnvoyVersionHook_semver(t *testing.T) {
@@ -147,7 +147,7 @@ func TestEnvoyVersionHook_interpolateImage(t *testing.T) {
147147
"MY_ENVOY": "my/envoy",
148148
}, map[string]string{
149149
"MY_ENVOY": "my/envoy",
150-
}, nil, nil, "", ""))
150+
}, nil, nil, nil, "", ""))
151151
must.Eq(t, "my/envoy", task.Config["image"])
152152
})
153153

‎client/allocrunner/taskrunner/secrets_hook.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package taskrunner
66
import (
77
"context"
88
"fmt"
9-
"maps"
109
"path/filepath"
1110

1211
log "github.com/hashicorp/go-hclog"
@@ -73,9 +72,6 @@ type secretsHook struct {
7372

7473
// secrets to be fetched and populated for interpolation
7574
secrets []*structs.Secret
76-
77-
// taskrunner secrets map
78-
taskSecrets map[string]string
7975
}
8076

8177
func newSecretsHook(conf *secretsHookConfig, secrets []*structs.Secret) *secretsHook {
@@ -87,9 +83,6 @@ func newSecretsHook(conf *secretsHookConfig, secrets []*structs.Secret) *secrets
8783
envBuilder: conf.envBuilder,
8884
nomadNamespace: conf.nomadNamespace,
8985
secrets: secrets,
90-
// Future work will inject taskSecrets from the taskRunner, so that the taskrunner
91-
// can make these secrets available to other hooks.
92-
taskSecrets: make(map[string]string),
9386
}
9487
}
9588

@@ -146,13 +139,13 @@ func (h *secretsHook) Prestart(ctx context.Context, req *interfaces.TaskPrestart
146139
case <-unblock:
147140
}
148141

149-
// parse and copy variables to taskSecrets
142+
// parse and copy variables to envBuilder secrets
150143
for _, p := range providers {
151144
vars, err := p.Parse()
152145
if err != nil {
153146
return err
154147
}
155-
maps.Copy(h.taskSecrets, vars)
148+
h.envBuilder.SetSecrets(vars)
156149
}
157150

158151
return nil

‎client/allocrunner/taskrunner/secrets_hook_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,13 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) {
6666
alloc := mock.MinAlloc()
6767
task := alloc.Job.TaskGroups[0].Tasks[0]
6868

69+
taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region)
6970
conf := &secretsHookConfig{
7071
logger: testlog.HCLogger(t),
7172
lifecycle: trtesting.NewMockTaskHooks(),
7273
events: &trtesting.MockEmitter{},
7374
clientConfig: clientConfig,
74-
envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region),
75+
envBuilder: taskEnv,
7576
}
7677
secretHook := newSecretsHook(conf, []*structs.Secret{
7778
{
@@ -100,7 +101,7 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) {
100101
"secret.test_secret.key1": "value1",
101102
"secret.test_secret.key2": "value2",
102103
}
103-
must.Eq(t, expected, secretHook.taskSecrets)
104+
must.Eq(t, expected, taskEnv.Build().TaskSecrets)
104105
})
105106

106107
t.Run("returns early if context is cancelled", func(t *testing.T) {
@@ -140,13 +141,13 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) {
140141
alloc := mock.MinAlloc()
141142
task := alloc.Job.TaskGroups[0].Tasks[0]
142143

144+
taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region)
143145
conf := &secretsHookConfig{
144-
145146
logger: testlog.HCLogger(t),
146147
lifecycle: trtesting.NewMockTaskHooks(),
147148
events: &trtesting.MockEmitter{},
148149
clientConfig: clientConfig,
149-
envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region),
150+
envBuilder: taskEnv,
150151
}
151152
secretHook := newSecretsHook(conf, []*structs.Secret{
152153
{
@@ -172,7 +173,7 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) {
172173
must.NoError(t, err)
173174

174175
expected := map[string]string{}
175-
must.Eq(t, expected, secretHook.taskSecrets)
176+
must.Eq(t, expected, taskEnv.Build().TaskSecrets)
176177
})
177178

178179
t.Run("errors when failure building secret providers", func(t *testing.T) {
@@ -182,13 +183,13 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) {
182183
alloc := mock.MinAlloc()
183184
task := alloc.Job.TaskGroups[0].Tasks[0]
184185

186+
taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region)
185187
conf := &secretsHookConfig{
186-
187188
logger: testlog.HCLogger(t),
188189
lifecycle: trtesting.NewMockTaskHooks(),
189190
events: &trtesting.MockEmitter{},
190191
clientConfig: clientConfig,
191-
envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region),
192+
envBuilder: taskEnv,
192193
}
193194

194195
// give an invalid secret, in this case a nomad secret with bad namespace
@@ -214,7 +215,7 @@ func TestSecretsHook_Prestart_Nomad(t *testing.T) {
214215
must.Error(t, err)
215216

216217
expected := map[string]string{}
217-
must.Eq(t, expected, secretHook.taskSecrets)
218+
must.Eq(t, expected, taskEnv.Build().TaskSecrets)
218219
})
219220
}
220221

@@ -259,14 +260,13 @@ func TestSecretsHook_Prestart_Vault(t *testing.T) {
259260
alloc := mock.MinAlloc()
260261
task := alloc.Job.TaskGroups[0].Tasks[0]
261262

263+
taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region)
262264
conf := &secretsHookConfig{
263-
264-
// alloc: alloc,
265265
logger: testlog.HCLogger(t),
266266
lifecycle: trtesting.NewMockTaskHooks(),
267267
events: &trtesting.MockEmitter{},
268268
clientConfig: clientConfig,
269-
envBuilder: taskenv.NewBuilder(mock.Node(), alloc, task, clientConfig.Region),
269+
envBuilder: taskEnv,
270270
}
271271
secretHook := newSecretsHook(conf, []*structs.Secret{
272272
{
@@ -296,5 +296,5 @@ func TestSecretsHook_Prestart_Vault(t *testing.T) {
296296
"secret.test_secret.secret": "secret",
297297
}
298298

299-
must.Eq(t, exp, secretHook.taskSecrets)
299+
must.Eq(t, exp, taskEnv.Build().TaskSecrets)
300300
}

‎client/allocrunner/taskrunner/template/template_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1699,6 +1699,7 @@ func TestTaskTemplateManager_Env_InterpolatedDest(t *testing.T) {
16991699
map[string]string{"NOMAD_META_path": "exists"},
17001700
map[string]string{},
17011701
map[string]string{},
1702+
map[string]string{},
17021703
d, "")
17031704

17041705
vars, err := loadTemplateEnv(templates, taskEnv)

‎client/taskenv/env.go

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -154,15 +154,18 @@ const (
154154
nodeMetaPrefix = "meta."
155155
)
156156

157-
// TaskEnv is a task's environment as well as node attribute's for
158-
// interpolation.
157+
// TaskEnv is a task's environment as well as node attribute's and
158+
// task secrets for interpolation.
159159
type TaskEnv struct {
160160
// NodeAttrs is the map of node attributes for interpolation
161161
NodeAttrs map[string]string
162162

163163
// EnvMap is the map of environment variables
164164
EnvMap map[string]string
165165

166+
// TaskSecrets is the map of secrets populated from the secrets hook
167+
TaskSecrets map[string]string
168+
166169
// deviceEnv is the environment variables populated from the device hooks.
167170
deviceEnv map[string]string
168171

@@ -185,11 +188,12 @@ type TaskEnv struct {
185188

186189
// NewTaskEnv creates a new task environment with the given environment, device
187190
// environment and node attribute maps.
188-
func NewTaskEnv(env, envClient, deviceEnv, node map[string]string, clientTaskDir, clientAllocDir string) *TaskEnv {
191+
func NewTaskEnv(env, envClient, deviceEnv, node map[string]string, secrets map[string]string, clientTaskDir, clientAllocDir string) *TaskEnv {
189192
return &TaskEnv{
190193
NodeAttrs: node,
191194
deviceEnv: deviceEnv,
192195
EnvMap: env,
196+
TaskSecrets: secrets,
193197
EnvMapClient: envClient,
194198
clientTaskDir: clientTaskDir,
195199
clientSharedAllocDir: clientAllocDir,
@@ -311,6 +315,13 @@ func (t *TaskEnv) AllValues() (map[string]cty.Value, map[string]error, error) {
311315
}
312316
}
313317

318+
// Prepare task-based secrets for use in interpolation
319+
for k, v := range t.TaskSecrets {
320+
if err := addNestedKey(allMap, k, v); err != nil {
321+
errs[k] = err
322+
}
323+
}
324+
314325
// Add flat envMap as a Map to allMap so users can access any key via
315326
// HCL2's indexing syntax: ${env["foo...bar"]}
316327
allMap["env"] = cty.MapVal(envMap)
@@ -359,10 +370,10 @@ func (t *TaskEnv) ParseAndReplace(args []string) []string {
359370
}
360371

361372
// ReplaceEnv takes an arg and replaces all occurrences of environment variables
362-
// and Nomad variables. If the variable is found in the passed map it is
363-
// replaced, otherwise the original string is returned.
373+
// and Node attributes, and task secrets. If the variable is found in the passed map
374+
// it is replaced, otherwise the original string is returned.
364375
func (t *TaskEnv) ReplaceEnv(arg string) string {
365-
return hargs.ReplaceEnv(arg, t.EnvMap, t.NodeAttrs)
376+
return hargs.ReplaceEnv(arg, t.EnvMap, t.NodeAttrs, t.TaskSecrets)
366377
}
367378

368379
// replaceEnvClient takes an arg and replaces all occurrences of client-specific
@@ -425,6 +436,9 @@ type Builder struct {
425436
// nodeAttrs are Node attributes and metadata
426437
nodeAttrs map[string]string
427438

439+
// taskSecrets are secrets populated from the secrets hook
440+
taskSecrets map[string]string
441+
428442
// taskMeta are the meta attributes on the task
429443
taskMeta map[string]string
430444

@@ -516,9 +530,10 @@ func NewBuilder(node *structs.Node, alloc *structs.Allocation, task *structs.Tas
516530
// NewEmptyBuilder creates a new environment builder.
517531
func NewEmptyBuilder() *Builder {
518532
return &Builder{
519-
mu: &sync.RWMutex{},
520-
hookEnvs: map[string]map[string]string{},
521-
envvars: make(map[string]string),
533+
mu: &sync.RWMutex{},
534+
hookEnvs: map[string]map[string]string{},
535+
envvars: make(map[string]string),
536+
taskSecrets: make(map[string]string),
522537
}
523538
}
524539

@@ -649,7 +664,7 @@ func (b *Builder) buildEnv(allocDir, localDir, secretsDir string,
649664

650665
// Copy interpolated task env vars second as they override host env vars
651666
for k, v := range b.envvars {
652-
envMap[k] = hargs.ReplaceEnv(v, nodeAttrs, envMap)
667+
envMap[k] = hargs.ReplaceEnv(v, nodeAttrs, envMap, b.taskSecrets)
653668
}
654669

655670
// Copy hook env vars in the order the hooks were run
@@ -709,7 +724,13 @@ func (b *Builder) Build() *TaskEnv {
709724
envMap, deviceEnvs := b.buildEnv(b.allocDir, b.localDir, b.secretsDir, nodeAttrs)
710725
envMapClient, _ := b.buildEnv(b.clientSharedAllocDir, b.clientTaskLocalDir, b.clientTaskSecretsDir, nodeAttrs)
711726

712-
return NewTaskEnv(envMap, envMapClient, deviceEnvs, nodeAttrs, b.clientTaskRoot, b.clientSharedAllocDir)
727+
return NewTaskEnv(envMap, envMapClient, deviceEnvs, nodeAttrs, b.taskSecrets, b.clientTaskRoot, b.clientSharedAllocDir)
728+
}
729+
730+
func (b *Builder) SetSecrets(secrets map[string]string) {
731+
b.mu.Lock()
732+
defer b.mu.Unlock()
733+
maps.Copy(b.taskSecrets, secrets)
713734
}
714735

715736
// SetHookEnv sets environment variables from a hook. Variables are

‎client/taskenv/env_test.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ const (
3939
envOneVal = "127.0.0.1"
4040
envTwoKey = "NOMAD_PORT_WEB"
4141
envTwoVal = ":80"
42+
43+
// Secrets populated from secrets hook
44+
testSecret = "foo"
4245
)
4346

4447
var (
@@ -65,7 +68,13 @@ func testEnvBuilder() *Builder {
6568
envOneKey: envOneVal,
6669
envTwoKey: envTwoVal,
6770
}
68-
return NewBuilder(n, mock.Alloc(), task, "global")
71+
72+
b := NewBuilder(n, mock.Alloc(), task, "global")
73+
b.taskSecrets = map[string]string{
74+
testSecret: testSecret,
75+
}
76+
77+
return b
6978
}
7079

7180
func TestEnvironment_ParseAndReplace_Env(t *testing.T) {
@@ -145,13 +154,13 @@ func TestEnvironment_ParseAndReplace_Mixed(t *testing.T) {
145154
func TestEnvironment_ReplaceEnv_Mixed(t *testing.T) {
146155
ci.Parallel(t)
147156

148-
input := fmt.Sprintf("${%v}${%v%v}", nodeNameKey, nodeAttributePrefix, attrKey)
149-
exp := fmt.Sprintf("%v%v", nodeName, attrVal)
157+
input := fmt.Sprintf("${%v}${%v%v}${%v}", nodeNameKey, nodeAttributePrefix, attrKey, testSecret)
158+
exp := fmt.Sprintf("%v%v%v", nodeName, attrVal, testSecret)
150159
env := testEnvBuilder()
151160
act := env.Build().ReplaceEnv(input)
152161

153162
if act != exp {
154-
t.Fatalf("ParseAndReplace(%v) returned %#v; want %#v", input, act, exp)
163+
t.Fatalf("ReplaceEnv(%v) returned %#v; want %#v", input, act, exp)
155164
}
156165
}
157166

@@ -364,6 +373,10 @@ func TestEnvironment_AllValues(t *testing.T) {
364373
}
365374
env.mu.Unlock()
366375

376+
env.taskSecrets = map[string]string{
377+
"secret.testsecret.test": "foo",
378+
}
379+
367380
values, errs, err := env.Build().AllValues()
368381
require.NoError(t, err)
369382

@@ -397,6 +410,9 @@ func TestEnvironment_AllValues(t *testing.T) {
397410
"node.attr.kernel.name": "linux",
398411
"node.attr.nomad.version": "0.5.0",
399412

413+
// Task Secrets for interpreting task config
414+
`secret.testsecret.test`: "foo",
415+
400416
// Env
401417
"taskEnvKey": "taskEnvVal",
402418
"NOMAD_ADDR_http": "127.0.0.1:80",

‎client/taskenv/services_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func TestInterpolateServices(t *testing.T) {
122122
var testEnv = NewTaskEnv(
123123
map[string]string{"foo": "bar", "baz": "blah"},
124124
map[string]string{"foo": "bar", "baz": "blah"},
125-
nil, nil, "", "")
125+
nil, nil, nil, "", "")
126126

127127
func TestInterpolate_interpolateMapStringSliceString(t *testing.T) {
128128
ci.Parallel(t)
@@ -219,7 +219,7 @@ func TestInterpolate_interpolateConnect(t *testing.T) {
219219
"service1": "_service1",
220220
"host1": "_host1",
221221
}
222-
env := NewTaskEnv(e, e, nil, nil, "", "")
222+
env := NewTaskEnv(e, e, nil, nil, nil, "", "")
223223

224224
connect := &structs.ConsulConnect{
225225
Native: false,

0 commit comments

Comments
 (0)