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

INS-62 / Make Vault-PKI formula stateful + blocking #1

Merged
merged 19 commits into from
Aug 13, 2019
Merged

Conversation

jfryman
Copy link
Contributor

@jfryman jfryman commented Jan 9, 2019

This PR updates the Vault-PKI formula to add a few features:

  • 'cert' state should be stateful (not appear to have changes each time it is included in high state)

  • 'cert' state should have a blocking mode to wait and poll for the cert delivery, will allow use of requisites on the cert state namely by consul + nginx which rely on a cert to start cleanly

Requires: ripple/salt-runner-vault-pki#4

Copy link
Contributor

@dmwilcox dmwilcox left a comment

Choose a reason for hiding this comment

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

Looks good, just some docs + naming stuff to iron out.

cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/init.sls Outdated Show resolved Hide resolved
cert/init.sls Outdated Show resolved Hide resolved
cert/init.sls Outdated Show resolved Hide resolved
@jfryman jfryman force-pushed the jfryman/INS-62 branch 2 times, most recently from dd5dc38 to 5ee43c4 Compare January 24, 2019 00:23
@jfryman jfryman force-pushed the jfryman/INS-62 branch 5 times, most recently from 945cf08 to 44fa5f7 Compare January 31, 2019 00:27
@jfryman jfryman force-pushed the jfryman/INS-62 branch 5 times, most recently from b525884 to 4ab4398 Compare January 31, 2019 21:36
@jfryman
Copy link
Contributor Author

jfryman commented Jan 31, 2019

To use this with testing, you will need to export the following variable:

export VAULT_PKI_RUNNER_BRANCH=jfryman/INS-62

Then you can proceed with vagrant commands.

This commit updates Vault-PKI to rely less on the salt reactor for
processing a signed certificate request. Instead of the response
being sent back out of band, and having another salt run to activate,
the Vault-PKI now waits and watches the event bus waiting for a return
message and then reacts internally. This simplifies the overall
execution strategy for this module, and allows for blocking and state
dependency management which has not been possible up to this point.
Copy link
Contributor

@dmwilcox dmwilcox left a comment

Choose a reason for hiding this comment

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

Looks good -- mostly just clean-up stuff. I haven't reviewed testing yet but wanted to give you something to chew on. Thanks!

cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
cert/files/vault_pki.py Outdated Show resolved Hide resolved
Copy link
Contributor

@dmwilcox dmwilcox left a comment

Choose a reason for hiding this comment

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

Please fix one pep8 nit and submit, this looks great! Thank you!

cert/files/vault_pki.py Outdated Show resolved Hide resolved
@jfryman jfryman merged commit 8cf3304 into master Aug 13, 2019
@jfryman jfryman deleted the jfryman/INS-62 branch August 13, 2019 18:58
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