Repository navigation
(Draft) NixOS vars - #547171
(Draft) NixOS vars#547171prescientmoon wants to merge 158 commits into
Conversation
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/modular-services-meeting-6-2026-08-07/79408/1 |
|
There is now NixOS/nix#16300 |
|
Currently we cannot detect if a generator without dependents (a root generator) fails regenerating. To solve this we could instead do: Before generating, delete the metadata file, after it finished generating update the secret with a meta file containing a transaction ID.
|
The transaction ID is pretty much equivalent to the ID we already store in the metadata. We already track parent staleness! There's two reasons a root generator might regenerate:
If we're in the latter case, then a failing generator will not break anything (the files are set after the generator runs). If we're in the former, then we'll keep regenerating as long as the user passes the same flags in future invocations. Whether we should keep regenerating even if the user removes those flags is questionable (I'd argue we shouldn't, hence why that's currently not the case). Implementing the other behavior (remembering the failure even if the user stops passing the flags) wouldn't require any big change (we'd simply invalidate the metadata before running the generator). I will say, this behavior isn't unique to root / dependency-less generators! Consider the following scenario: the user defines secrets A, B, and C. At first, B depends on A, and generation goes perfectly fine. Then, the user adds C as another dependency of B. The CLI detects the staleness of B (it's dependency list changed!), so an update is triggered. Say the update fails in this case, and the user once again removes C from B's dependencies. Since the dependency list matches the on-disk metadata, the CLI will now no longer attempt to regenerate B (even though it failed in the past!). This happens for the same reason the root-generator scenario you described happens, although I think it is what one would expect! |
There was a problem hiding this comment.
Thanks for contributing to the documentation.
Follow the documentation style guide, especially:
- Show, don’t tell. Start with a minimal working example. Explain after the code.
- Avoid meta-commentary. Don’t write "This section explains how to…".
- Use present tense, imperative mood, and active voice. For example: "Run the command," not "The user should run the following command."
- Avoid vague references like "this" when the referent is unclear. Write "This command creates a folder," not "This will create a folder."
- Be direct. Avoid "should", "might", "typically", and "usually".
- Cut filler words like "simply", "just", "easily", and "basically". Use "to" instead of "in order to".
For larger changes, such as adding or removing whole sections, ask the NixOS documentation team for review.
b898670 to
f6de203
Compare
I am not sure whether this is actually related / relevant. It seems to be more about providing Nix with the credentials that it needs than general secret/variable management for NixOS systems... |
PerchunPak
left a comment
There was a problem hiding this comment.
Nice talk! Sorry for leaving only nitpicks, don't really have time to review throughtly
Signed-off-by: prescientmoon <git@moonythm.dev>
Signed-off-by: prescientmoon <git@moonythm.dev>
…igs from Nix Signed-off-by: prescientmoon <git@moonythm.dev>
Signed-off-by: prescientmoon <git@moonythm.dev>
Signed-off-by: prescientmoon <git@moonythm.dev>
Signed-off-by: prescientmoon <git@moonythm.dev>
nixos-vars: nixos-vars: - fix naming inconsistencies (run & script, "backendModule" not being nixos-vars: clear enough (is it a generator or a prompt backend?) etc) nixos-vars: - make jsonify idempotent nixos-vars: - make jsonify accept both target and host nixpkgs instances, reusing nixos-vars: the one the given NixOS config has been built with in case none has been nixos-vars: given Signed-off-by: prescientmoon <git@moonythm.dev>
The entry was apparently in the toplevel gitignore. Thank you @PerchunPak!
a993a97 to
068e85d
Compare
|
Is there a timeline for getting this merged? |
I would expect it after the next stable release or branch off if we don't find any major issues :) |
DavHau
left a comment
There was a problem hiding this comment.
Thanks a lot for working on this! I finally got around having a look at it.
Stabilization Roadmap
I think this project is missing a plan on how it gets stabilized.
If I'm not mistaken this is currently in a POC state with nobody using it. There is no usable backend code and it is hidden behind a nixpkgs PR branch. Naturally, as there hasn't been any exposure to real use cases, there will be many issues with it and once this is in nixpkgs upstream, fixing the issues just creates unnecessary churn and maybe even prevent it from becoming as good as it could, since contributors might be reluctant on breaking backwards compat.
I would suggest to put this into a separate repo, add a usable backend, and let people use is as a dependency to their nix projects, then iterate and fix all the issues, and once it is mature enough, merge it into nixpkgs.
There have been so many surprises when we developed vars in clan-core, which only came up once the feature was used in real projects, and I think the same will happen to this implementation as well. Putting it into nixpkgs already will inevitably make a lot of people depend on it too soon, which will make it hard to change it in meaningful ways.
This is my most important point here. Everything else that follows is not that relevant until this is answered.
The rest of my review is mostly about high level issues. I haven't scrutinized every implementation detail, because I don't think this is the most useful thing to do here right now. I believe getting some exposure to real users would be much more effective in gaining confidence in this project than having a bunch of developers approving code that has never been used.
Testing and Type checking
The python code has no unit tests. Even if the code is not that large, this is a bad practice as the code will grow in the future and this situation invites people to add more untested code to it. Untested code is usually written in a hard-to-test style which is hard to fix later.
Some types are partially wrong. -> Setup a type checker that actually runs in CI.
optimally this code would survive a ruff and mypy check.
currently mypy reports 82 errors in 6 files
Unsafe example code
Large parts of the code added here seems to be unsafe example code that should not be used, yet it is exposed under a location under modulesPath that can easily be imported. I suggest moving all example code under nixos/tests/nixos-secrets and mention this in the docs. This would clearly underlines the intent that it should not be used.
Blind spots
This section is about known problems of the original vars implementation in clan-core, which are ignored by the implementation of this PR.
I'm aware that this PRs implementation intentionally wants to be lean and doesn't yet have the feature set that would trigger some of these issues, but still it is worthwhile mentioning them, because they will re-appear as soon as more features are added, if ignored by the fundamental architecture.
Performance Issues
In short, the way vars were implemented in clan-core introduced a significant performance issue, because deploying one machine required evaluating all generators of all other machines. Since generators can depend on any other nixos option or packages, the worst case of this scenario is having to evaluate all toplevel's of all machines whenever deploying any machine.
The source of all evil is a secret sharing feature that this PR's implementation doesn't have yet, but since this is a highly needed feature for any infrastructure with more than one machine it will inevitably be added sooner or later. Therefore this implementation should have at least a viable plan on how this can be added without re-introducing the performance bottleneck of clan vars.
Example of shared vars:
You have multiple CI builders that all need the same API key to push to some cache. The key is the same across all machines. You do not want to re-generate or re prompt it for each individual machine.
Example of shared vars with dependents:
You have multiple machines within the same VPN. Your VPN has a network ID which is shared across all machines. You only want to generate it once for all machines. If you ever change your network ID, all machines have to re-roll their keys (their generators depend on that shared network ID file).
The problem we have in clan with this is the following: Whenever a shared secret gets re-generated, we need to invalidate all dependant secrets of all machines, therefore we must know the graph of the dependents across all machines at once, which required us to evaluate all secrets across all machines on every single deployment.
Evaluation feedback cycles
In clan vars, we made it so that non-secret (eg. public) generated files became part of the repository and they could be read inside the nixos evaluation itself.
This allowed to create evaluation feedback cycles, where the properties or even existence of one generated file can be influenced by the existence of another generated file. This means a single evaluation of generators cannot guarantee you to know the whole final graph of of generators.
While this sounds like an edge case. It happened to use in practice many times over by accident to create these loops. Those usually surface as one of the following hard to debug defects:
- generating vars crashes, because the evaluation of a generator depends on a another generated file that doesn't exist yet
- generating all vars requires multiple (unknown amount of) rounds of vars evaluation and generation, because each generation leads to new vars being activated in the next evaluation.
This is pretty bad. We tried to prevent this by adding some mitigations, but those were quite expensive in terms of compute and potentially not reliable.
The best would be to explicitly discourage reading vars back into the nixos config. Backends should be discouraged to support reading back any generated state during evaluation time, to prevent these loops from happening.
Some minor notes I took while reading
- what is
storeinsecrets.store.<secret> =(unclear at the point in the docs where it first appears) - spelling and grammar mistakes (maybe pipe everything through an LLM to fix those)
configis written instead ofconfig.nixin a bunch of examples--set user=manual: why ismanuala file or directory, why not prompt the user?
| # | ||
| # We're only doing it this way in order to keep the example portable | ||
| # (absolute paths would break it on other people's machines!). | ||
| settings.store.age.identity.host = toString ../common/key-host.txt; |
There was a problem hiding this comment.
Paths to the local filesystem should not be put into nix options. This becomes important as soon as the repo is used by more than one admin. Different people will use different keys and store them under different locations.
As a rule of thumb, any file path that is not committed to the repo should be set via an environment variable instead of a nix option. Changing this would also make your do *not* do this disclaimer unnecessary.
Also, storing important key material in plain text should be discouraged. Better to use ssh public keys in the example. Age supports them natively and they can be password protected.
There was a problem hiding this comment.
Yeah, that's kind of the reason all the comments are there :p I will maybe try to make the example use env vars just to show people how to do it, I guess...
There was a problem hiding this comment.
Now that I think about it, the disclaimer would still have to be there for the target machine's key...
| backends.defaults.prompt = "simple"; | ||
|
|
||
| settings.store.age.publicKeys = [ | ||
| "age13ar5t7vvsssmckjhjtngy3p5y0v4k896ecjrxveql9ysu8gxhe9sdar3k3" # Host |
There was a problem hiding this comment.
Best to use ssh public keys here instead. Most people already have ssh keys, age supports them natively and ssh key material is password protected most often. This is much better than storing raw key material on disk which is what the native age key generator does by default.
There was a problem hiding this comment.
I agree in principle, although this is an example backend that is explicitly asking not to be used in production (and is mostly there to showcase the API), so I'm not sure it particularly matters. I'll consider switching it over to using SSH keys though.
| type = enum [ | ||
| "hidden" | ||
| "line" | ||
| "multiline" |
There was a problem hiding this comment.
multiline-hidden is missing here
There was a problem hiding this comment.
Hmm, how would a backend handle this in practice? The idea with multiline was that a backend would, for example, open the file in $EDITOR.
Still, if multiline-hidden is something people commonly need, I'd be open to adding it.
There was a problem hiding this comment.
openssh private keys are encoded into multiple lines for example. So if you'd want to paste one for whatever reason, it would need that input mode. clan-core implements that by having the user paste into a blind (non echoing) terminal, then finish with Ctrl+D. The $EDITOR should not be used for secrets as it might cache it or heep a history of some sorts.
There was a problem hiding this comment.
Hmm, I see the point regarding $EDITOR. I think sops-nix and the like do use $EDITOR for this kind of stuff though. Is that just a bad practice that's already common place?
|
Oops, closed this by accident, then re-opened |
|
Hi @DavHau, thank you for the review! Some unordered notes (not covering every point):
|
That's a good start but a larger user base would be much better.
Nix flakes feature was added with that same idea in mind. It became hard to change very quickly, despite its experimental status. Statuses don't mean anything. Exposure is what locks you in. Merging this into nixpkgs is the wrong kind of exposure for the state the project is in, I believe, and probably still will be after being tested by just 2 people IMO.
I don't see how this solves the issue I was describing. You still need means of declaring something as I'm almost certain that you will encounter some unforeseen problems once you actually implement this. One more reason to have a fully working backend and a separate repo to iterate on.
nice 👍 |
|
As someone who's been waiting almost 2 years for this feature, I'm ready to consume this however; from nixpkgs (after merge), from this PR directly, or from a different repository this gets moved to for testing. I agree with @DavHau that lock-in from exposure could block any refinements needed here. That said, the same thing was done in #370444 where development moved to Lassulus/vars. I hope the work doesn't fizzle out once again. |
This will let the argument be extensible in the future.
I thought I had gotten a warning about the name changing, but apparently not!
|
@DavHau Re: sharing Can you elaborate a bit more on "Whenever you change something that is shared there are potential side effects on other files of other machines that you cannot see or plan without evaluating all of them."? It is true that changing something in a "shared" secret's declaration can lead to the configurations of other machines potentially changing as well. Nonetheless, such a change is meaningless unless the new versions of the other configurations are also to be deployed. But deploying said configurations would require evaluating them anyway, so I'm not sure where the issue lies... Is the issue that there's no way to do it incrementally? |
This is exactly the problem. You have to evaluate all machines, which can take several minutes. Depending how many machines you have, cost scaling linearly if you have more nixos configurations in lets say a flake. I'd like to see examples how setups with more than one machine work.
The issue is performance, but also incremental deployments. If you have many machines you cannot expect all of them to evaluate or update at the same time. I'd also like to use |
I've been working on a re-implementation of NixOS vars as part of the Google Summer of Code project, together with my mentor @lassulus. For info about the API, check the README included with this PR.
Things done
This PR implements a basic CLI for working with vars, a NixOS module that can be used together with the CLI, and some simple examples. There's also a simple NixOS test.
A bunch of concepts in the API have temporary names. Feel free to suggest better ones!