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

VAULT-30189: enos_remote_exec: Use non-static ID as prefix for scripts #21

Merged
merged 3 commits into from
Aug 29, 2024

Conversation

ryancragun
Copy link
Contributor

@ryancragun ryancragun commented Aug 27, 2024

How to read this pull request

When enos_remote_exec executes a script on a remote machine it copies
the contents to a local place on the target disk before executing it.
Because scripts can be re-used by different resources but provided
different env vars, we used to consider the env vars as part of the SHA
but it looks like it was it was erroneously removed[0] at some point.

What we have now is a case where multiple resources can use the same
script and therefore the same SHA in parallel. That leads to a race where
multiple resources are writing, executing, and cleaning up scripts with
the same SHA.

Instead of just reverting to the prior behavior, we now generate a
non-static ID for each enos_remote_exec resource at apply time. When
we copy over script contents we prepend this ID, along with the content
SHA, to prevent any script mod races.

[0] 44fe793

Signed-off-by: Ryan Cragun [email protected]

Checklist

  • The commit message includes an explanation of the changes
  • Manual validation of the changes have been performed (if possible)
  • New or modified code has requisite test coverage (if possible)
  • I have performed a self-review of the changes
  • I have made necessary changes and/or pull requests for documentation
  • I have written useful comments in the code
  • Version file/release label updated, if release needed

When `enos_remote_exec` executes a script on a remote machine it copies
the contents to a local place on the target disk before executing it.
Because scripts can be re-used by different resources but provided
different env vars, we used to consider the env vars as part of the SHA
but it looks like it was it was erroneously removed[0] at some point.

What we have now is a case where multiple resources can use the same
script and therefore the same SHA in parallel. That leads to a race where
multiple resources are writing, executing, and cleaning up scripts with
the same SHA.

Instead, of reverting to the prior behavior, we instead now generate a
non-static ID for each `enos_remote_exec` resource at apply time. When
we copy over script contents we prepend this ID, along with the content
SHA, to prevent any script mod races.

[0] 44fe793

Signed-off-by: Ryan Cragun <[email protected]>
@ryancragun ryancragun requested a review from a team as a code owner August 27, 2024 16:08
@ryancragun ryancragun changed the title Vault 30189 VAULT-30189: enos_remote_exec: Use non-static ID as prefix for scripts Aug 27, 2024
@ryancragun ryancragun merged commit 5d34826 into main Aug 29, 2024
19 checks passed
@ryancragun ryancragun deleted the vault-30189 branch August 29, 2024 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants