From 6b6aa7cc260109c4efff0fa12954094130f39276 Mon Sep 17 00:00:00 2001 From: Michael Smithhisler Date: Mon, 23 Sep 2024 10:27:00 -0400 Subject: [PATCH] identity: adds ability to specify custom filepath for saving workload identities (#24038) --- api/tasks.go | 1 + .../allocrunner/taskrunner/identity_hook.go | 15 +++-- .../taskrunner/identity_hook_test.go | 60 +++++++++++++------ command/agent/job_endpoint.go | 1 + e2e/workload_id/input/identity.nomad | 21 +++++++ e2e/workload_id/workload_id_test.go | 9 +++ nomad/config.go | 3 + nomad/structs/config/workload_id.go | 12 +++- nomad/structs/config/workload_id_test.go | 35 +++++++++++ nomad/structs/structs.go | 9 ++- nomad/structs/workload_id.go | 15 ++++- nomad/structs/workload_id_test.go | 14 +++++ .../docs/job-specification/identity.mdx | 5 ++ 13 files changed, 176 insertions(+), 24 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index cc9d14331..d1a9ee53c 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -1246,6 +1246,7 @@ type WorkloadIdentity struct { ChangeSignal string `mapstructure:"change_signal" hcl:"change_signal,optional"` Env bool `hcl:"env,optional"` File bool `hcl:"file,optional"` + Filepath string `hcl:"filepath,optional"` ServiceName string `hcl:"service_name,optional"` TTL time.Duration `mapstructure:"ttl" hcl:"ttl,optional"` } diff --git a/client/allocrunner/taskrunner/identity_hook.go b/client/allocrunner/taskrunner/identity_hook.go index 6b4eb1dd5..32b4d7bd0 100644 --- a/client/allocrunner/taskrunner/identity_hook.go +++ b/client/allocrunner/taskrunner/identity_hook.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/consul-template/signals" log "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/allocrunner/interfaces" ti "github.com/hashicorp/nomad/client/allocrunner/taskrunner/interfaces" "github.com/hashicorp/nomad/client/taskenv" @@ -38,7 +39,7 @@ type tokenSetter interface { type identityHook struct { alloc *structs.Allocation task *structs.Task - tokenDir string + taskDir *allocdir.TaskDir envBuilder *taskenv.Builder lifecycle ti.TaskLifecycle ts tokenSetter @@ -54,7 +55,7 @@ func newIdentityHook(tr *TaskRunner, logger log.Logger) *identityHook { h := &identityHook{ alloc: tr.Alloc(), task: tr.Task(), - tokenDir: tr.taskDir.SecretsDir, + taskDir: tr.taskDir, envBuilder: tr.envBuilder, lifecycle: tr, ts: tr, @@ -216,7 +217,10 @@ func (h *identityHook) setDefaultToken() error { // Handle file writing if id := h.task.Identity; id != nil && id.File { // Write token as owner readable only - tokenPath := filepath.Join(h.tokenDir, wiTokenFile) + tokenPath := filepath.Join(h.taskDir.SecretsDir, wiTokenFile) + if id.Filepath != "" { + tokenPath = filepath.Join(h.taskDir.Dir, id.Filepath) + } if err := users.WriteFileFor(tokenPath, []byte(token), h.task.User); err != nil { return fmt.Errorf("failed to write nomad token: %w", err) } @@ -233,7 +237,10 @@ func (h *identityHook) setAltToken(widspec *structs.WorkloadIdentity, rawJWT str } if widspec.File { - tokenPath := filepath.Join(h.tokenDir, fmt.Sprintf("nomad_%s.jwt", widspec.Name)) + tokenPath := filepath.Join(h.taskDir.SecretsDir, fmt.Sprintf("nomad_%s.jwt", widspec.Name)) + if widspec.Filepath != "" { + tokenPath = filepath.Join(h.taskDir.Dir, widspec.Filepath) + } if err := users.WriteFileFor(tokenPath, []byte(rawJWT), h.task.User); err != nil { return fmt.Errorf("failed to write token for identity %q: %w", widspec.Name, err) } diff --git a/client/allocrunner/taskrunner/identity_hook_test.go b/client/allocrunner/taskrunner/identity_hook_test.go index b3b7fd21d..d3d7c9343 100644 --- a/client/allocrunner/taskrunner/identity_hook_test.go +++ b/client/allocrunner/taskrunner/identity_hook_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/allocrunner/interfaces" trtesting "github.com/hashicorp/nomad/client/allocrunner/taskrunner/testing" cstate "github.com/hashicorp/nomad/client/state" @@ -68,9 +69,19 @@ func TestIdentityHook_RenewAll(t *testing.T) { ChangeMode: "signal", ChangeSignal: "SIGHUP", }, + { + Name: "foo", + Audience: []string{"foo"}, + File: true, + Filepath: "foo.jwt", + TTL: ttl, + }, } - secretsDir := t.TempDir() + mockTaskDir := &allocdir.TaskDir{ + SecretsDir: t.TempDir(), + Dir: t.TempDir(), + } mockTR := &MockTokenSetter{} @@ -90,7 +101,7 @@ func TestIdentityHook_RenewAll(t *testing.T) { h := &identityHook{ alloc: alloc, task: task, - tokenDir: secretsDir, + taskDir: mockTaskDir, envBuilder: taskenv.NewBuilder(node, alloc, task, alloc.Job.Region), ts: mockTR, lifecycle: mockLifecycle, @@ -109,13 +120,18 @@ func TestIdentityHook_RenewAll(t *testing.T) { // Assert initial tokens were set in Prestart must.Eq(t, alloc.SignedIdentities["web"], mockTR.defaultToken) - must.FileNotExists(t, filepath.Join(secretsDir, wiTokenFile)) - must.FileNotExists(t, filepath.Join(secretsDir, "nomad_consul.jwt")) + must.FileNotExists(t, filepath.Join(mockTaskDir.SecretsDir, wiTokenFile)) + must.FileNotExists(t, filepath.Join(mockTaskDir.SecretsDir, "nomad_consul.jwt")) must.MapContainsKey(t, env, "NOMAD_TOKEN_consul") - must.FileExists(t, filepath.Join(secretsDir, "nomad_vault.jwt")) + must.FileExists(t, filepath.Join(mockTaskDir.SecretsDir, "nomad_vault.jwt")) + // Assert foo token was written to correct directory + must.FileNotExists(t, filepath.Join(mockTaskDir.SecretsDir, "foo.jwt")) + must.FileExists(t, filepath.Join(mockTaskDir.Dir, "foo.jwt")) origConsul := env["NOMAD_TOKEN_consul"] - origVault := testutil.MustReadFile(t, secretsDir, "nomad_vault.jwt") + origVault := testutil.MustReadFile(t, mockTaskDir.SecretsDir, "nomad_vault.jwt") + + origFoo := testutil.MustReadFile(t, mockTaskDir.Dir, "foo.jwt") // Tokens should be rotated by their expiration wait := time.Until(start.Add(ttl)) @@ -144,14 +160,18 @@ func TestIdentityHook_RenewAll(t *testing.T) { must.StrContains(t, newConsul, ".") // ensure new token is JWTish must.NotEq(t, newConsul, origConsul) - newVault := testutil.MustReadFile(t, secretsDir, "nomad_vault.jwt") + newVault := testutil.MustReadFile(t, mockTaskDir.SecretsDir, "nomad_vault.jwt") must.StrContains(t, string(newVault), ".") // ensure new token is JWTish must.NotEq(t, newVault, origVault) + newFoo := testutil.MustReadFile(t, mockTaskDir.Dir, "foo.jwt") + must.StrContains(t, string(newFoo), ".") + must.NotEq(t, newFoo, origFoo) + // Assert Stop work. Tokens should not have changed. time.Sleep(wait) must.Eq(t, newConsul, h.envBuilder.Build().EnvMap["NOMAD_TOKEN_consul"]) - must.Eq(t, newVault, testutil.MustReadFile(t, secretsDir, "nomad_vault.jwt")) + must.Eq(t, newVault, testutil.MustReadFile(t, mockTaskDir.SecretsDir, "nomad_vault.jwt")) } // TestIdentityHook_RenewOne asserts token renewal only renews tokens with a TTL. @@ -179,7 +199,9 @@ func TestIdentityHook_RenewOne(t *testing.T) { }, } - secretsDir := t.TempDir() + mockTaskDir := &allocdir.TaskDir{ + SecretsDir: t.TempDir(), + } mockTR := &MockTokenSetter{} @@ -197,7 +219,7 @@ func TestIdentityHook_RenewOne(t *testing.T) { h := &identityHook{ alloc: alloc, task: task, - tokenDir: secretsDir, + taskDir: mockTaskDir, envBuilder: taskenv.NewBuilder(node, alloc, task, alloc.Job.Region), ts: mockTR, widmgr: mockWIDMgr, @@ -216,13 +238,13 @@ func TestIdentityHook_RenewOne(t *testing.T) { // Assert initial tokens were set in Prestart must.Eq(t, alloc.SignedIdentities["web"], mockTR.defaultToken) - must.FileNotExists(t, filepath.Join(secretsDir, wiTokenFile)) - must.FileNotExists(t, filepath.Join(secretsDir, "nomad_consul.jwt")) + must.FileNotExists(t, filepath.Join(mockTaskDir.SecretsDir, wiTokenFile)) + must.FileNotExists(t, filepath.Join(mockTaskDir.SecretsDir, "nomad_consul.jwt")) must.MapContainsKey(t, env, "NOMAD_TOKEN_consul") - must.FileExists(t, filepath.Join(secretsDir, "nomad_vault.jwt")) + must.FileExists(t, filepath.Join(mockTaskDir.SecretsDir, "nomad_vault.jwt")) origConsul := env["NOMAD_TOKEN_consul"] - origVault := testutil.MustReadFile(t, secretsDir, "nomad_vault.jwt") + origVault := testutil.MustReadFile(t, mockTaskDir.SecretsDir, "nomad_vault.jwt") // One token should be rotated by their expiration wait := time.Until(start.Add(ttl)) @@ -237,14 +259,14 @@ func TestIdentityHook_RenewOne(t *testing.T) { must.StrContains(t, newConsul, ".") // ensure new token is JWTish must.Eq(t, newConsul, origConsul) - newVault := testutil.MustReadFile(t, secretsDir, "nomad_vault.jwt") + newVault := testutil.MustReadFile(t, mockTaskDir.SecretsDir, "nomad_vault.jwt") must.StrContains(t, string(newVault), ".") // ensure new token is JWTish must.NotEq(t, newVault, origVault) // Assert Stop work. Tokens should not have changed. time.Sleep(wait) must.Eq(t, newConsul, h.envBuilder.Build().EnvMap["NOMAD_TOKEN_consul"]) - must.Eq(t, newVault, testutil.MustReadFile(t, secretsDir, "nomad_vault.jwt")) + must.Eq(t, newVault, testutil.MustReadFile(t, mockTaskDir.SecretsDir, "nomad_vault.jwt")) } // TestIdentityHook_ErrorWriting assert Prestart returns an error if the @@ -260,10 +282,14 @@ func TestIdentityHook_ErrorWriting(t *testing.T) { stopCtx, stop := context.WithCancel(context.Background()) t.Cleanup(stop) + mockTaskDir := &allocdir.TaskDir{ + SecretsDir: "/this-should-not-exist", + } + h := &identityHook{ alloc: alloc, task: task, - tokenDir: "/this-should-not-exist", + taskDir: mockTaskDir, envBuilder: taskenv.NewBuilder(node, alloc, task, alloc.Job.Region), ts: &MockTokenSetter{}, logger: testlog.HCLogger(t), diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index f625f3a93..a6142f5ee 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1681,6 +1681,7 @@ func apiWorkloadIdentityToStructs(in *api.WorkloadIdentity) *structs.WorkloadIde ChangeSignal: in.ChangeSignal, Env: in.Env, File: in.File, + Filepath: in.Filepath, ServiceName: in.ServiceName, TTL: in.TTL, } diff --git a/e2e/workload_id/input/identity.nomad b/e2e/workload_id/input/identity.nomad index cd8e21aa7..3e941077e 100644 --- a/e2e/workload_id/input/identity.nomad +++ b/e2e/workload_id/input/identity.nomad @@ -90,6 +90,27 @@ job "identity" { } } + task "filepath" { + + identity { + file = true + filepath = "local/nomad_token" + } + + driver = "docker" + config { + image = "bash:5" + + #HACK without the ending `sleep 2` we seem to sometimes miss logs :( + args = ["-c", "wc -c < local/nomad_token; env | grep NOMAD_TOKEN; echo done; sleep 2"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + # falsey task should be the same as no identity block task "falsey" { diff --git a/e2e/workload_id/workload_id_test.go b/e2e/workload_id/workload_id_test.go index c2818216f..89066c450 100644 --- a/e2e/workload_id/workload_id_test.go +++ b/e2e/workload_id/workload_id_test.go @@ -70,6 +70,11 @@ func testIdentity(t *testing.T) { env: false, file: true, }, + { + task: "filepath", + env: false, + file: true, + }, { task: "falsey", env: false, @@ -94,6 +99,10 @@ func testIdentity(t *testing.T) { must.StrHasSuffix(t, "done\n", logs, ps) lines := strings.Split(logs, "\n") + + // Whether filepath is set or not, the log length assertion should + // be the same, assuming the token was read from the correct location. + // So we don't need special switch cases for filepath. switch { case tc.env && tc.file: must.Len(t, 4, lines, ps) diff --git a/nomad/config.go b/nomad/config.go index 1c0905631..3a17623e8 100644 --- a/nomad/config.go +++ b/nomad/config.go @@ -545,6 +545,9 @@ func workloadIdentityFromConfig(widConfig *config.WorkloadIdentityConfig) *struc if widConfig.File != nil { wid.File = *widConfig.File } + if widConfig.Filepath != "" { + wid.Filepath = widConfig.Filepath + } if widConfig.TTL != nil { wid.TTL = *widConfig.TTL } diff --git a/nomad/structs/config/workload_id.go b/nomad/structs/config/workload_id.go index 890195840..872acd81e 100644 --- a/nomad/structs/config/workload_id.go +++ b/nomad/structs/config/workload_id.go @@ -31,9 +31,13 @@ type WorkloadIdentityConfig struct { Env *bool `mapstructure:"env"` // File writes the Workload Identity into the Task's secrets directory - // if set. + // or specified filepath if set. File *bool `mapstructure:"file"` + // Filepath write the Workload Identity to a specified directory in the + // Task's filesystem + Filepath string `mapstructure:"filepath"` + // TTL is used to determine the expiration of the credentials created for // this identity (eg the JWT "exp" claim). TTL *time.Duration `mapstructure:"-"` @@ -83,6 +87,9 @@ func (wi *WorkloadIdentityConfig) Equal(other *WorkloadIdentityConfig) bool { if !pointer.Eq(wi.File, other.File) { return false } + if wi.Filepath != other.Filepath { + return false + } if !pointer.Eq(wi.TTL, other.TTL) { return false } @@ -119,6 +126,9 @@ func (wi *WorkloadIdentityConfig) Merge(other *WorkloadIdentityConfig) *Workload result.Env = pointer.Merge(result.Env, other.Env) result.File = pointer.Merge(result.File, other.File) result.TTL = pointer.Merge(result.TTL, other.TTL) + if other.Filepath != "" { + result.Filepath = other.Filepath + } if other.TTLHCL != "" { result.TTLHCL = other.TTLHCL } diff --git a/nomad/structs/config/workload_id_test.go b/nomad/structs/config/workload_id_test.go index ec7f0e44f..0073f32fc 100644 --- a/nomad/structs/config/workload_id_test.go +++ b/nomad/structs/config/workload_id_test.go @@ -20,6 +20,7 @@ func TestWorkloadIdentityConfig_Copy(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(true), File: pointer.Of(false), + Filepath: "foo", TTL: pointer.Of(time.Hour), } @@ -36,12 +37,14 @@ func TestWorkloadIdentityConfig_Copy(t *testing.T) { clone.Audience[0] = "changed" *clone.Env = false *clone.File = true + clone.Filepath = "changed" *clone.TTL = time.Second must.NotEq(t, original, clone) must.NotEqOp(t, original, clone) must.NotEqOp(t, original.Env, clone.Env) must.NotEqOp(t, original.File, clone.File) + must.NotEqOp(t, original.Filepath, clone.Filepath) must.NotEqOp(t, original.TTL, clone.TTL) } @@ -61,6 +64,7 @@ func TestWorkloadIdentityConfig_Equal(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(true), File: pointer.Of(false), + Filepath: "foo", TTL: pointer.Of(time.Hour), }, b: &WorkloadIdentityConfig{ @@ -68,6 +72,7 @@ func TestWorkloadIdentityConfig_Equal(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(true), File: pointer.Of(false), + Filepath: "foo", TTL: pointer.Of(time.Hour), }, expectEq: true, @@ -122,6 +127,16 @@ func TestWorkloadIdentityConfig_Equal(t *testing.T) { }, expectEq: false, }, + { + name: "different filepath", + a: &WorkloadIdentityConfig{ + Filepath: "a", + }, + b: &WorkloadIdentityConfig{ + Filepath: "b", + }, + expectEq: false, + }, { name: "different file nil", a: &WorkloadIdentityConfig{ @@ -173,6 +188,7 @@ func TestWorkloadIdentityConfig_Merge(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(true), File: pointer.Of(false), + Filepath: "test", TTL: pointer.Of(time.Hour), }, }, @@ -186,6 +202,7 @@ func TestWorkloadIdentityConfig_Merge(t *testing.T) { Audience: []string{"aud", "other"}, Env: pointer.Of(true), File: pointer.Of(false), + Filepath: "test", TTL: pointer.Of(time.Hour), }, }, @@ -199,6 +216,7 @@ func TestWorkloadIdentityConfig_Merge(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(false), File: pointer.Of(false), + Filepath: "test", TTL: pointer.Of(time.Hour), }, }, @@ -212,6 +230,21 @@ func TestWorkloadIdentityConfig_Merge(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(true), File: pointer.Of(true), + Filepath: "test", + TTL: pointer.Of(time.Hour), + }, + }, + { + name: "merge filepath", + other: &WorkloadIdentityConfig{ + Filepath: "other", + }, + expected: &WorkloadIdentityConfig{ + Name: "test", + Audience: []string{"aud"}, + Env: pointer.Of(true), + File: pointer.Of(false), + Filepath: "other", TTL: pointer.Of(time.Hour), }, }, @@ -225,6 +258,7 @@ func TestWorkloadIdentityConfig_Merge(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(true), File: pointer.Of(false), + Filepath: "test", TTL: pointer.Of(time.Second), }, }, @@ -237,6 +271,7 @@ func TestWorkloadIdentityConfig_Merge(t *testing.T) { Audience: []string{"aud"}, Env: pointer.Of(true), File: pointer.Of(false), + Filepath: "test", TTL: pointer.Of(time.Hour), } got := original.Merge(tc.other) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 8e62e0599..96cb8cbb2 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -8340,7 +8340,14 @@ func (t *Task) Validate(jobType string, tg *TaskGroup) error { // TODO: Investigate validation of the PluginMountDir. Not much we can do apart from check IsAbs until after we understand its execution environment though :( } - // Validate Identity/Identities + // Validate default Identity + if t.Identity != nil { + if err := t.Identity.Validate(); err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Identity %q is invalid: %w", t.Identity.Name, err)) + } + } + + // Validate Identities for _, wid := range t.Identities { // Task.Canonicalize should move the default identity out of the Identities // slice, so if one is found that means it is a duplicate. diff --git a/nomad/structs/workload_id.go b/nomad/structs/workload_id.go index 0787e2ad7..f2b2b0c77 100644 --- a/nomad/structs/workload_id.go +++ b/nomad/structs/workload_id.go @@ -304,9 +304,13 @@ type WorkloadIdentity struct { Env bool // File writes the Workload Identity into the Task's secrets directory - // if set. + // or path specified by Filepath if set. File bool + // Filepath is used to specify a custom path for the Task's Workload + // Identity JWT. + Filepath string + // ServiceName is used to bind the identity to a correct Consul service. ServiceName string @@ -356,6 +360,7 @@ func (wi *WorkloadIdentity) Copy() *WorkloadIdentity { ChangeSignal: wi.ChangeSignal, Env: wi.Env, File: wi.File, + Filepath: wi.Filepath, ServiceName: wi.ServiceName, TTL: wi.TTL, } @@ -390,6 +395,10 @@ func (wi *WorkloadIdentity) Equal(other *WorkloadIdentity) bool { return false } + if wi.Filepath != other.Filepath { + return false + } + if wi.ServiceName != other.ServiceName { return false } @@ -462,6 +471,10 @@ func (wi *WorkloadIdentity) Validate() error { mErr.Errors = append(mErr.Errors, fmt.Errorf("ttl must be >= 0")) } + if wi.Filepath != "" && !wi.File { + mErr.Errors = append(mErr.Errors, fmt.Errorf("file parameter must be true in order to specify filepath")) + } + return mErr.ErrorOrNil() } diff --git a/nomad/structs/workload_id_test.go b/nomad/structs/workload_id_test.go index e4afdaca8..af455bc96 100644 --- a/nomad/structs/workload_id_test.go +++ b/nomad/structs/workload_id_test.go @@ -575,6 +575,12 @@ func TestWorkloadIdentity_Equal(t *testing.T) { newWI.File = false must.Equal(t, orig, newWI) + newWI.Filepath = "foo" + must.NotEqual(t, orig, newWI) + + newWI.Filepath = "" + must.Equal(t, orig, newWI) + newWI.Name = "foo" must.NotEqual(t, orig, newWI) @@ -769,6 +775,14 @@ func TestWorkloadIdentity_Validate(t *testing.T) { }, Warn: "identities without an expiration are insecure", }, + { + Desc: "Filepath set without file", + In: WorkloadIdentity{ + Name: "foo", + Filepath: "foo", + }, + Err: "file parameter must be true in order to specify filepath", + }, } for _, tc := range cases { diff --git a/website/content/docs/job-specification/identity.mdx b/website/content/docs/job-specification/identity.mdx index ec2299274..a02f809c4 100644 --- a/website/content/docs/job-specification/identity.mdx +++ b/website/content/docs/job-specification/identity.mdx @@ -34,6 +34,7 @@ job "docs" { identity { env = true file = true + filepath = "local/example.jwt" # Restart on token renewal to get the new env var change_mode = "restart" @@ -81,6 +82,9 @@ job "docs" { [`task.user`][taskuser] parameter is set, the token file will only be readable by that user. Otherwise the file is readable by everyone but is protected by parent directory permissions. +- `filepath` `(string: "")` - If not empty and file is `true`, the workload + identity will be available at the specified location relative to the + [task working directory][] instead of the `NOMAD_SECRETS_DIR`. - `ttl` `(string: "")` - The lifetime of the identity before it expires. The client will renew the identity at roughly half the TTL. This is specified using a label suffix like "30s" or "1h". You may not set a TTL on the default @@ -355,3 +359,4 @@ EOF [taskapi]: /nomad/api-docs/task-api [taskuser]: /nomad/docs/job-specification/task#user "Nomad task Block" [windows]: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/ +[task working directory]: /nomad/docs/runtime/environment#task-directories 'Task Directories'