Skip to content
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.

Sign up for GitHub

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

Jump to bottom

nixos/transmission: allow prestart to create directories #42537

Closed
wants to merge 2 commits into from
Closed

nixos/transmission: allow prestart to create directories #42537

wants to merge 2 commits into from

Conversation

emmanuelrosa
Copy link
Contributor

@emmanuelrosa emmanuelrosa commented Jun 24, 2018 •
edited

Motivation for this change

This change executes the systemd transmission service pre-start script as root
so that it's able to create any directories specified in the configuration. In addition,
a smoke-test is added to the existing tests.

Pre-start script
  • Directories leading to a given directory are created (if missing) in mode 775 and owned by root (this is new).
  • If the final directory does not exist:
    • Mode set to 770 (left unchanged)
    • Group is changed to transmission (changed).
    • Owner and group are changed to transmission (changed).
    • Ownership is left unchanged; Hence if the directory had to be created the file it will be owned by root.

Note that this does not guarantee that the directories are accessible by
transmission. For example, if the download-dir is set to /home/alice/torrents
the pre-start script will be able to create the directory, but because
/home/alice is owned by user alice and the mode is 700, transmission
won't be able to access it (since it runs under the user transmission).

Smoke-test

This change also updates the transmission NixOS test to attempt downloading a torrent.
The purpose is to ensure the transmission daemon can access its directories.

For example, by default adding a torrent via transmission-remote causes the daemon
to save the .torrent file to /var/lib/transmission/.config/torrents

The download is cancelled right after it's started.

Closes #41643

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • [ ] macOS
    • [ ] other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

The command nix path-info -S nixos/tests -f transmission.nix produced: error: path '/nix/store/5fzg0xpyhx82slw7i7d5761bkh9k3s8q-vm-test-run-transmission' is not valid I don't know how to determine the closure size for this change.

@matthewbauer
Copy link
Member

@GrahamcOfBorg test transmission

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: tests.transmission

Partial log (click to expand)

machine: exit status 1
machine: output:
error: command `/nix/store/i0dzl38z9q2xn113p1c4kq2knqkzmx71-transmission-2.94/bin/transmission-remote -a https://cdimage.debian.org/debian-cd/current/amd64/bt-cd/debian-9.4.0-amd64-netinst.iso.torrent' did not succeed (exit code 1)
command `/nix/store/i0dzl38z9q2xn113p1c4kq2knqkzmx71-transmission-2.94/bin/transmission-remote -a https://cdimage.debian.org/debian-cd/current/amd64/bt-cd/debian-9.4.0-amd64-netinst.iso.torrent' did not succeed (exit code 1)
cleaning up
killing machine (pid 593)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
builder for '/nix/store/v8c0pphgzjmgvs0xarz0adrlywwjnxai-vm-test-run-transmission.drv' failed with exit code 255
error: build of '/nix/store/v8c0pphgzjmgvs0xarz0adrlywwjnxai-vm-test-run-transmission.drv' failed

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: tests.transmission

Partial log (click to expand)

building '/nix/store/svlyrdkfcqs8i8rswvlynxznzwxn8aaw-unit-qemu-guest-agent.service.drv'...
cannot build derivation '/nix/store/wqlmvgiw3ymm7vgm716vqcbcjyipsavv-system-units.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/nx7hh1100r84hvj6wp48c72cv59jd89d-etc.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/vqx1pmayxqm5qcib807vzwdv7aj03fw9-nixos-system-machine-18.09.git.ccf590b.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/jd86qrc51680xpyj5awf0qnf8kx7p6xr-closure-info.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/4mq6vx3m17j6wv58mhr3m3fq6n1hwv15-run-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/4xp7f564g8s41jd75pmf3n9yylyy6jab-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/xzws8sggzk7m3rkczi17rb1ww4dlr0a6-nixos-test-driver-transmission.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/wc14mhbpqgw52vjsc4grzkybyn2fplgl-vm-test-run-transmission.drv': 1 dependencies couldn't be built
error: build of '/nix/store/wc14mhbpqgw52vjsc4grzkybyn2fplgl-vm-test-run-transmission.drv' failed

@emmanuelrosa
Copy link
Contributor Author

Modified smoke test to use a local copy of the torrent file instead of downloading from Debian's site. The download now fails for some reason; Using a local copy allows the test to run.

@bb2020
Copy link
Member

bb2020 commented Aug 22, 2018 •
edited

I think the service should not create fullSettings.download-dir and fullSettings.incomplete-dir directories. These directories may be mounted after system init and if so the service will always crash while starting.
Edit: It definitely should. Also see comment below.

@bb2020
Copy link
Member

bb2020 commented Sep 2, 2018 •
edited

It looks the ownership should also be changed and mode should be 770, not 755. If it is not writable by group, then creating the group is purposeless.

@emmanuelrosa
Copy link
Contributor Author

755/drwxr-xr-x is used because when the directories are created the absolute path is specified along with the "-p" option, causing mkdir to create any necessary parent directories. Those parent directories (/home, /var, etc) most likely would be 755. 770 is only used on the final directory in the path since that's the directory transmission should be writing to. Leaving the owner as root was probably an oversight on my part, so thank you for bringing it to my attention.

@bb2020
Copy link
Member

bb2020 commented Sep 4, 2018

Oh, you have a chmod 770 beneath mkdir, sorry I did not see it.
chown transmission:transmission instead of chgrp looks like the way to go then.

@emmanuelrosa
Copy link
Contributor Author

Yeah, that's what I just changed. I'm building the test VM now.

I'm also adding an if-then block so that the directories are created only if they don't already exist. That way if users have other permissions, or decide to change them after the fact, the script will leave them alone.

Emmanuel Rosa added 2 commits September 4, 2018 21:38
nixos/transmission: allow prestart to create dirs
08e603b 
This change executes the systemd transmission daemon pre-start script as root
so that it's able to create any directories specified in the configuration.

* Directories leading to a directory are created in mode 775 and are owned by root,
  if those directories are missing (changed).
* If the final directory doesn't already exist:
  * Mode set to 770 (unchanged)
  * Ownership and group are changed to `transmission` (changed).

Note that this does not guarantee that the directories are accessible by
transmission. For example, if the `download-dir` is set to `/home/alice/torrents`
the pre-start script will be able to create the directory, but because
`/home/alice` is owned by user `alice` and the mode is 700, transmission
won't be able to access it (since it runs under the user `transmission`).

Closes NixOS#41643
nixos/transmission: add smoke-test
15b232f 
This change updates the transmission NixOS test to attempt downloading a torrent.
The purpose is to ensure the transmission daemon can access its directories.

For example, by default adding a torrent via `transmission-remote` causes the daemon
to save the .torrent file to /var/lib/transmission/.config/torrents
@emmanuelrosa emmanuelrosa deleted the transmission-mkdir branch December 30, 2018 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@emmanuelrosa @matthewbauer @GrahamcOfBorg @bb2020