Declare salt dependencies explicitly in provisioning #210
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
conorsch
force-pushed
the
declare-salt-dependencies-explicitly-in-provisioning
branch
from
6d9d31d to
55ea887
Compare
emkll
force-pushed
the
declare-salt-dependencies-explicitly-in-provisioning
branch
5 times, most recently
from
6c4d651 to
9d5ad56
Compare
emkll
reviewed
Nov 14, 2018
There was a problem hiding this comment.
Updated the docs and appended to this branch with instructions on completing the f26->28 transition, but still experiencing one error with make test:
test_we_have_the_eu (test_gpg.SD_GPG_Tests)
Confirmed the private key is not present in the gpg keyring of sd-gpg. Investigating
emkll
force-pushed
the
declare-salt-dependencies-explicitly-in-provisioning
branch
2 times, most recently
from
9e4f255 to
934ad08
Compare
emkll
approved these changes
Nov 14, 2018
There was a problem hiding this comment.
Looks good to me, the securedrop-client shortcut for sd-svs does not appear in the list after the clean install (not sure if it's due to clean install or if it's the new logic, I'm inclined to think the former)
With the changes to the readme appended to this PR, this PR should also close #183
@conorsch please review the changes to the docs and gpg fix and merge.
added 4 commits
November 14, 2018 15:13
The Salt logic for declaring dependencies was incorrect. In order to use
the `require` state attribute effectively, the state (.sls) file *must*
either:
1. Contain the referenced state elsewhere in the file; OR
2. Use `include` to pull in the state file in which the referenced
state is declared.
Additionally, it's important that the `require` line be an attribute on
a state, not on the unique state id (the human-readable name for the
task).
Using a wrapper script simply to support injecting the VM names via the "list-vms" target, since we need to join them with commas in the Salt targeting. Removes "clean" as an automatic step on the "all" action. It's still available to be run manually, but is now opt-in.
Since we're running it parallel, we must carefully declare dependencies, so that requisites are in place before tasks assuming their presence are run. Uses a bit of the "onchanged" pragma to trigger commands only when updates have happened to files on disk. The syncing appmenus in particular tasks a long time, so it's useful to run that only when VMs have changed. Note that we're now copying in the submission key into the homedir of sd-gpg; we were previously re-copying it to /tmp daily as part of the update script; the advantage now is that we can skip the import if the key didn't change.
The only customizations being written to `sd-whonix` are the HidServAuth token for the Journalist Interface. The Whonix Gateway VM already supports customizations of such torrc entries via /usr/local/, so the template isn't buying us anything. Don't bother to provision it.
emkll
force-pushed
the
declare-salt-dependencies-explicitly-in-provisioning
branch
from
934ad08 to
2fbba36
Compare
The key is no longer copied to /tmp but rather from /home/user/.gnupg
emkll
force-pushed
the
declare-salt-dependencies-explicitly-in-provisioning
branch
from
2fbba36 to
49f4856
Compare
| qfile-agent : Fatal error: File copy: Disk quota exceeded; Last file: <...> (error type: Disk quota exceeded) '/usr/lib/qubes/qrexec-client-vm dom0 qubes.Receiveupdates /usr/lib/qubes/qfile-agent /var/lib/qubes/dom0-updates/packages/*.rpm' failed with exit code 1! | ||
| ``` | ||
|
|
||
| ##### Upgrading `sys-net`, `sys-usb` and `sys-firewall` to fedora-28 |
There was a problem hiding this comment.
we also need to add instructions for updating whonix
These steps are only required when using Qubes 4.0. Qubes 4.0.1 (which is currently at RC status) will no longer require these steps.
Just for readability, also yum -> dnf since it redirects anyway. Not hardcoding any `qvm-remove` commands because the dependency chain of VM relations is a bit tricky; an exercise left to the reader, for the nonce, unfortunately.
|
Beautiful additions, @emkll. Docs are on point. We still have one small issue, that the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uses proper "include" and "require" syntax in the Salt configs. We had been applying these directives inconsistently, and
--show-outputreveals complaining about inability to import, causing race conditions when the salt states are ordered (which happens automatically prior to run).Changes:
sd-whonix-template; there were zero configurations of the template, so it's unnecessary. We've always added the HidServAuth line directly tosd-whonix, where it persists due to/usr/local/persistences in AppVMs.The rebuild story is now vastly faster: on clean installs, 35m -> 15m. On rebuilds (where VMs already exist), 35m -> 6m.
Testing
Use the normal test plan:
make cleanmake allmake testTry running
make allagain.