vault: update task runner vault hook to support workload identity #18534

lgfa29 · 2023-09-18T20:58:59Z

Update the task runner Vault hook and the Vault client so they are able to derive tokens using the task signed workload identity JWT.

The new flow is used whenever a task has an identity with the pattern vault_<cluster>, where <cluster> matches the value of the vault.cluster config applied to the task.

When using JWT and workload identities, the vault.token value in the Nomad server configuration will likely be empty. This would cause the legacy Vault client in the Nomad servers to fail on start.

When a vault.default_identity is given for the default cluster, the Vault client in the servers are replaced by a no-op implementation since it won't be used in any way.

Until Nomad 1.9, where the legacy flow will be removed, the default Vault cluster can still use the legacy flow. In Nomad ENT this allows operators to mix authentication flows, where the default cluster uses the legacy flow and additional clusters use the JWT flow. Non-default clusters are not allowed to use the legacy flow.

Note to reviewers: most code changes are for tests and moving things around to avoid circular dependencies. Main logic is implemented in cf81d72

tgross

This is looking great @lgfa29. I've left a few comments around areas I'm unclear on

client/allocrunner/taskrunner/task_runner_hooks.go

nomad/server.go

client/allocrunner/taskrunner/vault_hook.go

client/vaultclient/vaultclient.go

client/allocrunner/taskrunner/template_hook.go

client/allocrunner/taskrunner/vault_hook.go

nomad/config.go

nomad/structs/workload_id.go

tgross

LGTM, nice work on all the added tests. My only potential concern is the handling of recoverable/non-recoverable errors. But once you're happy with that I'm 👍 to merge.

client/allocrunner/taskrunner/vault_hook.go

tgross · 2023-10-16T13:53:59Z

client/allocrunner/taskrunner/vault_hook.go

+		return "", structs.WrapRecoverable(
+			fmt.Sprintf("failed to derive Vault token for identity %s: %v", h.widName, err),
+			err,
+		)


Likewise, I'm not sure we can know if this is recoverable or not. It could be an issue with the Vault auth config, in which case there's nothing Nomad can do about it.

This could also happen due to an intermittent connectivity issue (network blip, Vault agent restart etc.) so making it recoverable allow us to retry.

client/allocrunner/taskrunner/vault_hook_test.go

tgross · 2023-10-16T14:42:01Z

client/allocrunner/taskrunner/vault_hook_test.go

+	testCases := []struct {
+		name                string
+		vaultBlock          *structs.Vault
+		verifyTaskLifecycle func(*trtesting.MockTaskHooks) error


I really like how this test works 👍

tgross · 2023-10-16T14:43:16Z

client/vaultclient/vaultclient_test.go

+	return nil
+}
+
+func TestVaultClient_DeriveTokenWithJWT(t *testing.T) {


Do we have a test helper that can skip the test if there's no Vault binary around? That might be nice to reduce wait times and spurious errors for non-CI development.

Good point, I will poke around other tests like this 👍

I didn't find any check like this so I updated NewTestVault in be3cedd to skip the test if the binary is not found.

Receiving a nil identity is an unexpected result as the WIDManager should have all identities by this point.

lgfa29 · 2023-10-16T20:45:05Z

Rebased against main to fix conflict.

github-actions · 2025-02-08T02:15:50Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

vercel bot deployed to Preview – nomad-storybook-and-ui September 18, 2023 21:01 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from 829bf43 to 711e2e8 Compare September 19, 2023 22:54

vercel bot deployed to Preview – nomad-storybook-and-ui September 19, 2023 22:57 View deployment

tgross reviewed Sep 20, 2023

View reviewed changes

client/allocrunner/taskrunner/task_runner_hooks.go Outdated Show resolved Hide resolved

nomad/server.go Outdated Show resolved Hide resolved

client/allocrunner/taskrunner/vault_hook.go Outdated Show resolved Hide resolved

client/vaultclient/vaultclient.go Outdated Show resolved Hide resolved

jorgemarey reviewed Sep 20, 2023

View reviewed changes

client/vaultclient/vaultclient.go Outdated Show resolved Hide resolved

lgfa29 force-pushed the f-vault-hook-wid branch from 711e2e8 to e79c4df Compare September 20, 2023 19:41

vercel bot deployed to Preview – nomad-storybook-and-ui September 20, 2023 19:43 View deployment

Base automatically changed from f-consul-wi-consul_hook to main September 21, 2023 15:31

lgfa29 force-pushed the f-vault-hook-wid branch from e79c4df to 356a755 Compare September 22, 2023 16:43

vercel bot deployed to Preview – nomad-storybook-and-ui September 22, 2023 16:45 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from 356a755 to 3f06537 Compare September 23, 2023 01:56

vercel bot deployed to Preview – nomad-storybook-and-ui September 23, 2023 01:59 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from 3f06537 to 7ddead2 Compare September 23, 2023 01:59

vercel bot deployed to Preview – nomad-storybook-and-ui September 23, 2023 02:02 View deployment

tgross reviewed Sep 25, 2023

View reviewed changes

This was referenced Sep 26, 2023

config: fix some issues with workload identity and multi Consul and Vault #18590

Merged

vault: update identity name to start with vault_ #18591

Merged

lgfa29 force-pushed the f-vault-hook-wid branch from 7ddead2 to d76323c Compare September 29, 2023 15:25

vercel bot deployed to Preview – nomad-storybook-and-ui September 29, 2023 15:31 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from d76323c to bcde86b Compare September 29, 2023 17:17

vercel bot deployed to Preview – nomad-storybook-and-ui September 29, 2023 17:20 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from bcde86b to c110ee3 Compare September 29, 2023 19:14

vercel bot deployed to Preview – nomad-storybook-and-ui September 29, 2023 19:17 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from c110ee3 to c98262b Compare September 29, 2023 20:05

vercel bot deployed to Preview – nomad-storybook-and-ui September 29, 2023 20:08 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from c98262b to 6c3e32b Compare October 12, 2023 15:57

vercel bot deployed to Preview – nomad-storybook-and-ui October 12, 2023 16:00 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui October 12, 2023 22:40 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui October 14, 2023 01:35 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui October 14, 2023 03:25 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from 2499ae0 to 9485001 Compare October 14, 2023 04:07

vercel bot deployed to Preview – nomad-storybook-and-ui October 14, 2023 04:10 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from 9485001 to 71fe098 Compare October 14, 2023 04:29

lgfa29 added this to the 1.7.0 milestone Oct 14, 2023

lgfa29 requested review from pkazmierczak and tgross October 14, 2023 04:32

vercel bot deployed to Preview – nomad-storybook-and-ui October 14, 2023 04:32 View deployment

lgfa29 marked this pull request as ready for review October 14, 2023 04:32

lgfa29 force-pushed the f-vault-hook-wid branch from 71fe098 to 89bc720 Compare October 14, 2023 04:38

vercel bot deployed to Preview – nomad-storybook-and-ui October 14, 2023 04:41 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch from 89bc720 to 7b34993 Compare October 14, 2023 05:15

vercel bot deployed to Preview – nomad-storybook-and-ui October 14, 2023 05:17 View deployment

lgfa29 force-pushed the f-vault-hook-wid branch 2 times, most recently from ca4feee to 41fe4e1 Compare October 14, 2023 05:21

vercel bot deployed to Preview – nomad-storybook-and-ui October 14, 2023 05:25 View deployment

tgross approved these changes Oct 16, 2023

View reviewed changes

vault: jwt login

29fefae

vercel bot deployed to Preview – nomad-storybook-and-ui October 16, 2023 20:39 View deployment

lgfa29 added 5 commits October 16, 2023 16:44

vault: add tests for vault_hook

4cb14a5

vault: add JWT token derivation test

7dbf325

vault: make nil identity error unrecoverable

0f81346

Receiving a nil identity is an unexpected result as the WIDManager should have all identities by this point.

vault: setupTestHook -> setupTestVaultHook

37ec3cf

test: skip tests if vault binary is not found

717d531

lgfa29 force-pushed the f-vault-hook-wid branch from be3cedd to 717d531 Compare October 16, 2023 20:44

vercel bot deployed to Preview – nomad-storybook-and-ui October 16, 2023 20:47 View deployment

lgfa29 merged commit 349c032 into main Oct 16, 2023

lgfa29 deleted the f-vault-hook-wid branch October 16, 2023 23:37

github-actions bot locked as resolved and limited conversation to collaborators Feb 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vault: update task runner vault hook to support workload identity #18534

vault: update task runner vault hook to support workload identity #18534

lgfa29 commented Sep 18, 2023 •

edited

Loading

tgross left a comment

tgross left a comment

tgross Oct 16, 2023

lgfa29 Oct 16, 2023

tgross Oct 16, 2023

tgross Oct 16, 2023

lgfa29 Oct 16, 2023

lgfa29 Oct 16, 2023

lgfa29 commented Oct 16, 2023

github-actions bot commented Feb 8, 2025

vault: update task runner vault hook to support workload identity #18534

vault: update task runner vault hook to support workload identity #18534

Conversation

lgfa29 commented Sep 18, 2023 • edited Loading

tgross left a comment

Choose a reason for hiding this comment

tgross left a comment

Choose a reason for hiding this comment

tgross Oct 16, 2023

Choose a reason for hiding this comment

lgfa29 Oct 16, 2023

Choose a reason for hiding this comment

tgross Oct 16, 2023

Choose a reason for hiding this comment

tgross Oct 16, 2023

Choose a reason for hiding this comment

lgfa29 Oct 16, 2023

Choose a reason for hiding this comment

lgfa29 Oct 16, 2023

Choose a reason for hiding this comment

lgfa29 commented Oct 16, 2023

github-actions bot commented Feb 8, 2025

lgfa29 commented Sep 18, 2023 •

edited

Loading