From fd4b79f04ae2bc733e817b0cc237555be95971d9 Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 16 Aug 2026 20:10:55 +0200 Subject: [PATCH] refactor(#3354): make the derived hive clients a definition of the option The client list was `cfg.oidc.clients ++ hiveClients`, where the first half comes through the submodule and the second was a raw attrset from this module's `let` block. That list is only half-typed: a field added to the submodule exists on the declared entries and not on the derived ones, so reading it plainly is an eval error the moment hive identities are on. The operator asked whether it should be uniformly typed instead of guarding each read, and it should. The hive identities are now declared the same way an operator declares a client, so the module system applies the submodule to them and every option's default is present. Downstream reads one uniformly-typed list and the guard added for the field that broke is gone with it. --- nix/host-modules/swarm-authelia.nix | 62 ++++++++++++++++------------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/nix/host-modules/swarm-authelia.nix b/nix/host-modules/swarm-authelia.nix index d40da6d2..4bc72f10 100644 --- a/nix/host-modules/swarm-authelia.nix +++ b/nix/host-modules/swarm-authelia.nix @@ -79,6 +79,14 @@ let # per service from it. Were the queue to declare this list, the next # consumer would collide on the same client id — and only at the # moment it landed. + # + # Fed to the option as a DEFINITION in the config block below, rather + # than appended to the declared list downstream. That is what puts it + # through the submodule: one list, one type, every option's default + # present. Appending a raw attrset instead left the list half-typed — + # and a field later added to the submodule then existed on the + # declared entries and not on these, which is an eval error reachable + # only once hive identities are on. hiveClients = lib.mapAttrsToList (name: _: { id = "hive-${name}"; description = "HyperHive hive ${name}"; @@ -86,21 +94,18 @@ let redirectUris = [ ]; }) hyperhiveCfg.swarm.hives; - # Everything downstream renders and mints from this, not from the - # declared list alone. - allClients = cfg.oidc.clients ++ lib.optionals cfg.oidc.hiveIdentities hiveClients; - # authelia refuses to start with an OIDC provider that has no clients, # so the provider is derived from the client list rather than carrying # its own `enable`: one fact, and it cannot contradict itself. An empty # list is the default, which makes every hive that has not opted in # byte-identical to before. # - # ⚠️ Derived from `allClients`, so hive identities can turn the provider - # on by themselves. That is only reachable where the queue is already - # enabled (`hiveIdentities` defaults to it) — and a queue-enabled hive - # already contributes a client, so no existing deployment flips. - oidcEnabled = allClients != [ ]; + # ⚠️ The derived hive identities are definitions of this same option, + # so they can turn the provider on by themselves. That is only + # reachable where the queue is already enabled (`hiveIdentities` + # defaults to it) — and a queue-enabled hive already contributes a + # client, so no existing deployment flips. + oidcEnabled = cfg.oidc.clients != [ ]; # Secrets that are 64 random bytes of hex and nothing more. The OIDC # hmac key joins them; the issuer key does not (see below — it is RSA). @@ -144,19 +149,12 @@ let printf -- " client_secret: '%s'\n" "$(cat ${lib.escapeShellArg "${clientsDir}/${c.id}.digest"})" printf -- ' authorization_policy: one_factor\n' '' - # ⚠️ `or null`, and it is not defensive noise: `allClients` is NOT - # uniformly submodule-typed. `cfg.oidc.clients` entries come through - # the submodule and carry every option's default; `hiveClients` is a - # raw attrset built in this file's `let` block and carries only the - # four fields written there. So a field added to the submodule exists - # on one half of this list and not the other, and reading it plainly - # is an eval error the moment `hiveIdentities` is on. - # - # That is exactly how this broke: `tokenEndpointAuthMethod` was added - # to the submodule and read here, which is fine for a declared client - # and fatal for a derived one. Anything read here needs `or` unless - # every producer is known to set it. - + lib.optionalString ((c.tokenEndpointAuthMethod or null) != null) '' + # Read plainly, and that is a property of the list rather than of + # this line: every entry reaching here is a definition of + # `oidc.clients`, so the module system has applied the submodule and + # each option's default is present. A derived entry that names only + # the fields it cares about still arrives with the rest filled in. + + lib.optionalString (c.tokenEndpointAuthMethod != null) '' printf -- ' token_endpoint_auth_method: %s\n' ${lib.escapeShellArg c.tokenEndpointAuthMethod} '' + ( @@ -231,7 +229,7 @@ let ${lib.concatMapStrings (c: '' mint ${lib.escapeShellArg c.id} - '') allClients} + '') cfg.oidc.clients} # Re-rendered every boot, deliberately: the secret is minted once, # but the metadata around it (a new redirect URI, a renamed client) @@ -242,7 +240,7 @@ let printf -- 'identity_providers:\n' printf -- ' oidc:\n' printf -- ' clients:\n' - ${lib.concatMapStrings renderClient allClients} + ${lib.concatMapStrings renderClient cfg.oidc.clients} } > ${lib.escapeShellArg "${clientsFile}.tmp"} chmod 0600 ${lib.escapeShellArg "${clientsFile}.tmp"} mv ${lib.escapeShellArg "${clientsFile}.tmp"} ${lib.escapeShellArg clientsFile} @@ -550,6 +548,14 @@ in }; config = lib.mkIf (hyperhiveCfg.enable && cfg.enable) { + # The derived half of the client list, declared the same way an + # operator declares one. Everything downstream then reads a single + # uniformly-typed `cfg.oidc.clients` and cannot tell the two apart — + # including the assertions below, which is why a hive named `x` + # colliding with a declared `hive-x` is caught rather than rendered + # twice. + services.hyperhive.swarm.authelia.oidc.clients = lib.mkIf cfg.oidc.hiveIdentities hiveClients; + # A redirect URI on a machine client is not harmless-but-unused: it # means whoever wrote it believes a browser is involved. Failing here # is how that belief gets corrected at the point it was expressed, @@ -562,7 +568,7 @@ in + "kind = \"machine\" but declares redirectUris. A client_credentials " + "client has nobody to redirect; drop the URIs or make it " + "kind = \"interactive\"."; - }) allClients + }) cfg.oidc.clients # Two clients sharing an id renders two YAML entries under one name. # Newly reachable now that part of the list is DERIVED: a hive called # `x` and a service client called `hive-x` never met before. Authelia @@ -570,12 +576,14 @@ in # sources here is the cheaper failure. ++ [ { - assertion = lib.length (lib.unique (map (c: c.id) allClients)) == lib.length allClients; + assertion = lib.length (lib.unique (map (c: c.id) cfg.oidc.clients)) == lib.length cfg.oidc.clients; message = "services.hyperhive.swarm.authelia: duplicate OIDC client id(s): " + lib.concatStringsSep ", " ( lib.unique ( - lib.filter (id: lib.count (x: x == id) (map (c: c.id) allClients) > 1) (map (c: c.id) allClients) + lib.filter (id: lib.count (x: x == id) (map (c: c.id) cfg.oidc.clients) > 1) ( + map (c: c.id) cfg.oidc.clients + ) ) ) + ". Hive identities are named `hive-` from "