vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

swarm-nats: cut the comments back to what the code cannot say

Browse source 
Review: high comment-to-code ratio. Measured — 197 of 337 added lines
were comments, and net of the block that only moved it was still +135
comment against +117 code.

Kept the constraints, the rejected alternatives and the costs; dropped
everything that restated what the code plainly does, and the
slash-stripping explanation that was duplicated in the generator.
This commit is contained in:
atlas 2026-08-16 16:19:30 +02:00
commit 7c3154e3f4
1 changed files with 54 additions and 115 deletions
Download patch file Download diff file Expand all files Collapse all files

169
nix/host-modules/swarm-nats.nix
View file

@ -49,57 +49,38 @@ let
autoUserSeed = "${autoSeedDir}/callout-user.seed";
autoIssuerSeed = "${autoSeedDir}/issuer.seed";
# The runtime config directory. World-readable is correct — everything in
# it is public (the server publishes these keys to every client that
# connects) — and it deliberately does not live in the 0700 responder
# secret dir, which the `nats` user cannot traverse.
# The runtime config directory: wrapper, settings symlink and fragment.
# World-readable is correct (everything in it is public) and it is not in
# the 0700 responder secret dir, which the `nats` user cannot traverse.
#
# ⚠️ ALL THREE FILES LIVE IN ONE DIRECTORY, AND THAT IS FORCED, NOT
# TIDINESS. `include` paths are resolved with Go's `filepath.Join` against
# the config file's own directory, which **strips a leading slash** — so an
# absolute include silently becomes a relative one:
#
# include "/nix/store/…-nats.conf"
# → open agents/atlas/…/nix/store/…-nats.conf: no such file
#
# Measured, and it is invisible to every eval-level check: the wrapper
# renders perfectly and the server refuses to start. ⇒ the includes must be
# bare filenames, which means the wrapper, the settings and the fragment
# have to be siblings. (Also measured: resolution is relative to the CONFIG
# FILE, not the process CWD, so an absolute `-c` path is fine.)
# ⚠️ ONE DIRECTORY IS FORCED, NOT TIDINESS. NATS resolves an `include` with
# `filepath.Join(configDir, path)`, which strips a leading slash, so an
# absolute include silently becomes relative and is never found. The
# includes must therefore be bare filenames, i.e. siblings. Invisible to
# eval: the wrapper renders perfectly and the server refuses to start.
runtimeDir = "/var/lib/nats-callout";
runtimeWrapper = "${runtimeDir}/nats.conf";
hostRuntimeDir = "/var/lib/nixos-containers/${machine}${runtimeDir}";
natsFormat = pkgs.formats.json { };
# Upstream's rendered settings, read back out of the container's own
# evaluated config and re-rendered with the same generator — so this is the
# artifact `services.nats` would have used, not a transcription of it.
# Referencing it here is also what keeps it alive: with `ExecStart` pointed
# at a runtime path, nothing else in the closure names it, and the string
# context of this reference is what stops it being garbage-collected out
# from under a running server.
#
# ⚠️ It should be the very same store path as upstream's (same function,
# same value), but nothing depends on that being true: if it ever diverged,
# this is still a correct rendering of the same settings.
# Upstream's rendered settings, re-rendered with the same generator on the
# same value — the artifact `services.nats` would have used, not a
# transcription. ⚠️ This reference is also the only thing keeping it alive:
# `ExecStart` names a runtime path, so nothing else in the closure names
# the store file, and its string context is what stops it being
# garbage-collected out from under a running server.
renderedSettings = natsFormat.generate "nats.conf" (
config.containers.${machine}.config.services.nats.settings
);
# The two blocks whose contents depend on the callout keys, defined ONCE
# and rendered TWICE: into `services.nats.settings` with the operator's
# values, and into the runtime fragment with placeholders the generator
# substitutes.
# Defined once, rendered twice: into `settings` with the operator's values,
# and into the runtime fragment with placeholders the generator fills in.
#
# 🪤 Do NOT inline these into `settings` and hand-write the fragment. Both
# files are loaded, and the fragment is the LATER definition, so it WINS
# (measured — state/probe-3112-override.sh). A hand-written copy would
# mean a future edit to `settings` alone is silently ignored on exactly
# the hives that use auto mode: the config looks changed and behaves
# unchanged. One definition is what makes that impossible rather than
# merely unlikely.
# 🪤 Do not inline these and hand-write the fragment instead. Both files are
# loaded and the fragment is the LATER definition, so it wins — a
# hand-written copy would make a future edit to `settings` silently
# ineffective on exactly the hives that use auto mode.
calloutBlocks =
{
userKey,
@ -132,11 +113,10 @@ let
# hole. So the only way to get a live server here is a real key
# whose seed nobody but the responder holds.
#
# 🔒 That last property is what makes auto mode safe. In auto mode
# this renders as "" — which the server refuses to start on — and
# the fragment overrides it at runtime. Every field the merge
# fails to reach therefore keeps a value that FAILS CLOSED LOUDLY,
# so an incomplete merge can never leave a walk-in-able server.
# 🔒 That property is what makes auto mode safe: it renders as ""
# there, so any field the fragment fails to override keeps a value
# the server refuses to start on. An incomplete merge cannot leave
# a walk-in-able server.
${calloutAccount}.users = [ { nkey = userKey; } ];
# ⚠️ `services.nats.jetstream = true` gives the SERVER JetStream;
# an account gets it only from its own grant. Measured against a
@ -153,11 +133,9 @@ let
# responder mints credentials; it has no business holding stream
# state.
#
# ⚠️ This is also why the fragment renders the COMPLETE accounts
# block rather than only the AUTH entry: if a later definition
# replaced the block instead of merging into it, a partial fragment
# would drop this grant, and the result is a healthy-looking server
# whose every KV op fails at runtime.
# ⚠️ Also why the fragment renders the COMPLETE accounts block: if a
# later definition replaced rather than merged, a partial one would
# drop this grant and every KV op would fail on a healthy server.
${clientAccount} = {
jetstream = "enabled";
};
@ -259,21 +237,15 @@ in
Generate the auth-callout nkeys on this host instead of taking
them from `calloutUserPublicKey` / `calloutIssuerPublicKey`.
A first-boot unit mints both keypairs if they are absent, keeps
the seeds host-side at `0600`, and writes only the public halves
into a fragment the server reads. Nothing secret is evaluated,
so nothing secret reaches the nix store.
Off by default: an option says what it is, not what a deployment
shape implies. The all-local mode turns this on — see
`services.hyperhive.enableAllLocalDefaults` in
./local-defaults.nix, which is the one place that decides what
"the whole swarm is this box" means.
A first-boot unit mints both keypairs if absent, keeps the seeds
host-side at `0600`, and writes only the public halves into a
fragment the server reads. Nothing secret is evaluated, so
nothing secret reaches the nix store.
Leave it off wherever the queue and its clients are not the same
operator's problem: the seeds have to be distributed to whoever
runs the responder, and generating them here would just move
that distribution somewhere less visible.
operator's problem: the seeds must reach whoever runs the
responder, and minting them here only moves that distribution
somewhere less visible. `enableAllLocalDefaults` turns it on.
'';
};
@ -492,52 +464,28 @@ in
serverName = "swarm-nats";
port = cfg.port;
# ⚠️ Build-time validation cannot survive auto mode, and this
# is measured rather than assumed. In auto mode the keys do
# not exist until the generator runs, so this renders with
# empty strings, and `nats-server -t` rejects exactly that:
#
# User entry requires a user
# Expected callout user to be a valid public account nkey,
# got ""
#
# ⇒ leaving it on would fail the BUILD of every all-local
# hive. Upstream's own description of this option names the
# case: disable it "when the config includes other files".
#
# The check is not lost, it MOVES: the server parses both
# halves at start, with the fragment present, and refuses to
# come up if either is malformed. Later rather than at build
# time — say so in the docs; do not paper over it with an
# assertion of our own, which would be a second system
# deciding what counts as a valid config.
# In auto mode the keys are empty until the generator runs,
# and `nats-server -t` rejects that ("Expected callout user to
# be a valid public account nkey, got \"\""), so leaving this
# on fails the BUILD of every all-local hive. Upstream's own
# description names the case: disable it when the config
# includes other files. The check moves to server start.
validateConfig = !cfg.autoGenerateCallout;
# One definition, rendered here with the operator's values and
# into the runtime fragment with placeholders. See
# `calloutBlocks` for why it is not written out twice.
settings = calloutBlocks {
userKey = cfg.calloutUserPublicKey;
issuerKey = cfg.calloutIssuerPublicKey;
};
};
# Auto mode only: point the server at a wrapper that includes
# upstream's rendered settings *verbatim* plus the runtime
# fragment. The alternative — teaching this module to render the
# config itself — would throw away upstream's `settings`
# rendering, which is where all the reviewed reasoning lives.
# A wrapper that includes upstream's rendered settings verbatim
# plus the runtime fragment; rendering the config ourselves
# instead would throw away upstream's `settings`, where the
# reviewed reasoning lives. The generator writes it, because the
# includes must be siblings of the fragment (see `runtimeDir`).
#
# The wrapper is written by the generator rather than being a
# store file, because its includes must be siblings of the
# fragment (see `runtimeDir`). A server started before the
# generator has run therefore finds no config at all and refuses,
# which is the same fail-closed direction as every other failure
# mode here.
#
# `mkForce` because `services.nats` defines ExecStart inside an
# `mkMerge`; a plain override conflicts rather than winning
# (measured — state/probe-3112-mkforce.sh).
# `mkForce`: upstream defines ExecStart inside an `mkMerge`, so a
# plain override conflicts rather than wins.
systemd.services.nats.serviceConfig.ExecStart = lib.mkIf cfg.autoGenerateCallout (
lib.mkForce "${pkgs.nats-server}/bin/nats-server -c ${runtimeWrapper}"
);
@ -629,16 +577,10 @@ in
'';
};
# Auto mode: mint the callout keypairs on this host before anything
# that needs them exists.
#
# ⚠️ ON THE HOST, not inside the container, and that is the decision
# the container boundary forces. The responder is a separate unit that
# needs the user SEED; generating inside the container would trap the
# seed there and require a way back out — a new secret-export path,
# which is precisely the mechanism this issue exists to avoid
# inventing. Here, the seeds never leave a 0700 host directory and only
# the public halves cross the boundary.
# ⚠️ Minted on the HOST, not in the container, because the responder is
# a separate unit that needs the user seed: generating it inside would
# trap it there and require a secret-export path back out — the exact
# mechanism this is meant to avoid inventing. Only public halves cross.
systemd.services.swarm-nats-callout-keys = lib.mkIf cfg.autoGenerateCallout {
description = "mint the swarm queue's auth-callout nkeys";
before = [ "container@swarm-nats.service" ];
@ -694,12 +636,9 @@ in
# would silently run yesterday's config.
ln -sfn ${renderedSettings} ${lib.escapeShellArg hostRuntimeDir}/settings.conf
# The wrapper. Bare filenames on purpose: NATS resolves an include
# against the config file's own directory and strips a leading
# slash, so an absolute path here would silently fail to open.
# printf, not a heredoc: this lives inside a nix indented string, so
# a heredoc's terminator depends on nix's indentation stripping and
# would break the moment `nix fmt` re-indented this block.
# The wrapper. Bare filenames — see `runtimeDir`. printf rather than
# a heredoc, whose terminator would depend on nix's indentation
# stripping and break the next time `nix fmt` touched this block.
printf 'include "settings.conf"\ninclude "callout.conf"\n' \
> ${lib.escapeShellArg hostRuntimeDir}/nats.conf.tmp
chmod 0444 ${lib.escapeShellArg hostRuntimeDir}/nats.conf.tmp