restic-rest-server module: init #39943
Conversation
|
cc @Lassulus |
| preStart = '' | ||
| # Make sure directories exist with correct owner | ||
| mkdir -p ${cfg.dataDir} | ||
| chown restic:restic ${cfg.dataDir} |
There was a problem hiding this comment.
This can better be done using users.users.restic.createHome.
There was a problem hiding this comment.
Good idea thanks. I will update the PR.
| privateRepos = mkOption { | ||
| default = false; | ||
| type = types.bool; | ||
| description = "Enable private repos."; |
There was a problem hiding this comment.
Maybe this could be a little more self-explanatory?
There was a problem hiding this comment.
I had a more comprehensice description here. But it turned out very long and in the end was more or less the same as was written in the readme of rest-server. So I think I can just copy that in here.
| after = [ "network.target" ]; | ||
| wantedBy = [ "multi-user.target" ]; | ||
| serviceConfig = { | ||
| PermissionsStartOnly = true; |
| ${optionalString cfg.appendOnly "--append-only"} \ | ||
| ${optionalString cfg.privateRepos "--private-repos"} \ | ||
| ${optionalString cfg.prometheus "--prometheus"} \ | ||
| ${concatStringsSep " " cfg.extraFlags} \ |
There was a problem hiding this comment.
You could use escapeShellArgs here.
| serviceConfig = { | ||
| PermissionsStartOnly = true; | ||
| ExecStart = '' | ||
| ${cfg.package}/bin/rest-server server \ |
There was a problem hiding this comment.
The second server shouldn't be there, should it?
There was a problem hiding this comment.
I will check the command line, but I think you are right.
| ''; | ||
| Type = "simple"; | ||
| User = "restic"; | ||
| Group = "restic"; |
There was a problem hiding this comment.
Did you think about using some of the hardening options like ProtectSystem = "strict" and ReadWritePaths?
You can also hava a look at the BorgBackup module.
There was a problem hiding this comment.
No I haven't. This is just a minimal service atm. But I will take a look at the borg backup service and see if I can add some hardening here.
|
@dotlambda All comments addressed |
|
Right now, the user needs to put the Another thing that would be nice to have is a combined test for the restic and rest-server modules. But that can be done in another PR. |
|
@dotlambda I tought about adding an option to set the
Also they seem to have changed some things regarding authentication after 0.9.7 (like the I also think a test might be good but as you said this can be done in a next PR. |
Motivation for this change
Allow running the restic REST server as module.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)/cc @dotlambda as you added the binary