Feedback on Qubes/Salt blogpost from Ben Grande

qubes-blogpost-ben-grande-feedback.txt

- Always enforce preferences on the `qvm.prefs` module: many times I see
  `label` only in `qvm.present`, which is only run on creation of the
  qube, not when it already exists. The `qvm.prefs` module is checked
  everytime even if the qube already exists. I think label should be
  enforced not let for the user to modify and then later the state
  ignore the label change, but of course I don't know your internal
  policy about this matter and I might be wrong depending on it.

- Do not specify literal names as the `source` value of `qvm.clone`,
  such as `debian-11`. If you do this, on the next template update to
  `debian-12`, you will need to change every reference, this means, code
  duplication and error prone, people often forget to update multiple
  references, even when doing a regex search. I'd recommend using a
  single point of reference, always to the same template version (if it
  is Debian, then 12, if it is Fedora, then 39) and if a software
  requires an old template version, then only in this case it can be
  specified explicitly. See how I do it in clone.sls[1], using the
  clone-template.sls macro[2] and the template version is especified
  only once[3].

- Do not place source files of the module `file` in the root of the
  formula, such as `salt://guardian/nautilus-extension.py`. Put it at
  least in a more specialized directory such as `guardian/files/`, this
  is much better to separate state files from the files placed in
  minions. With a more organized file structure, it is easier to grasp
  the formula, imagine when dealing with multiple files that should be
  placed on different minions plus when the file name is the same, would
  you rather put a prefix in the file name (minion1-file, minion2-file)
  or put it in separate folders (guardian/files/minion1/file,
  guardian/files/minion2/file)? I prefer the latter, it is much easier
  to read and doesn't require to change every minion file to have a
  different prefix. One good source of file hierarchy is from the
  apache-formula[4].

- Use `state.apply` rather than `state.highstate`[5], even when applying
  a highstate rather than a sls file, it is easier to understand and
  that is the Salt Project recommendation.

- Create unique state IDs, do not use `install-packages`, if you apply
  multiple states that have the same ID, the state will fail with
  conflicting IDs (happens when applying a highstate that calls multiple
  states on different projects). Prefer unique IDs, such as
  `guardian-media-install-packages`, `guardian-proxy-install-packages`,
  this way, the proxy packages and the media packages from different
  states can be applied simutaneously with an error. In your blog post,
  this doesn't happen because there is not multiple calls, but it may
  happen when your formulas get bigger.

- Lint your state files with `salt-lint`[6], it helps developer detect
  errors and get them fixed. Example: `mode: 555`, it should be
  `"0555"`, quoted value[7] with preceeding zero[8]. Many editors allows
  to have linters integrated for better experience, I use it with Vim,
  but you can use it with GUI IDEs such as VSCode.
  
[0]: https://www.theguardian.com/info/2024/apr/04/when-security-matters-working-with-qubes-os-at-the-guardian
[1]: https://github.com/ben-grande/qusal/blob/main/salt/dev/clone.sls
[2]: https://github.com/ben-grande/qusal/blob/main/salt/utils/macros/clone-template.sls
[3]: https://github.com/ben-grande/qusal/blob/main/salt/debian/template.jinja#L8
[4]: https://github.com/saltstack-formulas/apache-formula/tree/master/apache
[5]: https://docs.saltproject.io/en/latest/topics/tutorials/states_pt1.html#install-the-package
[6]: https://salt-lint.readthedocs.io/en/latest/
[7]: https://salt-lint.readthedocs.io/en/latest/rules/formatting/#207
[8]: https://salt-lint.readthedocs.io/en/latest/rules/formatting/#208