nixos/nzbget: fix broken service, add a nixos test, as well as some general improvements

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

@@ -83,6 +83,14 @@
       The same applies to ModemManager where modem-manager.service is now called ModemManager.service again.
     </para>
    </listitem>
+   <listitem>
+    <para>
+      The <option>services.nzbget.configFile</option> and <option>services.nzbget.openFirewall</option>
+      options were removed as they are managed internally by the nzbget. The
+      <option>services.nzbget.dataDir</option> option hadn't actually been used by
+      the module for some time and so was removed as cleanup.
+    </para>
+   </listitem>
   </itemizedlist>
  </section>
 

nixos/modules/rename.nix

@@ -45,6 +45,9 @@ with lib;
     (mkRemovedOptionModule [ "services" "neo4j" "port" ] "Use services.neo4j.http.listenAddress instead.")
     (mkRemovedOptionModule [ "services" "neo4j" "boltPort" ] "Use services.neo4j.bolt.listenAddress instead.")
     (mkRemovedOptionModule [ "services" "neo4j" "httpsPort" ] "Use services.neo4j.https.listenAddress instead.")
+    (mkRemovedOptionModule [ "services" "misc" "nzbget" "configFile" ] "The configuration of nzbget is now managed by users through the web interface.")
+    (mkRemovedOptionModule [ "services" "misc" "nzbget" "dataDir" ] "The data directory for nzbget is now /var/lib/nzbget.")
+    (mkRemovedOptionModule [ "services" "misc" "nzbget" "openFirewall" ] "The port used by nzbget is managed through the web interface so you should adjust your firewall rules accordingly.")
     (mkRemovedOptionModule [ "services" "prometheus" "alertmanager" "user" ] "The alertmanager service is now using systemd's DynamicUser mechanism which obviates a user setting.")
     (mkRemovedOptionModule [ "services" "prometheus" "alertmanager" "group" ] "The alertmanager service is now using systemd's DynamicUser mechanism which obviates a group setting.")
     (mkRenamedOptionModule [ "services" "tor" "relay" "portSpec" ] [ "services" "tor" "relay" "port" ])

nixos/modules/services/misc/nzbget.nix

@@ -4,33 +4,35 @@ with lib;
 
 let
   cfg = config.services.nzbget;
-  dataDir = builtins.dirOf cfg.configFile;
-in {
+  pkg = pkgs.nzbget;
+  stateDir = "/var/lib/nzbget";
+  configFile = "${stateDir}/nzbget.conf";
+  configOpts = concatStringsSep " " (mapAttrsToList (name: value: "-o ${name}=${value}") nixosOpts);
+
+  nixosOpts = {
+    # allows nzbget to run as a "simple" service
+    OutputMode = "loggable";
+    # use journald for logging
+    WriteLog = "none";
+    ErrorTarget = "screen";
+    WarningTarget = "screen";
+    InfoTarget = "screen";
+    DetailTarget = "screen";
+    # required paths
+    ConfigTemplate = "${pkg}/share/nzbget/nzbget.conf";
+    WebDir = "${pkg}/share/nzbget/webui";
+    # nixos handles package updates
+    UpdateCheck = "none";
+  };
+
+in
+{
+  # interface
+
   options = {
     services.nzbget = {
       enable = mkEnableOption "NZBGet";
 
-      package = mkOption {
-        type = types.package;
-        default = pkgs.nzbget;
-        defaultText = "pkgs.nzbget";
-        description = "The NZBGet package to use";
-      };
-
-      dataDir = mkOption {
-        type = types.str;
-        default = "/var/lib/nzbget";
-        description = "The directory where NZBGet stores its configuration files.";
-      };
-
-      openFirewall = mkOption {
-        type = types.bool;
-        default = false;
-        description = ''
-          Open ports in the firewall for the NZBGet web interface
-        '';
-      };
-
       user = mkOption {
         type = types.str;
         default = "nzbget";
@@ -42,15 +44,11 @@ in {
         default = "nzbget";
         description = "Group under which NZBGet runs";
       };
-
-      configFile = mkOption {
-        type = types.str;
-        default = "/var/lib/nzbget/nzbget.conf";
-        description = "Path for NZBGet's config file. (If this doesn't exist, the default config template is copied here.)";
-      };
     };
   };
 
+  # implementation
+
   config = mkIf cfg.enable {
     systemd.services.nzbget = {
       description = "NZBGet Daemon";
@@ -61,50 +59,26 @@ in {
         p7zip
       ];
       preStart = ''
-        cfgtemplate=${cfg.package}/share/nzbget/nzbget.conf
-        if [ ! -f ${cfg.configFile} ]; then
-          echo "${cfg.configFile} not found. Copying default config $cfgtemplate to ${cfg.configFile}"
-          install -m 0700 $cfgtemplate ${cfg.configFile}
-          echo "Setting temporary \$MAINDIR variable in default config required in order to allow nzbget to complete initial start"
-          echo "Remember to change this to a proper value once NZBGet startup has been completed"
-          sed -i -e 's/MainDir=.*/MainDir=\/tmp/g' ${cfg.configFile}
+        if [ ! -f ${configFile} ]; then
+          ${pkgs.coreutils}/bin/install -m 0700 ${pkg}/share/nzbget/nzbget.conf ${configFile}
         fi
       '';
 
-      script = ''
-        args="--daemon --configfile ${cfg.configFile}"
-        # The script in preStart (above) copies nzbget's config template to datadir on first run, containing paths that point to the nzbget derivation installed at the time.
-        # These paths break when nzbget is upgraded & the original derivation is garbage collected. If such broken paths are found in the config file, override them to point to
-        # the currently installed nzbget derivation.
-        cfgfallback () {
-          local hit=`grep -Po "(?<=^$1=).*+" "${cfg.configFile}" | sed 's/[ \t]*$//'` # Strip trailing whitespace
-          ( test $hit && test -e $hit ) || {
-            echo "In ${cfg.configFile}, valid $1 not found; falling back to $1=$2"
-            args+=" -o $1=$2"
-          }
-        }
-        cfgfallback ConfigTemplate ${cfg.package}/share/nzbget/nzbget.conf
-        cfgfallback WebDir ${cfg.package}/share/nzbget/webui
-        ${cfg.package}/bin/nzbget $args
-      '';
-
       serviceConfig = {
-        StateDirectory = dataDir;
-        StateDirectoryMode = "0700";
-        Type = "forking";
+        StateDirectory = "nzbget";
+        StateDirectoryMode = "0750";
         User = cfg.user;
         Group = cfg.group;
-        PermissionsStartOnly = "true";
+        UMask = "0002";
         Restart = "on-failure";
+        ExecStart = "${pkg}/bin/nzbget --server --configfile ${stateDir}/nzbget.conf ${configOpts}";
+        ExecStop = "${pkg}/bin/nzbget --quit";
       };
     };
 
-    networking.firewall = mkIf cfg.openFirewall {
-      allowedTCPPorts = [ 8989 ];
-    };
-
     users.users = mkIf (cfg.user == "nzbget") {
       nzbget = {
+        home = stateDir;
         group = cfg.group;
         uid = config.ids.uids.nzbget;
       };

nixos/tests/all-tests.nix

@@ -172,6 +172,7 @@ in
   nix-ssh-serve = handleTest ./nix-ssh-serve.nix {};
   novacomd = handleTestOn ["x86_64-linux"] ./novacomd.nix {};
   nsd = handleTest ./nsd.nix {};
+  nzbget = handleTest ./nzbget.nix {};
   openldap = handleTest ./openldap.nix {};
   opensmtpd = handleTest ./opensmtpd.nix {};
   openssh = handleTest ./openssh.nix {};

nixos/tests/nzbget.nix

@@ -0,0 +1,26 @@
+import ./make-test.nix ({ pkgs, ...} : {
+  name = "nzbget";
+  meta = with pkgs.stdenv.lib.maintainers; {
+    maintainers = [ aanderse flokli ];
+  };
+
+  nodes = {
+    server = { ... }: {
+      services.nzbget.enable = true;
+
+      # hack, don't add (unfree) unrar to nzbget's path,
+      # so we can run this test in CI
+      systemd.services.nzbget.path = pkgs.stdenv.lib.mkForce [ pkgs.p7zip ];
+    };
+  };
+
+  testScript = ''
+    startAll;
+
+    $server->waitForUnit("nzbget.service");
+    $server->waitForUnit("network.target");
+    $server->waitForOpenPort(6789);
+    $server->succeed("curl -s -u nzbget:tegbzn6789 http://127.0.0.1:6789 | grep -q 'This file is part of nzbget'");
+    $server->succeed("${pkgs.nzbget}/bin/nzbget -n -o ControlIP=127.0.0.1 -o ControlPort=6789 -o ControlPassword=tegbzn6789 -V");
+  '';
+})