Improve the way unset env variables are managed. Remove USER/PASS requirement.

[GGLinnk]

---

• [x] I have read the contributing guideline and understand that I have made the correct modifications

---

Description:

Alters the way environment variables change settings.json. Also kills the transmission by sending a kill command instead of using the remote with USER/PASS. The block list is now updated when the transmission is reloaded.

Benefits of this PR and context:

Benefits

From now on, to define USER/PASS or WHITELIST or HOST_WHITELIST, they must be given the desired value or an empty string. This prevents parameters from being unintentionally overwritten when environment variables are disabled.

It's no longer required to set USER/PASS as an environment variable to reload/stop the container.

Workaround

To be able to totally get rid of the USER and PASS I had to replace remote by alternatives. About blocklist I've been forced to simply reload transmission itself instead of sending the remote command.

Context

The reason I'm doing this is because I think that settings should NEVER be overwritten unless user wanted to. This have been cause of hours of research (yes, I didn't looked the README enough) about why my settings wasn't applied until I figured out that settings was overwritten by the s6 scripts.

How Has This Been Tested?

All changes were verified by shellcheck and the docker image was built and tested locally.

Source / References:

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.4-r0-pkg-5a3b296b-dev-5b305961b78ec0c9c4d6743458b4bc1752b25b56-pr-258/index.html https://ci-tests.linuxserver.io/lspipepr/transmission/4.0.4-r0-pkg-5a3b296b-dev-5b305961b78ec0c9c4d6743458b4bc1752b25b56-pr-258/shellcheck-result.xml	Tag	Passed
amd64-4.0.4-r0-pkg-5a3b296b-dev-5b305961b78ec0c9c4d6743458b4bc1752b25b56-pr-258	✅
arm64v8-4.0.4-r0-pkg-5a3b296b-dev-5b305961b78ec0c9c4d6743458b4bc1752b25b56-pr-258	✅

[LinuxServer-CI]

This pull request has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions.

[GGLinnk]

To prevent this PR from being closed due to inactivity, I'm posting this comment. I understand that extra GitHub comments can be annoying, but it's frustrating to think that this PR might be closed simply because the bot assumes I've been inactive.

I'm patiently waiting for a review or confirmation from the maintainers.

If you find my PR useful, I'll gladly rebase to resolve any conflicts. If you have any concerns or believe that this PR isn't valuable, please don't hesitate to let me know. If you genuinely think it's not worth pursuing, please close this PR and do not let it ghost.

Your feedback is important, and I'm here to make any necessary improvements.

[aptalca]

Thanks for the PR.

We intend for the USER/PASS vars to be active vars, not just a first time setup.

Any setting that's available via env vars are expected to be managed by the env vars. USER/PASS issue explicitly stated in the readme: https://github.com/linuxserver/docker-transmission#securing-the-webui-with-a-usernamepassword

I believe restarting transmission daily on cron just to update blocklists is poor behavior when it allows an update command via cli.

Iirc killing the transmission process leads to unclean shutdowns and various issues related to that including getting banned by trackers.

Tail ensures s6 doesn't shut everything down before transmission has a chance to cleanly shut down. If you're getting an s6 timeout, it likely takes transmission longer than the default 10 seconds to shut down.

[LinuxServer-CI]

This pull request has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions.

[GGLinnk]

We intend for the USER/PASS vars to be active vars, not just a first time setup.

I think this is bad behavior, config file should not be constantly affected by environment variables. Or at least only when intended.

Any setting that's available via env vars are expected to be managed by the env vars. USER/PASS issue explicitly stated in the readme.

Even if specified in documentation it can be misleading. It's not normal to force user to use only that method.

I believe restarting transmission daily on cron just to update blocklists is poor behavior when it allows an update command via cli.

And I believe that it's a good behavior, and it's not concerning to regularly restart, it is actually a good thing. It allow to clean caches and fix eventual bugs that may occurs over time.

Also, we should take into consideration than the doc states that the daemon generates its .bin file only when it start. I don't know if the remote blocking list update actually refresh the .bin files, it may only refresh the list itself, this needs to be verified but the docs give me an impression of "no, it does not". https://github.com/transmission/transmission/blob/main/docs/Blocklists.md => "Only when it starts it creates new .bin files."

Iirc killing the transmission process leads to unclean shutdowns and various issues related to that including getting banned by trackers.

Killing the process via a SIGHUP is actually a good way to shutdown transmission-daemon. It's actually how does work the service.

Tail ensures s6 doesn't shut everything down before transmission has a chance to cleanly shut down. If you're getting an s6 timeout, it likely takes transmission longer than the default 10 seconds to shut down.

That may be an actual issue and a fix is required. Killing an app that is too long to shutdown is normal, it's actually what does systemd about 90 secondes by default. The solution would be to set S6_KILL_FINISH_MAXTIME env variable to something like 10 or 30 seconds as a default (instead of the default 5) and this should actually depends on the user needs.

[LinuxServer-CI]

This pull request has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions[bot]]

This pull request is locked due to inactivity