diff --git a/docs/tools/swarmctl-cli.md b/docs/tools/swarmctl-cli.md index d38368fc..68acc6ce 100644 --- a/docs/tools/swarmctl-cli.md +++ b/docs/tools/swarmctl-cli.md @@ -26,8 +26,6 @@ swarm-level operator CLI * `--authelia-bin ` — 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 ` — Host-side path of authelia's users database — i.e. the path inside the container, prefixed with the container's root -* `--machine ` — Machine name of the authelia container, for `systemctl -M` -* `--unit ` — authelia's systemd unit inside that container * `--store ` — Canonical user store diff --git a/nix/host-modules/swarm-controller.nix b/nix/host-modules/swarm-controller.nix index 3e28e43a..06c42b5f 100644 --- a/nix/host-modules/swarm-controller.nix +++ b/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; diff --git a/swarmctl/src/main.rs b/swarmctl/src/main.rs index b3ff4dc2..fd0b88b3 100644 --- a/swarmctl/src/main.rs +++ b/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, - /// Machine name of the authelia container, for `systemctl -M`. - #[arg(long, value_name = "NAME")] - machine: Option, - /// authelia's systemd unit inside that container. - #[arg(long, value_name = "UNIT")] - unit: Option, /// Canonical user store. #[arg(long, value_name = "PATH")] store: Option, @@ -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, env: &str) -> Result { .with_context(|| missing(env)) } -fn string_from(flag: Option, env: &str) -> Result { - 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 restart `. 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 { .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. /// diff --git a/swarmctl/src/users.rs b/swarmctl/src/users.rs index 8d38e911..400d89a3 100644 --- a/swarmctl/src/users.rs +++ b/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}"); }