Skip to content
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

WI: set identity to client secret if missing #15121

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

tgross
Copy link
Member

@tgross tgross commented Nov 3, 2022

For #15115

Allocations created before 1.4.0 will not have a workload identity token. When the client running these allocs is upgraded to 1.4.x, the identity hook will run and replace the node secret ID token used previously with an empty string. This causes service discovery queries to fail.

Fallback to the node's secret ID when the allocation doesn't have a signed identity. Note that pre-1.4.0 allocations won't have templates that read Variables, so there's no threat that this new node ID secret will be able to read data that the allocation shouldn't have access to.

I've smoke-tested this by creating a build of Nomad that doesn't sign identities in the plan applier, and verified that we can query service registrations as expected now.

@tgross tgross added this to the 1.4.3 milestone Nov 3, 2022
@tgross tgross changed the title wi: set identity to client secret if missing WI: set identity to client secret if missing Nov 3, 2022
Allocations created before 1.4.0 will not have a workload identity token. When
the client running these allocs is upgraded to 1.4.x, the identity hook will run
and replace the node secret ID token used previously with an empty string. This
causes service discovery queries to fail.

Fallback to the node's secret ID when the allocation doesn't have a signed
identity. Note that pre-1.4.0 allocations won't have templates that read
Variables, so there's no threat that this new node ID secret will be able to
read data that the allocation shouldn't have access to.
Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testes locally in an upgrade scenario that failed before, and it works now. Thanks for the fix!

@tgross tgross added the backport/1.4.x backport to 1.4.x release line label Nov 3, 2022
@tgross tgross merged commit 5a5b4b0 into main Nov 3, 2022
@tgross tgross deleted the b-client-node-secret-on-missing-wi branch November 3, 2022 15:10
@github-actions
Copy link

github-actions bot commented Mar 4, 2023

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants