refactor(#3393): stop restarting authelia from swarmctl

authelia now watches the users file, so the restart is redundant -- and it
was the wrong shape twice over. It could fail: a login was refused for a
user whose record was already correct on disk, with nothing in either log
implicating the reload. And it only ever worked for this writer --
swarm-authelia-bridge writes the same file and cannot restart anything,
since running unprivileged inside the container is the whole reason it may
write it at all. A reload that depends on which process did the writing is
not a reload.

--machine/--unit and their two env vars existed solely to name a
systemctl -M target, so they go with it. That drops two required settings
from the operator surface.

The three objections previously recorded against watch are all answered
now, and are kept next to the decision rather than deleted: the key is
verified against the pinned build (validate-config accepts it and rejects
a misspelling), the watch is on the directory so a rename is observed, and
partial reads are structurally impossible because every writer of this
file goes through write_atomic.

docs/tools/swarmctl-cli.md

@@ -26,8 +26,6 @@ swarm-level operator CLI
 
 * `--authelia-bin <PATH>` — authelia binary used to hash passwords. The argon2 parameters must match the verifier's, so this has to be the *configured* package rather than whatever is on `PATH`
 * `--users-file <PATH>` — Host-side path of authelia's users database — i.e. the path inside the container, prefixed with the container's root
-* `--machine <NAME>` — Machine name of the authelia container, for `systemctl -M`
-* `--unit <UNIT>` — authelia's systemd unit inside that container
 * `--store <PATH>` — Canonical user store
 
 

nix/host-modules/swarm-controller.nix

@@ -29,8 +29,9 @@ let
     # parameters baked into a hash have to match the verifier's.
     SWARMCTL_AUTHELIA_BIN = "${autheliaCfg.package}/bin/authelia";
     SWARMCTL_AUTHELIA_USERS_FILE = autheliaCfg.hostUsersFile;
-    SWARMCTL_AUTHELIA_MACHINE = autheliaCfg.machine;
-    SWARMCTL_AUTHELIA_UNIT = autheliaCfg.unit;
+    # No MACHINE/UNIT here any more: `swarmctl` no longer restarts authelia,
+    # because authelia watches the users file itself. Those two values existed
+    # solely to name a `systemctl -M` target.
   };
 
   natsCfg = config.services.hyperhive.swarm.nats;

swarmctl/src/main.rs

@@ -69,12 +69,6 @@ struct PathArgs {
     /// the container, prefixed with the container's root.
     #[arg(long, value_name = "PATH")]
     users_file: Option<PathBuf>,
-    /// Machine name of the authelia container, for `systemctl -M`.
-    #[arg(long, value_name = "NAME")]
-    machine: Option<String>,
-    /// authelia's systemd unit inside that container.
-    #[arg(long, value_name = "UNIT")]
-    unit: Option<String>,
     /// Canonical user store.
     #[arg(long, value_name = "PATH")]
     store: Option<PathBuf>,
@@ -83,8 +77,6 @@ struct PathArgs {
 struct Paths {
     authelia_bin: PathBuf,
     users_file: PathBuf,
-    machine: String,
-    unit: String,
     store: PathBuf,
 }
 
@@ -93,8 +85,6 @@ impl PathArgs {
         Ok(Paths {
             authelia_bin: path_from(self.authelia_bin, "SWARMCTL_AUTHELIA_BIN")?,
             users_file: path_from(self.users_file, "SWARMCTL_AUTHELIA_USERS_FILE")?,
-            machine: string_from(self.machine, "SWARMCTL_AUTHELIA_MACHINE")?,
-            unit: string_from(self.unit, "SWARMCTL_AUTHELIA_UNIT")?,
             store: self
                 .store
                 .or_else(|| std::env::var_os("SWARMCTL_STORE").map(PathBuf::from))
@@ -108,11 +98,6 @@ fn path_from(flag: Option<PathBuf>, env: &str) -> Result<PathBuf> {
         .with_context(|| missing(env))
 }
 
-fn string_from(flag: Option<String>, env: &str) -> Result<String> {
-    flag.or_else(|| std::env::var(env).ok())
-        .with_context(|| missing(env))
-}
-
 fn missing(env: &str) -> String {
     format!(
         "{env} is unset and no flag was given — swarmctl is installed and \
@@ -358,7 +343,34 @@ fn publish(paths: &Paths, store: &UserStore) -> Result<()> {
     write_atomic(&paths.store, &format!("{store_json}\n"))?;
     write_atomic(&paths.users_file, &rendered)?;
 
-    restart_authelia(&paths.machine, &paths.unit)
+    // No restart. Authelia is configured with
+    // `authentication_backend.file.watch`, so it re-reads this file itself.
+    //
+    // This used to shell out to `systemctl -M <machine> restart <unit>`. That
+    // was the wrong shape twice over: it could fail (it did — a login refused
+    // for a user whose record was already correct on disk, with nothing in
+    // either log implicating the reload), and it only ever worked for THIS
+    // writer. `swarm-authelia-bridge` writes the same file to create agent
+    // identities and cannot restart anything: running unprivileged inside the
+    // container is the whole reason it may write the file at all. A reload
+    // that depends on which process did the writing is not a reload.
+    //
+    // The three objections that previously kept `watch` out of this path are
+    // all now answered, and they were good objections — recorded here so the
+    // next reader does not have to re-earn them:
+    //
+    //  - "could not be verified against the pinned build" — it is now:
+    //    authelia v4.39.20's own `validate-config` accepts `watch` and
+    //    *rejects* a deliberate misspelling of it, so the key is recognised
+    //    rather than silently swallowed.
+    //  - "does the watch survive the rename(2) used above" — yes: authelia
+    //    watches the containing *directory*, so a rename into it is observed.
+    //    A watch on the old inode alone would indeed have missed it.
+    //  - "can it observe a partially written file" — it cannot, and that is
+    //    structural rather than a debounce we are trusting: every writer of
+    //    this file goes through `write_atomic` below, so no partial content is
+    //    ever visible under the final name. (Authelia debounces as well.)
+    Ok(())
 }
 
 /// Load the canonical store, or start an empty one if this deployment has
@@ -456,29 +468,6 @@ fn parse_field(stdout: &str, marker: &str) -> Option<String> {
         .filter(|value| !value.is_empty())
 }
 
-/// authelia re-reads its file backend at startup, so a users change needs
-/// a restart.
-///
-/// The file watcher (`authentication_backend.file.watch`) would remove
-/// this step entirely, and is deliberately not relied on: it could not be
-/// verified against the pinned build, and it carries two unknowns —
-/// whether the watch survives the `rename(2)` used above, and whether it
-/// can observe a partially written file. An explicit restart assumes
-/// nothing.
-fn restart_authelia(machine: &str, unit: &str) -> Result<()> {
-    let status = Command::new("systemctl")
-        .args(["-M", machine, "restart", unit])
-        .status()
-        .context("running systemctl")?;
-    if !status.success() {
-        bail!(
-            "restarting {unit} in {machine} failed ({status}); the users file is \
-             already written, so re-running the restart by hand completes the change"
-        );
-    }
-    Ok(())
-}
-
 /// Replace `path`'s contents atomically, preserving the existing owner
 /// and mode.
 ///

swarmctl/src/users.rs

@@ -495,7 +495,7 @@ mod tests {
                 ..UserUpdate::default()
             },
         )
-        .expect_err("a no-op must not restart authelia");
+        .expect_err("a no-op must not rewrite the user database");
         assert!(err.to_string().contains("nothing to change"), "{err}");
     }