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

feature: combine root and nonroot secret install; delay chowning #119

Merged
merged 2 commits into from
Sep 1, 2022
Merged

Conversation

ryantm
Copy link
Owner

@ryantm ryantm commented Jul 10, 2022

This simplifies agenix by combining the root and nonRoot secret installation into one place and delays setting the owner and group of the secrets until after the users and groups activation scripts are completed. This also fixes #117 by incorporating the changes from #118 to not switch over the secret directory symlink until after the secrets are decrypted.

This is a breaking change in the sense that someone might have depended on the user or group of a root secret being set before the "users" and "groups" activation scripts run, but that seems unlikely to me.

This is also a breaking change because I renamed a bunch of the activation scripts. I believe these will be module compile-time errors.

cc @jsimonetti

jsimonetti and others added 2 commits July 10, 2022 19:12
Change the order of operations to:

1. create new generation
2. decrypt secrets into new generation
3. symlink and remove old generation/secrets

Signed-off-by: Jeroen Simonetti <jeroen@simonetti.nl>
@ryantm ryantm requested a review from cole-h July 10, 2022 18:55
@jsimonetti
Copy link
Contributor

Perhaps it would be wise (considering the possible breaking) to tag a new release before and after merging?

Copy link
Collaborator

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

Sorry for letting this hang so long -- this looks fine to me, but I haven't tested it.

@cole-h
Copy link
Collaborator

cole-h commented Aug 5, 2022

We should be sure to cut a release before and after this merges, though.

@jsimonetti
Copy link
Contributor

Just curious, is there anything blocking this PR?

@ryantm ryantm merged commit 9f136ec into main Sep 1, 2022
@cole-h cole-h deleted the order branch September 1, 2022 15:44
@ryantm
Copy link
Owner Author

ryantm commented Sep 1, 2022

Both those releases have been cut and release notes added. Thanks for the reviews everyone!

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.

Order of operations
3 participants