nixos/flood: init

[3JlOy-PYCCKUi]

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

Priorities

Add a 👍 reaction to pull requests you find important.

[3JlOy-PYCCKUi]

@ofborg eval

[azahi]

If you have included tests, they also need to be specified in the package's passthru.tests attribute.

Also, Flood requires mediainfo to be present on runtime. However, I'm not sure how to properly handle this: you can try to find the call to it in the source code and patch it out there (or wrap the executable with PATH), or just add it to Path= in the systemd service.

Lastly, we probably should add asserts so that only one type of connection is specified at once, i.e. only rtorrent is allowed, not rtorrent and deluge etc. This can also be solved by adding an enable flag on each of these.

[azahi]

Oh, and you should also mention this module in the release notes for 24.05. See CONTRIBUTING.md

[rasmus-kirk]

Just a suggestion, but maybe add a dataDir or option like with services.rtorrent.dataDir? If I have lots of services running, I much prefer configuring the stateful directories myself. Take a look at #277220 for this.

Also, do we not need to create the flood user and group in this module? Something like:

    users.users = mkIf (cfg.user == "flood") {
      flood = {
        group = cfg.group;
        isSystemUser = true;
      };
    };

    users.groups = mkIf (cfg.group == "flood") {
      flood = {};
    };

[rasmus-kirk]

Passing in secret values through options is bad practice, as they will leak out into the nix store. So this is not really ideal. Unfortunately, Flood does not seem to have any way of loading in these options declaratively without using the command line options. I am currently trying to ask on the Flood Discord if there is a current way or how a PR that addresses this would be structured.

[ambroisie]

The technically correct one:

Suggested change

	ExecStart = "${cfg.package}/bin/flood ${lib.concatStringsSep " " args}";
	ExecStart = "${cfg.package}/bin/flood ${lib.escapeShellArgs " " args}";

[ambroisie]

Actually, I now know that the correct one is utils.escapeSystemdExecArgs.

[ambroisie]

Flood also supports stateful configuration of its auth/client configuration, which I might encourage using instead of creating options which leak secrets to the Nix store (though that is a very weak argument...).

[rasmus-kirk]

Flood also supports stateful configuration of its auth/client configuration, which I might encourage using instead of creating options which leak secrets to the Nix store (though that is a very weak argument...).

Added a PR for Flood for being able to pass the command line options as a JSON as well. They also compose, so you can set qbittorrent.user = qbuser and extraConfig = ./path/to/config.json where config.json contains:

{
  "qbpass": "qbpassword"
}

[fsnkty]

lib.mdDoc is now just an alias and can be safely removed everywhere.
see d36f950 and #237557

[fsnkty]

${lib.getExe cfg.package} would be nice.

[thiagokokada]

#317530 was merged, and while this PR does brings some more features it will need to be redone based in #317530 anyway.

So closing this PR.