-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
overseerr: init at 1.33.2 #278526
base: master
Are you sure you want to change the base?
overseerr: init at 1.33.2 #278526
Conversation
5ec133a
to
c920d5a
Compare
WorkingDirectory = "${pkgs.overseerr}/libexec/overseerr/deps/overseerr"; | ||
DynamicUser = true; | ||
ExecStart = "${pkgs.overseerr}/bin/overseerr"; | ||
BindPaths = [ "/var/lib/overseerr/:${pkgs.overseerr}/libexec/overseerr/deps/overseerr/config/" ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on here? Does it expect read and write config in libexec
?
In that case, I think it would warrant patching the package. Packages are supposed to be usable outside of nixos as well, and I wouldn't really consider packages that requires bindmounts into the nix store to be in a working condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a while and I don't remember it that well, but I think it does require writing to that dir, yes.
It seems like we can set a CONFIG_DIRECTORY
environment variable... what's the recommended thing to do here? Ask the user where to put it?
@ofborg eval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the feedback below, please rebase into 2 commits, one which adds the package, and a second which adds the module.
Additionally for the second commit, would be good to have the following:
Added a release notes entry if adding a new NixOS module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second reading, the previous feedback re: mdDoc
and mkEnableOption
may have been confusing - providing what I was looking for here.
37252c8
to
3eb88fb
Compare
I think all the points were addressed... I can rebase on master if you think it's good to go... |
1 similar comment
I think all the points were addressed... I can rebase on master if you think it's good to go... |
it won't be merged regardless if there's a merge conflict right? seems like it'd be a good idea to solve preemptively regardless |
Signed-off-by: Carlos Alexandro Becker <[email protected]>
Signed-off-by: Carlos Alexandro Becker <[email protected]>
fair, done @nu-nu-ko |
I ended up adding --- /dev/fd/63 2024-09-09 21:03:59.746049505 +0100
+++ modules/services/misc/overseerr.nix 2024-09-09 21:03:58.179092709 +0100
@@ -18,21 +18,44 @@
default = 5055;
description = ''The port which the Overseerr web UI should listen to.'';
};
+
+ dataDir = lib.mkOption {
+ type = lib.types.str;
+ default = "/var/lib/overseerr";
+ description = lib.mdDoc "The directory where Overseerr stores its data files.";
+ };
+
+ user = lib.mkOption {
+ type = lib.types.str;
+ default = "overseerr";
+ description = lib.mdDoc "User account under which Overseerr runs.";
+ };
+
+ group = lib.mkOption {
+ type = lib.types.str;
+ default = "overseerr";
+ description = lib.mdDoc "Group under which Overseerr runs.";
+ };
};
config = lib.mkIf cfg.enable {
+ systemd.tmpfiles.settings."10-overseerr".${cfg.dataDir}.d = {
+ inherit (cfg) user group;
+ mode = "0700";
+ };
+
systemd.services.overseerr = {
description = "Request management and media discovery tool for the Plex ecosystem";
after = [ "network.target" ];
wantedBy = [ "multi-user.target" ];
- environment.PORT = toString cfg.port;
+ environment = {
+ CONFIG_DIRECTORY = cfg.dataDir;
+ PORT = toString cfg.port;
+ };
serviceConfig = {
Type = "exec";
- StateDirectory = "overseerr";
WorkingDirectory = "${cfg.package}/libexec/overseerr/deps/overseerr";
- DynamicUser = true;
ExecStart = lib.getExe cfg.package;
- BindPaths = [ "/var/lib/overseerr/:${cfg.package}/libexec/overseerr/deps/overseerr/config/" ];
Restart = "on-failure";
ProtectHome = true;
ProtectSystem = "strict";
@@ -49,11 +72,26 @@
RestrictSUIDSGID = true;
RemoveIPC = true;
PrivateMounts = true;
+ ReadWritePaths = [ cfg.dataDir ];
+ User = cfg.user;
+ Group = cfg.group;
};
};
networking.firewall = lib.mkIf cfg.openFirewall {
allowedTCPPorts = [ cfg.port ];
};
+
+ users.users = lib.mkIf (cfg.user == "overseerr") {
+ overseerr = {
+ group = cfg.group;
+ home = cfg.dataDir;
+ uid = config.ids.uids.overseerr;
+ };
+ };
+
+ users.groups = lib.mkIf (cfg.group == "overseerr") {
+ overseerr.gid = config.ids.gids.overseerr;
+ };
};
}
|
Description of changes
First time packaging a node app, based myself more or less on jellyseerr (which is a fork of overseerr).
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.