diff options
author | Maximilian Bosch <maximilian@mbosch.me> | 2023-09-20 12:32:04 +0200 |
---|---|---|
committer | Maximilian Bosch <maximilian@mbosch.me> | 2023-09-20 15:48:03 +0200 |
commit | d004375485fd4bcb0532ff2ca5eef3639bf1f9e0 (patch) | |
tree | f73909b3e705ad32a02733f7b8fec86dc1e9f833 /nixos | |
parent | bd17fe35505e9684d8234b8c09cef7715639db5c (diff) | |
download | nixlib-d004375485fd4bcb0532ff2ca5eef3639bf1f9e0.tar nixlib-d004375485fd4bcb0532ff2ca5eef3639bf1f9e0.tar.gz nixlib-d004375485fd4bcb0532ff2ca5eef3639bf1f9e0.tar.bz2 nixlib-d004375485fd4bcb0532ff2ca5eef3639bf1f9e0.tar.lz nixlib-d004375485fd4bcb0532ff2ca5eef3639bf1f9e0.tar.xz nixlib-d004375485fd4bcb0532ff2ca5eef3639bf1f9e0.tar.zst nixlib-d004375485fd4bcb0532ff2ca5eef3639bf1f9e0.zip |
nixos/matrix-synapse: refactor assertions for missing listener resources
While reviewing other changes related to synapse I rediscovered the `lib.findFirst (...) (lib.last resources)` hack to find a listener supporting the `client` resource. We decided to keep it that way for now a while ago to avoid scope-creep on the RFC42 refactoring[1]. I wanted to take care of that and forgot about it. Anyways, I'm pretty sure that this is bogus: to register a user, you need the `client` API and not a random listener which happens to be the last one in the list. Also, you need something which serves the `client` API to have the entire synapse<->messenger interaction working (whereas `federation` is for synapse<->synapse). So I decided to error out if no `client` listener is found. A listener serving `client` can be defined in either the main synapse process or one of its workers via `services.matrix-synapse.workers`[2]. However it's generally nicer to use assertions for that because then it's possible to display multiple configuration errors at once and one doesn't have to chase one `throw` after another. I decided to also error out when using the result from `findFirst` though because module assertions aren't thrown necessarily when you evaluate a single config attribute, e.g. `config.environment.systemPackages` which depends on an existing client listener because of `registerNewMatrixUser`[3]. While at it I realized that if `settings.instance_map` is wrongly configured, e.g. by settings.instance_map = mkForce { /* no `main` in here */ } an `attribute ... missing` error will be thrown while evaluating the worker assertion. [1] https://github.com/NixOS/nixpkgs/pull/158605#discussion_r815500487 [2] This also means that `registerNewMatrixUser` will still work if you offload the entire `client` traffic to a worker. [3] And getting a useful error message is way better for debugging in such a case than `value is null while a set was expected`.
Diffstat (limited to 'nixos')
-rw-r--r-- | nixos/modules/services/matrix/synapse.nix | 39 |
1 files changed, 23 insertions, 16 deletions
diff --git a/nixos/modules/services/matrix/synapse.nix b/nixos/modules/services/matrix/synapse.nix index 554e9ca2ecc3..5cce36f41e50 100644 --- a/nixos/modules/services/matrix/synapse.nix +++ b/nixos/modules/services/matrix/synapse.nix @@ -15,26 +15,26 @@ let usePostgresql && (!(args ? host) || (elem args.host [ "localhost" "127.0.0.1" "::1" ])); hasWorkers = cfg.workers != { }; + listenerSupportsResource = resource: listener: + lib.any ({ names, ... }: builtins.elem resource names) listener.resources; + + clientListener = findFirst + (listenerSupportsResource "client") + null + (cfg.settings.listeners + ++ concatMap ({ worker_listeners, ... }: worker_listeners) (attrValues cfg.workers)); + registerNewMatrixUser = let - isIpv6 = x: lib.length (lib.splitString ":" x) > 1; - listener = - lib.findFirst ( - listener: lib.any ( - resource: lib.any ( - name: name == "client" - ) resource.names - ) listener.resources - ) (lib.last cfg.settings.listeners) cfg.settings.listeners; - # FIXME: Handle cases with missing client listener properly, - # don't rely on lib.last, this will not work. + isIpv6 = hasInfix ":"; # add a tail, so that without any bind_addresses we still have a useable address - bindAddress = head (listener.bind_addresses ++ [ "127.0.0.1" ]); - listenerProtocol = if listener.tls + bindAddress = head (clientListener.bind_addresses ++ [ "127.0.0.1" ]); + listenerProtocol = if clientListener.tls then "https" else "http"; in + assert assertMsg (clientListener != null) "No client listener found in synapse or one of its workers"; pkgs.writeShellScriptBin "matrix-synapse-register_new_matrix_user" '' exec ${cfg.package}/bin/register_new_matrix_user \ $@ \ @@ -44,7 +44,7 @@ let "[${bindAddress}]" else "${bindAddress}" - }:${builtins.toString listener.port}/" + }:${builtins.toString clientListener.port}/" ''; defaultExtras = [ @@ -938,6 +938,13 @@ in { config = mkIf cfg.enable { assertions = [ { + assertion = clientListener != null; + message = '' + At least one listener which serves the `client` resource via HTTP is required + by synapse in `services.matrix-synapse.settings.listeners` or in one of the workers! + ''; + } + { assertion = hasLocalPostgresDB -> config.services.postgresql.enable; message = '' Cannot deploy matrix-synapse with a configuration for a local postgresql database @@ -969,13 +976,13 @@ in { ( listener: listener.port == main.port - && (lib.any (resource: builtins.elem "replication" resource.names) listener.resources) + && listenerSupportsResource "replication" listener && (lib.any (bind: bind == main.host || bind == "0.0.0.0" || bind == "::") listener.bind_addresses) ) null cfg.settings.listeners; in - hasWorkers -> (listener != null); + hasWorkers -> (cfg.settings.instance_map ? main && listener != null); message = '' Workers for matrix-synapse require setting `services.matrix-synapse.settings.instance_map.main` to any listener configured in `services.matrix-synapse.settings.listeners` with a `"replication"` |