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/nzbget: cfg.configFile/cfg.dataDir owned by cfg.user/cfg.group #58928
nixos/nzbget: cfg.configFile/cfg.dataDir owned by cfg.user/cfg.group #58928
Conversation
These changes ensure, via the preStart script, that the configFile and dataDir are owned by the user & group that nzbget has been configured to run with. Without this, when the config template is copied over to facilitate initial startup, it's done so with root permissions. This causes the service to fail during the startup script (grep is used to inspect the file for some values). The dataDir is similar circumstances; without these changes, it's created with root permissions, which means the service cannot read or write data there, if it's running as a different user.
@@ -61,10 +61,14 @@ in { | |||
p7zip | |||
]; | |||
preStart = '' | |||
if [ ! -d ${cfg.dataDir} ]; then | |||
echo "${cfg.dataDir} directory not found. Creating it." | |||
install -d -o ${cfg.user} -g ${cfg.group} ${cfg.dataDir} |
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.
Use systemd.tmpfiles
instead for doing this.
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.
Could you expand a little on why systemd.tmpfiles
might be a better approach here?
The resulting directory is intended to be "long-lasting".
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.
I guess the name tmpfiles
is a bit misleading. It can be used for non-temporary files as well, which is in fact the main use of it in NixOS. Because this approach is declarative, it is preferred in NixOS.
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.
Ah okay, great. Thanks for the suggestion, I'll take a look!
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.
Oh and actually, there's serviceConfig.StateDirectory
too, which is preferred over tmpfiles if it works (it's not as flexible as tmpfiles, but I don't think that flexibility is needed for this specific case).
Sure @aanderse, might as well catch it now :) |
Motivation for this change
These changes ensure, via the
preStart
script, that theconfigFile
anddataDir
are owned by the user & group that
nzbget
has been configured to run with.Without this, when the config template is copied over to facilitate
initial startup, it's done so with
root
permissions. This causes theservice to fail during the startup script (
grep
is used to inspect thefile for some values). The
dataDir
is similar circumstances; without thesechanges, it's created with
root
permissions, which means the service cannotread or write data there, if it's running as a different user.
Example of failure scenario
Things done
Ensure the
configFile
anddataDir
are owned by the user & group the service has been configured to run with.sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)