imagegenius / docker-immich

Monolithic (Single) Docker Container for Immich
GNU General Public License v3.0
583 stars 26 forks source link

Container requires ability to change ownership of photo library to run #112

Closed aweebs closed 1 year ago

aweebs commented 1 year ago

I store my photos on a NAS and the network drive directory is mounted to my Immich docker container's /photo directory. Because of the way the network drive is mounted to the host OS, the directory is owned by root:root and the permissions are completely open (777). However, the ownership and permissions cannot be changed, the OS will not allow it. Trying to change the ownership results in an error, even as root.

When trying to start the Immich docker container, it fails after trying to change the owner on all files in the /photo directory. I believe the culprit is this line https://github.com/imagegenius/docker-immich/blob/main/root/etc/s6-overlay/s6-rc.d/init-config-immich/run#L25 since it will result in a return code of 1 instead of the expected 0.

Last few log lines from the Immich docker container:

chown: changing ownership of '/photos/library': Operation not permitted
chown: changing ownership of '/photos': Operation not permitted
s6-rc: warning: unable to start service init-config-immich: command exited 1

When mounting a regular directory to /photos the docker container starts up and works without issues.

Running chown on a mounted volume seems a bit odd. Should that be removed in favour of expecting users to setup their permissions correctly?

martabal commented 1 year ago

Did you try to use PUID and PGID ? You can find the explanation why we use chown here

hydazz commented 1 year ago

Did you try to use PUID and PGID ? You can find the explanation why we use chown here

changing the PUID and PGID will have no effect, the mount does not support changing permissions, so itll never work.

Running chown on a mounted volume seems a bit odd. Should that be removed in favour of expecting users to setup their permissions correctly?

far from the facts, we run chown on anything the immich services touch, as they run unprivileged, without applying proper permissions immich wont be able to write to the filesystem - by design, so immich does not run as root, which is the same behaviour for almost any docker container

martabal commented 1 year ago

@hydazz Running Immich as root can't do the trick, even if it's not ideal ?

aweebs commented 1 year ago

Running chown on a mounted volume seems a bit odd. Should that be removed in favour of expecting users to setup their permissions correctly?

far from the facts, we run chown on anything the immich services touch, as they run unprivileged, without applying proper permissions immich wont be able to write to the filesystem - by design, so immich does not run as root, which is the same behaviour for almost any docker container

Thanks for responding! I run several docker images, many from linuxserver, which have a directories from my NAS mounted into them. They also run as a specific user/group configured through environment variables, just like Immich. None of them try to change ownership of mounted volumes. My assumption would be that if I have mounted a volume and set a specific user id and group id to run as then the container shouldn't have to change the permissions on it, that should be on me to configure properly.

aweebs commented 1 year ago

@hydazz Running Immich as root can't do the trick, even if it's not ideal ?

@martabal sadly it does not. The chown command will fail no matter what when running on this directory, even as a root user. The issue is with running chown on the mounted volume as part of the startup script at all.

hydazz commented 1 year ago

Looks like you have 2 options;

make your own copy of etc/s6-overlay/s6-rc.d/init-config-immich /run, and remove the chown, mount it via a standard docker mount.

or explore other mounting solutions for your photos library that supports permissions

aweebs commented 1 year ago

I've successfully got around it with a modified init-config-immich/run and it's working. As mentioned previously, I haven't had this issue with any other docker container that I've mounted a directory from a network drive into, so I still think it's odd to chown an entire mounted data directory and it should be reconsidered if it's necessary.

martabal commented 1 year ago

Well, by changing the permissions of the /photos folder, we ensure that the abc user has read and write rights on all the files in the folder and avoid any potential security problems

aweebs commented 1 year ago

Well, by changing the permissions of the /photos folder, we ensure that the abc user has read and write rights on all the files in the folder and avoid any potential security problems

I understand that, my point is that it's a mounted directory which I have configured to have the same permission as the user which I have also configured to container to run as. This isn't an issue for most docker containers, so this feels like a reasonable expectation to make. Would you be open to me creating a PR that changes the script such that, specifically for the /photos directory, if it exists already and has the correct owners then ownership would not be recursively set?

hydazz commented 1 year ago

Well, by changing the permissions of the /photos folder, we ensure that the abc user has read and write rights on all the files in the folder and avoid any potential security problems

I understand that, my point is that it's a mounted directory which I have configured to have the same permission as the user which I have also configured to container to run as. This isn't an issue for most docker containers, so this feels like a reasonable expectation to make.

Would you be open to me creating a PR that changes the script such that, specifically for the /photos directory, if it exists already and has the correct owners then ownership would not be recursively set?

Feel free to open a PR, we are happy to consider anything as long as it does not introduce any complications for the average user

rexzhang commented 1 year ago

Well, by changing the permissions of the /photos folder, we ensure that the abc user has read and write rights on all the files in the folder and avoid any potential security problems

I understand that, my point is that it's a mounted directory which I have configured to have the same permission as the user which I have also configured to container to run as. This isn't an issue for most docker containers, so this feels like a reasonable expectation to make. Would you be open to me creating a PR that changes the script such that, specifically for the /photos directory, if it exists already and has the correct owners then ownership would not be recursively set?

Yes! When there are many pictures, each time the start time is unbearably

rexzhang commented 1 year ago

Is it possible to allow adding an environment variable to control the chown behavior? If so, I can do this

martabal commented 1 year ago

Is it possible to allow adding an environment variable to control the chown behavior? If so, I can do this

it's definitely doable

aweebs commented 1 year ago

For other's searching, the original issue I posted was fixed by @martabal in PR #125. This resolves my issue by only ignoring chown permission failures for the /pictures directory. Closing the issue since it's been fixed.