-
Notifications
You must be signed in to change notification settings - Fork 149
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
Docker container creates all files owned by root #439
Comments
I'm not a linux person. Did your PR fix this? |
I only modified the DEB script, not Docker. I haven't tested the new Docker. |
I mean, they both start with D don't they? So does: d'oh! |
@wtanksleyjr didn't realize you already had an issue for this. Can you try following the instruction at https://github.com/rmcrackan/Libation/blob/master/Documentation/Docker.md and see if it works correctly? |
Sure thing! Sorry, I've been struggling with a cold/flu/something.
Hopefully will get time tonight, we'll see if I can stay awake.
…-Wm
On Mon, Jan 23, 2023 at 11:36 AM pixil98 ***@***.***> wrote:
@wtanksleyjr <https://github.com/wtanksleyjr> didn't realize you already
had an issue for this. Can you try following the instruction at
https://github.com/rmcrackan/Libation/blob/master/Documentation/Docker.md
and see if it works correctly?
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7H6MTD2ONUGES4YQQGRTWT3MSDANCNFSM6AAAAAATSZEWMY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@wtanksleyjr any luck on this? |
Oh my! I just got over the flu, so today is a perfect day to do this. I'm
starting right now.
…-Wm
On Sun, Feb 5, 2023 at 5:42 AM rmcrackan ***@***.***> wrote:
@wtanksleyjr <https://github.com/wtanksleyjr> any luck on this?
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7H6IZAZNC7SI7BT2SE2LWV6U6BANCNFSM6AAAAAATSZEWMY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
OK, so ... there are a few problems. The little one is that you're using sudo to run as root, which really shouldn't be the case; this is an audiobook downloader, and shouldn't need root permissions. The big problem is that even when I don't run as root, the payload creates all files using root's account, so you wind up having to use I tried running like this (to run a single fetch, I just bought something at the new sale):
It worked, I got the file; but the resulting file is owned by root. So I have to I'm not sure at all how to fix this kind of thing. I know Docker can handle it, but I don't know how. |
I think I found how to fix this -- I dug around, and the option to use seems to be On the other hand, that clearly won't work on Windows, and I don't know what to do there. So the command that works is:
Notice this is not run with |
@wtanksleyjr Thanks for looking into this. Glad you're feeling better @pixil98 Can you verify? This is not in my wheelhouse |
The @wtanksleyjr if you want to tackle this I'm willing to review the pull request. Heads up that I'll be leaving for vacation in a week and won't be back until mid March. |
1. Unfortunately I'm not going to be using the Docker ... although I like
doing that in general, in this case I just can't make it work for me, and
don't know enough to figure it out ... and the .DEB install works for me.
2. I don't like running as root, though. I think people who install Docker
should _always_ add themselves to the group; it's simply a standard part of
the installation. I don't think we should recommend doing otherwise.
3. I suspect we've got a "good enough" with the recommendation to run it
with the "--user" addition. People who don't remember how to run with a
user will remember; people who don't want to will hopefully have a good
enough reason to (shudder) run as root.
4. Possibly we shouldn't use the currently provided script. Running a
docker with its own built-in cron is kind of a strange thing to do. There
are IMO better ways to do scheduled running, mainly using the system you're
actually using to do the scheduling. Additionally, it's unfortunate that
the main way to run it is a rather inflexible script; it would be better to
just let the user have full access to the CLI, as is done with other Docker
packages (for example, I use `grpcurl` through a Docker package, in order
to let me spoof the DNS using Docker's tools).
5. Likely if we do move forward with this, we should make an archive that
includes everything the user needs except their account files, particularly
generating useful config files. Like how the DEB install does it. All you
should have to do is copy in the account file, and make sure everything's
in the right place.
…-Wm
On Mon, Feb 13, 2023 at 9:29 AM pixil98 ***@***.***> wrote:
The sudo is there in the example because I didn't want to assume that the
user had added themselves to the docker group. I didn't want to end up
owning docker install/config support. That looks like a good way to run the
docker image as the current user and group ids. There is also a directive
that can be put in the Dockerfile that will set default user and group
ids that we should consider. I'm not sure what down stream effects that
would have. The liberate.sh script currently makes some assumptions that
the user it's running as it root and those would need to be updated. I
don't recall if there is a directive for the CLI that lets us pass in a
config path rather than assuming it's at ~/Libation, if not that would
help here.
@wtanksleyjr <https://github.com/wtanksleyjr> if you want to tackle this
I'm willing to review the pull request. Heads up that I'll be leaving for
vacation in a week and won't be back until mid March.
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7H6OGCCONAFY5PWXLVUTWXJVN5ANCNFSM6AAAAAATSZEWMY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@wtanksleyjr Thanks for the update. Should we just close out this ticket? |
Probably - but perhaps comment in the documentation that Docker could use
some work and we welcome advice on how to do it right.
…-Wm
On Thu, Feb 16, 2023 at 4:49 AM rmcrackan ***@***.***> wrote:
@wtanksleyjr <https://github.com/wtanksleyjr> Thanks for the update.
Should we just close out this ticket?
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7H6K5D5E2CC4ASRONICTWXYO4RANCNFSM6AAAAAATSZEWMY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Apologies adding to a closed thread but I'm struggling to get the docker image to map to my local user. My compose file is;
This results in;
If I remove Any advice appreciated. Thanks. |
I think there are a few things going wrong and there's not much that can be done without an MR to make some changes to how the docker image works. The first problem is that it's trying to setup a symlink from /config to where Libation expects the configuration to be, in the user's home directory. It's currently hard coded to be root's home directory. This could be changed to use the current user's home directory, or ideally, we'd just pass in a parameter to the CLI telling it where the config directory is. This would need to be changed in https://github.com/rmcrackan/Libation/blob/master/Docker/liberate.sh and possibly have a new CLI command added. The second problem is that it looks like the /libation folder isn't readable by your user. We create that folder in the Dockerfile, https://github.com/rmcrackan/Libation/blob/master/Dockerfile, on line 19 via the COPY command. We should just add a USER declaration in there to be a non-root user by default and then provide the COPY command with a --chown flag to set the user correctly on it. |
Maybe it would be easier if we didn't include the libation.sh script, and instead included a zipfile containing the basic config that the user is expected to customize and then map to /config (probably the zip would have same files the Debian installer copies into ~/Libation). Anyhow, the user can unzip it, revealing a docker-compose.yml file and a folder containing the config; they can then add a soft-link to their download space at the folder pointed to by the docker-compose, and then copy their own accounts.json file from a machine with graphics support, optionally with other changes for the other config file that has the application settings. They can then do I should try again to see if I can actually do this ... last time I tried I didn't know enough about either docker or how Libation actually works, I've learned a lot since due to the excellent Debian installer. |
I think the unfriendliness of the config is a real problem, but is separate from the non-root issue. Something I've seen done on other containers is to have an init parameter that creates a basic config the first time they're run, then users could go edit that config in the mapped volume and run it again. Another option could be to support at least basic config via envvars and either construct our own config if one isn't provided or ideally the CLI would allow them to be specified as switches (or maybe libation just reads the envvars directly?). |
True, but I was referring to making all of the writable files exist only on
user drive mappings, not inside the container. That way the permissions
don't matter, and the script doesn't do this kind of copying.
…On Thu, Sep 14, 2023 at 12:56 PM pixil98 ***@***.***> wrote:
I think the unfriendliness of the config is a real problem, but is
separate from the non-root issue. Something I've seen done on other
containers is to have an init parameter that creates a basic config the
first time they're run, then users could go edit that config in the mapped
volume and run it again. Another option could be to support at least basic
config via envvars and either construct our own config if one isn't
provided or ideally the CLI would allow them to be specified as switches
(or maybe libation just reads the envvars directly?).
—
Reply to this email directly, view it on GitHub
<#439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJ7H6JGN4UETRB23ZD4NPLX2NOORANCNFSM6AAAAAATSZEWMY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Ah sure, I'm not sure moving away from the entry script is the right move as users would need to know where to mount the directory, it will be in the home folder of the user the docker image is running as internally. A simple fix of updating the script to link it to ~/Libation instead of /root/Libation would probably suffice and keep the mount directory consistent. Longer term I'd rather have the ability to tell the CLI where the libation directory is via switch. @rmcrackan how hard would it be to add a |
In theory not difficult. In practice I've been too busy this summer to do anything and I'm kinda burned out on personal projects in general, hence my absence lately. Not sure about @Mbucari 's availability and/or interest. |
This is a problem created by placing the config directory in a home folder. Make it a fixed mount point like |
I'm a little burned out at the moment too, and I've also got a pretty full plate from now through October 1st. |
I haven't looked at Libation in long enough that I forgot all about appsettings.json. It wouldn't be hard to set some values in it using jq in the docker build. Could also take some elements in as env vars to further customize it at run time. |
Did this ever get solved? |
I don’t think so, it’s been on my list of things to do, but it’s pretty far down. |
In my use case, I am trying to have Libation dump the files directly into a directory that is shared by Nextcloud, which is also resident on the same docker host. In this case, the UID and GID are set to 33, so I have to have Libation run as that UID/GID to ensure it continues to write the files with the same permissions as the existing, Nextcloud-created files. This is what my docker-compose.yml looks like:
So far this seems to be working fine for me. YMMV. I'm sure there's a better fix to the unwritable |
Are you sure the |
They aren't currently available in this image. |
I have a prototype that runs as I've tested deploying it in a fresh kubernetes cluster and (after the tedious manual config) it downloads books as 1001:1001. It also correctly fails to read a database or config owned by root. I tried using a securityContext to override the user to something else and it fails to run. @muchtall, @wtanksleyjr, @robflate - Can you try my image and make sure it's working in your use cases? If so I'll clean it up a bit and submit a PR. Since I'm pretty close to a solution, I'm opening this issue back up so it's easier to find and so I get the satisfaction of closing it. |
I forgot I don't have permission to open arbitrary issues on this repo, @rmcrackan can you reopen this one and assign it to me? |
Thanks @pixil98. How do I run your prototype as user 1000:1000? |
Done :) |
@robflate right now, I don't expect overriding the user to work but you're welcome to try and report back. The method for overriding it will depend on how you're orchestrating the pod. I tested it in a Kubernetes cluster and used a securityContext to change the user. Docker compose should have an analogous concept and I believe the docker command itself can override the user/group via cli flags. From my initial testing, it looks like there are two primary things preventing uid changing from working. The first is that the Libation folder isn't readable (should be easy to fix). The second is that Libation expects to read the config from ~/Libation and the overridden user doesn't have a home directory. The best way to handle this would be if we can tell the libation cli to use a different config folder at runtime either via envvar or cli flag. |
Looking at it a bit more, the problem seems to be that the new user doesn't have write access to the /libation folder where app is actually stored. I have a feeling that is only a problem because it doesn't have a config file and the default config wants to store something in there (maybe books or logs). @rmcrackan how hard would it be to allow overriding the config folder path? |
Done :) The config path you see is a default convenience. It's actually whatever is pointed to by the appsettings.json which lives in the same dir as the main Libation executable assembly. Related: when you launch Libation for the first time, it asks if you're a new user. If you say you're not a new user, you can select your old config dir. |
I bet it’s trying to write out an initial appsettings.json that’s causing the problem. I’ll see about baking one into the docker image. Hopefully that will eliminate the need for write access completely. This is all coming back to me slowly. |
That did it, I have a docker image that can run as any arbitrary user as long as it can read/write the folders that are mounted to /config and /data. I'll clean it up and put together a PR. I'm making some other changes that will be breaking since the permissions thing is already going to be breaking for everyone. I'll write something up in the PR that can be included in the release. @robflate I've repushed the |
@pixil98 Amazing.
Thanks very much for working on this. |
Describe the bug
My log shows that it's not finding
/libation/appsettings.json
, which is odd because it's not supposed to be looking there. It's supposed to be looking in/config/Settings.json
, given how the Docker script is written. (I admit I'm speculating.)To Reproduce
Here's my docker-compose yml (unfortunately this editor flattened it, never seen it do that before!):
Expected behavior
I expected it to load my Settings.json file; instead it tries and fails to load a nonexistent file inside the container, that I can't map to.
Screenshots n/a
Platform Ubuntu Docker-Compose
Log Files
I really don't know how to get Docker log files.... Here's the backtrace.
The text was updated successfully, but these errors were encountered: