NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.58k stars 13.73k forks source link

nixos/prosody: rewrite module #260006

Open aanderse opened 11 months ago

aanderse commented 11 months ago

Description of changes

this branch/PR is a starting point for a prosody module rewrite i don't have the energy/motivation to push this PR through all by myself and it is still missing quite a bit... but my hope is that others would be interested in contributing so we could eventually get this merged

Things done

cc @toastal @ju1m @martinetd

toastal commented 11 months ago

it is still missing quite a bit

@aanderse Do we know what these things are?

aanderse commented 11 months ago

i got motivated and ended up writing the rest of the code required! i am running this code as of a few minor changes ago on my server and it looks to be fine

so what work remains?

who can contribute to the above list?

toastal commented 11 months ago

Is there an easy way to test out NixOS modules without building the entire branch? I know they don’t overlay. -- toastal ไข่ดาว | https://toast.al PGP: 7944 74b7 d236 dab9 c9ef e7f9 5cce 6f14 66d4 7c9e

SuperSandro2000 commented 11 months ago

Is there an easy way to test out NixOS modules without building the entire branch? I know they don’t overlay.

please see https://github.com/NixOS/nixpkgs/blob/master/nixos/README.md

SuperSandro2000 commented 11 months ago

FYI #260551

SuperSandro2000 commented 11 months ago

also pinging @astro @oxapentane

martinetd commented 11 months ago

Is there an easy way to test out NixOS modules without building the entire branch? I know they don’t overlay.

please see master/nixos/README.md

Hmm, "testing changes" there seems to suggest rebuilding with a local nixpkgs tree so presumably "building the entire branch", is there another way I missed?

I generally copy the service file to test in my /etc/nixos tree, update services.foo to anything else that doesn't conflict with something in nixpkgs and use that (adding the file to imports somewhere); but I'd also be curious to know if there's a better way of doing it; in this particular case there's a lot to rename so it wasn't exactly practical, see: https://cgit.notk.org/asmadeus/nixos-config.git/commit/?id=cff13c93469de890843b1245d3946f25c12dba19 Probably best to just use a nixpkgs fork at this point...

aanderse commented 11 months ago

i think we're getting close to feature freeze which means a new release is around the corner! has anybody had a chance to test this yet? it worked fine on my server so as long as you have backups i feel like anyone should be fine to test i ask because if this is all tested and approved we can merge right after the next NixOS release

btw... anyone willing to write release notes?

ju1m commented 10 months ago

@aanderse, it would be great to allow the new http_file_share module to replace http_upload in the xmppComplianceSuite checks. Below is a suggestion of how it could be done. Should you prefer to leave it for another PR, I could also later rebase https://github.com/NixOS/nixpkgs/pull/171855 once this PR has been merged.

diff --git a/nixos/modules/services/networking/prosody.nix b/nixos/modules/services/networking/prosody.nix
index 2d1baab7fad4..9a3c782ac908 100644
--- a/nixos/modules/services/networking/prosody.nix
+++ b/nixos/modules/services/networking/prosody.nix
@@ -2,7 +2,7 @@
 let
   inherit (lib) isAttrs isBool isList isInt isString;
   inherit (lib) mkChangedOptionModule mkDefault mkEnableOption mkForce mkIf mkMerge mkOption mkRenamedOptionModule;
-  inherit (lib) all any attrValues boolToString concatStringsSep filterAttrs genAttrs hasPrefix listToAttrs literalExpression mapAttrsToList mdDoc optionals optionalAttrs optionalString types unique;
+  inherit (lib) all any attrValues boolToString concatStringsSep elem filterAttrs genAttrs hasPrefix listToAttrs literalExpression mapAttrsToList mdDoc optionals optionalAttrs optionalString types unique;

   config' = config;
   cfg = config.services.prosody;
@@ -514,12 +514,12 @@ in

     assertions =
       let
-        checkForModule = mod: b: any (e: e.module == mod) (attrValues b.components);
+        checkForModule = mods: b: any (e: elem e.module mods) (attrValues b.components);
         virtualHosts = filterAttrs (_: v: v.settings.enabled) cfg.virtualHosts;

         # ensure a given module (e.g. muc or http_upload) is applied or available to every virtual host
-        mkAssertion = module: message: {
-          assertion = cfg.xmppComplianceSuite -> checkForModule module cfg || all (checkForModule module) (attrValues virtualHosts);
+        mkAssertion = modules: message: {
+          assertion = cfg.xmppComplianceSuite -> checkForModule modules cfg || all (checkForModule modules) (attrValues virtualHosts);
           message = message + ''

             Having a server not XEP-0423-compliant might make your XMPP
aanderse commented 10 months ago

@ju1m that sounds great! we will merge this PR in the next few weeks after branch off for 23.11 and then right after that can you make a PR to do this?

ju1m commented 10 months ago

@aanderse ok, I'll update https://github.com/NixOS/nixpkgs/pull/171855 to use the above patch once this PR has been merged.

aanderse commented 9 months ago

i will try to find some motivation to rebase this over the break is anyone keen to test? write release notes? it would be nice to get this merged soon

toastal commented 9 months ago

@aanderse I’d be interested in testing it out so long as it’s easy to test (it’s normally a pain since you can’t just drop in a overlay)

nixos-discourse commented 9 months ago

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixos-module-of-prosody-is-missing-crucial-functionalities-making-it-unusable/37168/4

aanderse commented 9 months ago

hey all - looks like some merge conflicts have popped up and they probably require more mental energy than i can give at this point. unfortunately the longer this PR sits the more likely additional merge conflicts are... if someone wants to chip in and either take over this PR, or resolve merge conflicts, i would appreciate it. i'm not sure if i have enough motivation to continue this and might just publish as a flake otherwise.

so if anyone wants to contribute to getting this merged please say so. sorry for the hassle.

amalgame21 commented 9 months ago

I modified the jitsi-meet.nix to work with your rewritten prosody module, with reference to https://github.com/jitsi/jitsi-meet/blob/master/doc/debian/jitsi-meet-prosody/prosody.cfg.lua-jvb.example and https://github.com/NixOS/nixpkgs/blob/nixos-23.11/nixos/modules/services/web-apps/jitsi-meet.nix.

It should be working just fine with your module, but I do not know if it is the correct way to set it. Please take a look. https://github.com/amalgame21/jitsimeet-nixos-module/blob/main/jitsi-meet-aanderse-custom.nix

I still cannot get the environmentFile for turnserver secret to work, because it conflicts with the jitsi-meet.nix module which have environmentFile hardcoded. Now I just set the secret directly, but the secret is present in /nix/store and it may have some security risk.

@aanderse

amalgame21 commented 9 months ago

The environmentFile can be changed to environmentFiles and type can be changed from nullOf path to listOf path to solve the conflict with jitsi-meet.nix

https://github.com/amalgame21/jitsimeet-nixos-module/blob/main/prosody-environmentFiles.nix

Sorry I don't know how to use github :-(

amalgame21 commented 9 months ago

Wrote some logic for JWT (Shared Secret Validation only) in jitsi-meet.nix. https://github.com/amalgame21/jitsimeet-nixos-module/blob/main/jitsi-meet-aanderse-JWT-custom.nix

toastal commented 8 months ago

@amalgame21 Those files looked pretty good to me. Do you plan to take over this merge request? I would love to see this be moved forward before the Nix hackathon next month to work on some tangential XMPP stuff.

amalgame21 commented 7 months ago

@amalgame21 Those files looked pretty good to me. Do you plan to take over this merge request? I would love to see this be moved forward before the Nix hackathon next month to work on some tangential XMPP stuff.

As a newcomer of Nix (about 2 months), actually I cannot understand a lot of code in the module and I was just editing some basic things. I think I am not capable of this task. Just feel free to use my code in jitsi-meet if someone are interested, I hope this can merge too.

amalgame21 commented 5 months ago

Hi, for the feature of ENV_ for secrets in environmentFile I think it is great idea to add those sercets without hardcode them in nix store. However I cannot get it insert into /etc/prosody/prosody.cfg (it becomes literally turn_external_secret = ENV_TURN_EXTERNAL_SECRET), and turned out my TURN relay is misconfigured but I did not noticed it because somehow XMPP calls still works. I tested import the environment file as a normal file and also tested combined the environmentFile option with sops-nix, neither of it seems to work. Now I just hardcoded it in nix store as a work around, but not ideal.

aanderse commented 5 months ago

entirely unrelated to this a friend asked me if i knew anything about self hosting video platforms

this resulted in me firing up a jitsi instance which was a pleasant experience thanks to the wonderful authors of that module

immediately after the next nixos release which is upcoming would be a good time to fix-up this PR and merge... maybe i can find the time and motivation

anyone still interested in this getting merged?

amalgame21 commented 5 months ago

entirely unrelated to this a friend asked me if i knew anything about self hosting video platforms

this resulted in me firing up a jitsi instance which was a pleasant experience thanks to the wonderful authors of that module

immediately after the next nixos release which is upcoming would be a good time to fix-up this PR and merge... maybe i can find the time and motivation

anyone still interested in this getting merged?

Yes of course! This rewrite have a lot of great changes, It is great for all XMPP and jitsi NixOS user if it can be merged!

longregen commented 1 month ago

It would be amazing if this PR could be pushed forward.