nixos/znc: More flexible module, cleanups #45470

infinisil · 2018-08-22T16:53:14Z

Motivation for this change

This is the follow up on #41467, which I needed to clean up for a while, and I finally did it.

This adds a new option services.znc.config, which can represent the full ZNC configuration as a Nix value, see its description for more info. It's much more flexible than the older services.znc.confOptions.* options. It allows the user to override previously fixed values and to set options that the module system might not know about, as it happens on every ZNC upgrade.

Because this new option doesn't know about all valid ZNC options however, it can't do any checks for it to be correct. This doesn't have an easy solution unfortunately. It will generate a syntactically valid file though.

Other changes:

Clean up everything, remove unused options

It's now possible to:

Have multiple users
Set channels as detached
Define QuitMsg
And everything else that wasn't previously possible

The new config option will get converted into the xml-like syntax znc expects, works with all config values, see https://wiki.znc.in/Configuration. This is what one could look like:

services.znc.useLegacyConfig = false;
services.znc.config = {
  # Listener.l.Port = 5000; <- This is added by default
  LoadModule = [ "webadmin" "adminlog" ];
  User.paul = {
    Admin = true;
    Nick = "paul";
    AltNick = "paul1";
    LoadModule = [ "chansaver" "controlpanel" ];
    Network.freenode = {
      Server = "chat.freenode.net +6697";
      LoadModule = [ "simple_away" ];
      Chan = {
        "#nixos" = { Detached = false; };
        "##linux" = { Disabled = true; };
      };
    };
    Pass.password = {
      Method = "sha256";
      Hash = "e2ce303c7ea75c571d80d8540a8699b46535be6a085be3414947d638e48d9e93";
      Salt = "l5Xryew4g*!oa(ECfX2o";
    };
  };
}

To see the current config, you can either inspect it via nix-instantiate:

$ nix-instantiate --eval --strict '<nixpkgs/nixos>' -A config.services.znc.config
{ Listener = { l = { IPv4 = true; IPv6 = true; Port = 5000; SSL = true; }; }; LoadModule = [ "webadmin" "adminlog" ]; User = { znc = { Admin = true; AltNick = "znc-user_"; Ident = "znc-user"; LoadModule = [ "chansaver" "controlpanel" ]; Network = { freenode = { Chan = { }; LoadModule = [ "simple_away" ]; Server = "test +6697 "; extraConfig = null; }; }; Nick = "znc-user"; RealName = "znc-user"; extraConfig = [ "<Pass password>\n     Method = sha256\n     Hash = e2ce303c7ea75c571d80d8540a8699b46535be6a085be3414947d638e48d9e93\n     Salt = l5Xryew4g*!oa(ECfX2o\n</Pass>\n" ]; }; }; Version = "1.7.1"; }

Or directly build the config file and look at that:

$ nix-build '<nixpkgs/nixos>' -A config.services.znc.configFile
/nix/store/330zxcdjai2ciw3risy9iqymnda46i2i-znc-generated.conf

Unless you disable the new option services.znc.useLegacyConfig, the values in services.znc.confOptions.* will also have influence on services.znc.config, such as a default list of modules, default user, etc. This isn't problematic, the new config options handles merging without trouble.

Things done

Ping @nocoolnametom @viric @schneefux @LnL7

Review commit by commit

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
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 nox --run "nox-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)
Fits CONTRIBUTING.md.

LnL7 · 2018-08-22T19:25:08Z

nixos/modules/services/networking/znc.nix

-        '';
-      };
-
-      confOptions = {


Since this removes options it definitively need a changelog entry.

I added an entry in the release notes for the removed options (the ones that can't be warned against by the module system, the ones in the submodule)

Oh sorry, forgot to remove this one when I got to the end. 😄

I mean it is a valid point though!

Ah, didn't look at that part in depth and assumed it was compatible. So I guess good thing I forgot.

LnL7 · 2018-08-22T19:38:23Z

nixos/modules/services/networking/znc/default.nix

+      description = "ZNC Server";
+      wantedBy = [ "multi-user.target" ];
+      after = [ "network-online.target" ];
+      serviceConfig = {


I wonder if this should have restartIfChanged = false

I don't see any compelling reason for that.

To avoid disconnecting.

infinisil · 2018-10-12T14:52:56Z

nixos/modules/services/networking/znc/default.nix

+      config = {
+        Version = (builtins.parseDrvName pkgs.znc.name).version;
+        Listener.l.Port = mkDefault 5000;
+        Listener.l.SSL = mkDefault true;


The default for ZNC is to not use SSL, so I explicitly set the default here to turn it on, consistent with the defaults in the old-style config.

infinisil · 2018-10-12T14:55:05Z

I have now tested this with my own ZNC setup, and I think this is ready to be merged.

Ping @LnL7 @dustinlacewell

dhess · 2018-10-12T17:40:52Z

I'm a ZNC user and this looks good to me. I would request that a (near) future change is made to keep the secret (password and/or password hash) out of the config file so that it's not written to the store, however.

LnL7 · 2018-10-12T19:04:54Z

nixos/doc/manual/release-notes/rl-1809.xml

@@ -200,6 +200,14 @@ $ nix-instantiate -E '(import &lt;nixpkgsunstable&gt; {}).gitFull'
      from your config without any issues.
    </para>
   </listitem>
+   <listitem>


Don't forget to move this to 19.03

LnL7 · 2018-10-12T19:05:52Z

nixos/modules/services/networking/znc/default.nix

+      description = "ZNC Server";
+      wantedBy = [ "multi-user.target" ];
+      after = [ "network-online.target" ];
+      serviceConfig = {


To avoid disconnecting.

LnL7

Looks good to me, but I don't run znc on nixos.

Move legacy options to separate file

infinisil · 2018-10-13T13:10:35Z

Rebased on master to get the release note manual file on 19.03 and moved the release notes there

I'll ask around a bit more, but if nobody complains I'll merge this myself.

infinisil · 2018-10-13T13:13:10Z

Worth mentioning: The first and second commit don't produce a working znc config, so they're not ideal for bisecting, but it would take a lot of work to split them up better. I could squash them if that's desired.

infinisil · 2018-10-14T18:25:15Z

Squashed the 2 relevant commits into one, made more sense like that

infinisil · 2018-10-14T18:27:35Z

The nixos/znc: add config option commit is the meaty one.

This option represents the ZNC configuration as a Nix value. It will be converted to a syntactically valid file. This provides: - Flexibility: Any ZNC option can be used - Modularity: These values can be set from any NixOS module and will be merged correctly - Overridability: Default values can be overridden Also done: Remove unused/unneeded options, mkRemovedOptionModule unfortunately doesn't work inside submodules (yet). The options userName and modulePackages were never used to begin with

infinisil · 2018-10-14T18:40:32Z

Pushed some minor description updates

nixos/znc: Fix regressions introduced in #45470

LnL7 reviewed Aug 22, 2018

View reviewed changes

GrahamcOfBorg added 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Aug 22, 2018

infinisil force-pushed the znc-config branch from 9e64992 to 6210bd7 Compare October 12, 2018 14:51

infinisil commented Oct 12, 2018

View reviewed changes

dhess approved these changes Oct 12, 2018

View reviewed changes

LnL7 reviewed Oct 12, 2018

View reviewed changes

LnL7 approved these changes Oct 12, 2018

View reviewed changes

nixos/znc: move to own folder

4eee2cd

Move legacy options to separate file

infinisil force-pushed the znc-config branch from 6210bd7 to 3ee4ab3 Compare October 13, 2018 13:09

infinisil force-pushed the znc-config branch from 3ee4ab3 to 624201a Compare October 14, 2018 18:20

infinisil force-pushed the znc-config branch from 624201a to fa234a1 Compare October 14, 2018 18:26

infinisil added 2 commits October 14, 2018 20:39

nixos/znc: Add release note entry for removed options

7e31678

infinisil force-pushed the znc-config branch from fa234a1 to 7e31678 Compare October 14, 2018 18:40

infinisil merged commit e443bbf into NixOS:master Oct 17, 2018

infinisil deleted the znc-config branch October 17, 2018 01:01

infinisil added a commit to infinisil/system that referenced this pull request Oct 18, 2018

Test NixOS/nixpkgs#45470

ef96c0f

infinisil mentioned this pull request Oct 20, 2018

nixos/znc: Fix regressions introduced in #45470 #48756

Merged

9 tasks

infinisil added a commit that referenced this pull request Oct 20, 2018

Merge pull request #48756 from Infinisil/fix/znc-extraZncConf

2146c6d

nixos/znc: Fix regressions introduced in #45470

This was referenced Nov 10, 2018

.github/CODEOWNERS: Add infinisil to NixOS modules #50118

Merged

nginx NixOS module: generated configuration has many superfluous newlines #34377

Closed

infinisil mentioned this pull request Dec 29, 2018

Nginx formatter #34985

Closed

9 tasks

infinisil mentioned this pull request Jan 28, 2019

Add support for burp client/server services #48762

Closed

9 tasks

infinisil mentioned this pull request Feb 18, 2019

nixos/icingaweb2: Init the module #55957

Merged

10 tasks

infinisil mentioned this pull request Mar 5, 2019

nixos/davmail: init #52096

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/znc: More flexible module, cleanups #45470

nixos/znc: More flexible module, cleanups #45470

infinisil commented Aug 22, 2018 •

edited

Loading

LnL7 Aug 22, 2018

infinisil Aug 22, 2018

LnL7 Aug 22, 2018

infinisil Aug 22, 2018

LnL7 Aug 22, 2018

LnL7 Aug 22, 2018

infinisil Aug 22, 2018

LnL7 Oct 12, 2018

infinisil Oct 12, 2018

infinisil commented Oct 12, 2018

dhess commented Oct 12, 2018

LnL7 Oct 12, 2018

LnL7 Oct 12, 2018

LnL7 left a comment

infinisil commented Oct 13, 2018

infinisil commented Oct 13, 2018

infinisil commented Oct 14, 2018

infinisil commented Oct 14, 2018

infinisil commented Oct 14, 2018

nixos/znc: More flexible module, cleanups #45470

nixos/znc: More flexible module, cleanups #45470

Conversation

infinisil commented Aug 22, 2018 • edited Loading

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

infinisil commented Oct 12, 2018

dhess commented Oct 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LnL7 left a comment

Choose a reason for hiding this comment

infinisil commented Oct 13, 2018

infinisil commented Oct 13, 2018

infinisil commented Oct 14, 2018

infinisil commented Oct 14, 2018

infinisil commented Oct 14, 2018

infinisil commented Aug 22, 2018 •

edited

Loading