Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos-infra jitsi prep #297809

Closed
wants to merge 4 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions nixos/modules/services/web-apps/jitsi-meet.nix
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,19 @@ in
};

secureDomain.enable = mkEnableOption (lib.mdDoc "Authenticated room creation");

updateMucs = mkOption {
type = with types; attrsOf anything;
default = { };
description = "This is a very hacky work around setting and only added because prosody muc is a list.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This descriptions tells me nothing at all about how this option is expected to be used or what it does. My understanding is that this is a way to inject extra Prosody settings for a MUC, which is difficult to do otherwise due to the combination of 1. the Prosody module not using an attrset for its instances; 2. the Jitsi module being in charge of configuring Prosody itself?

  1. Have you considered refactoring Prosody to use an attrset like what your updateMucs option uses as its API? It should even be doable to keep compatibility with the old list format by auto-transforming the list into an attrset using v.domain.
  2. If this is not possible, please improve the naming and documentation here. How it's implemented should be of no concern to a user of the option and has (IMHO) no place in the documentation.

example = {
"conference.jitsi.example.com".extraModules = [
"lobby_autostart"
"muc_mam"
"vcard_muc"
];
};
};
};

config = mkIf cfg.enable {
Expand All @@ -205,7 +218,16 @@ in
tls = mkDefault true;
websocket = mkDefault true;
};
muc = [
# the piece of code bellow looks at the domain name of the attrs in the list and updates the attrs if the domain name matches.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: below

# this is done to allow adding/overriding settings in a convenient way
muc = lib.flatten (builtins.map (muc:
let
result = lib.mapAttrsToList (
n: v: if muc.domain == n then (lib.recursiveUpdate muc v) else null
) cfg.updateMucs;
in
if result == [ null ] then muc else result
) [
{
domain = "conference.${cfg.hostName}";
name = "Jitsi Meet MUC";
Expand Down Expand Up @@ -249,7 +271,7 @@ in
storage = "memory"
'';
}
];
]);
extraModules = [
"pubsub"
"smacks"
Expand Down