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/prosody: add MUC extraConfig + fixes #86649

Merged
merged 2 commits into from May 4, 2020

Conversation

@mmilata
Copy link
Member

mmilata commented May 3, 2020

Thanks for the work in #86067 @NinjaTrappeur, even though it probably wasn't the goal it makes the Jitsi Meet module compose better with user-defined Prosody configuration! I'd like to propose for a few modifications in this PR:

  • move the main extraConfig before the component definitions in the config file, so that it actually applies globally instead of to the http_upload component
  • add extraConfig option to the MUC submodule so that it's possible to provide additional options (like storage or admins)
  • fix nixos/tests/xmpp/prosod-mysql.nix

cc @flokli

Motivation for this change

Mostly #82920.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@NinjaTrappeur
Copy link
Contributor

NinjaTrappeur commented May 3, 2020

Thanks! Sounds good!

Aha, nice catch regarding the mysql test.

@GrahamcOfBorg test prosodyMysql

Copy link
Contributor

NinjaTrappeur left a comment

LGTM!

@mmilata
Copy link
Member Author

mmilata commented May 3, 2020

Looks like xmpp/ejabberd.nix is also broken though I'm not familiar enough with it to fix it:/

@mmilata
Copy link
Member Author

mmilata commented May 3, 2020

Btw the jitsi module now only uses global extraConfig to add external component:

        Component "focus.${cfg.hostName}"
          component_secret = os.getenv("JICOFO_COMPONENT_SECRET")

I'm wondering whether it would make sense to extend the Prosody module with options for external components. The os.getenv probably makes this needlessly complicated so I guess just adding this snippet to extraConfig with mkAfter minimizes the chance of bad ordering of extraConfigs from multiple sources.

@NinjaTrappeur
Copy link
Contributor

NinjaTrappeur commented May 3, 2020

Indeed.

This test, together with the two prosody ones were broken for a long time actually. The send-xmpp.py script was always exiting with a 0 code, regardless of the script outcome. #86067 is trying its best to fix that :/

After a quick scan, it looks like the ebjabberd test is lacking the http_upload support, which we now aim to ship by default, we'll definitely need to setup this module for this test to pass.

I added this to my todo list.

This matter is however out of scope for this PR, let's merge this ;)

networking.extraHosts = ''
${nodes.server.config.networking.primaryIPAddress} example.com
${nodes.server.config.networking.primaryIPAddress} conference.example.com
${nodes.server.config.networking.primaryIPAddress} uploads.example.com
'';
Comment on lines +9 to +13

This comment has been minimized.

Copy link
@flokli

flokli May 3, 2020

Contributor

The test fixes should be a separate commit

This comment has been minimized.

Copy link
@mmilata

mmilata May 3, 2020

Author Member

Done, PTAL.

mmilata added 2 commits May 3, 2020
Add extraConfig option for the muc submodule.

Also move the global extraConfig before all components and
virtualhosts, because the manual states:

    The configuration is divided into two parts. The first part is known as
    the "global" section. All settings here apply to the whole server, and
    are the default for all virtual hosts.

    The second half of the file is a series of VirtualHost and Component
    definitions. Settings under each VirtualHost or Component line apply
    only to that host.

Before, if at least one muc was defined, or uploadHttp enabled, the
global extraConfig would end up after "muc" or "http_upload" component
making it apply to that component only and not globally.
Since 8aea528 xmpp-sendmessage.nix tests MUC and HTTP upload,
change the test to reflect this.
@mmilata mmilata force-pushed the mmilata:prosody-muc-extraconfig branch from 3c5ab7a to 96146a9 May 3, 2020
@flokli flokli merged commit dd38a54 into NixOS:master May 4, 2020
14 checks passed
14 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96146a9"; rev="96146a947678aaef6ba1ec5b8887cc79a70603ca"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96146a9"; rev="96146a947678aaef6ba1ec5b8887cc79a70603ca"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96146a9"; rev="96146a947678aaef6ba1ec5b8887cc79a70603ca"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96146a9"; rev="96146a947678aaef6ba1ec5b8887cc79a70603ca"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96146a9"; rev="96146a947678aaef6ba1ec5b8887cc79a70603ca"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96146a9"; rev="96146a947678aaef6ba1ec5b8887cc79a70603ca"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="96146a9"; rev="96146a947678aaef6ba1ec5b8887cc79a70603ca"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@mmilata mmilata deleted the mmilata:prosody-muc-extraconfig branch May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.