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.

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

--update-mode=exec/container inconsistent file permissions #3708

Closed
leoluk opened this issue Aug 16, 2020 · 8 comments · Fixed by #4623
Closed

--update-mode=exec/container inconsistent file permissions #3708

leoluk opened this issue Aug 16, 2020 · 8 comments · Fixed by #4623
Labels
enhancement New feature or request

Comments

@leoluk
Copy link

leoluk commented Aug 16, 2020

Problem

We use a Dockerfile that runs as non-root (i.e. USER 1000).

With --update-mode=container, synced files are - presumably - created via docker exec as root and can't be read by the application user because they're owned by root.

With --update-mode=exec, k8s defaults to the container user and everything works as expected.

See also: #3060

Proposed Solution

Always use the container user to sync files

@maiamcc
Copy link
Contributor

maiamcc commented Aug 17, 2020

Thanks for the report @leoluk! Let me dig into how this looks in our code and get back to you.

@maiamcc
Copy link
Contributor

maiamcc commented Aug 18, 2020

Am I right in thinking that you set USER 1000 early in Dockerfile execution, such that any files you copy into your image are owned by user 1000? Your proposed solution works great here, but conflicts with the needs of #3060 where a user is creating all the files in their image as root, and only switching to a non-root user to execute the entrypoint: in their case, syncing files as the container user causes permission errors as Tilt attempts to overwrite a file owned by root as a non-root user.

I agree that the different file permissions behavior between the two update modes isn't great :-/ but unfortunately a blanket solution of either "run all syncs/runs as root," OR "run all syncs and runs as the container user" is going to fail for some set of users.

I might instead propose a Tiltfile-wide setting a la update_settings(use_container_user=True) to tell Tilt to run live updates (syncs and runs) with the container user, if present--would that work for your use case?

Unfortunately, I'm not sure we'll be able to prioritize this feature in the near term; does the workaround of running with --update-mode=exec work for you for the time being?

@maiamcc maiamcc added the enhancement New feature or request label Aug 18, 2020
@leoluk
Copy link
Author

leoluk commented Aug 19, 2020

Am I right in thinking that you set USER 1000 early in Dockerfile execution, such that any files you copy into your image are owned by user 1000?

I might instead propose a Tiltfile-wide setting a la update_settings(use_container_user=True) to tell Tilt to run live updates (syncs and runs) with the container user, if present--would that work for your use case?

Yes, the files in the container are owned by the unprivileged user.

Agreed that it should be configurable.

Unfortunately, I'm not sure we'll be able to prioritize this feature in the near term; does the workaround of running with --update-mode=exec work for you for the time being?

Yes, the workaround works just fine.

@mrsimo
Copy link

mrsimo commented Feb 12, 2021

We're using Tilt with a docker-compose based setup (everything has been working perfectly), and we have this problem, only --update-mode=exec doesn't seem to apply to docker-compose. Files are copied as root, messing up the whole thing.

Would you have a workaround for this? I'm out of ideas at the moment.

I might instead propose a Tiltfile-wide setting a la update_settings(use_container_user=True)

I'd be happy with a parameter in the sync method to specify the user and the group as well.

@TimothyLoyer
Copy link

I just want to add my voice to those having issues with live updating using the root user rather than the user specified in the Docker file. My employer prefers us to use non-root users in our containers when possible, and having a process writing files as root is mangling our local dev environments.

@maiamcc
Copy link
Contributor

maiamcc commented May 12, 2021

I just want to add my voice to those having issues with live updating using the root user rather than the user specified in the Docker file. My employer prefers us to use non-root users in our containers when possible, and having a process writing files as root is mangling our local dev environments.

@TimothyLoyer that makes sense! If you're running on Kubernetes, a workaround is to invoke Tilt with --update-mode=exec (though as @mrsimo notes, that won't work with Tilt + Docker Compose).

@nicks
Copy link
Member

nicks commented Jun 8, 2021

I investigated this a bit.

There's a couple of crisscrossing concerns here. The big one is that docker cp broke how they do UID mapping. See discussion here:
moby/moby#34096

i.e., if you docker cp a file, it will simply use the UIDs that the file has on the host machine, even if they map to completely different users on the host machine vs inside the container.

So whatever we do, we'll have to do the UID mapping ourselves. There's a few ways to do that.

IMO, the best way to handle this is to use a docker exec API (similar to how we use kubectl exec API to copy files), and not use the docker cp APIs at all. The Docker APIs that map directly to the CRI tend to be very heavily used and battle-hardened...the ones that don't map to the CRI tend to be...softer.

@nicks
Copy link
Member

nicks commented Jun 10, 2021

this should be fixed in 0.20.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants