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

Standardise method for mapping uid/gid inside the container with uid/gid outside the container #5

Open
lucymhdavies opened this issue Jan 25, 2018 · 8 comments

Comments

@lucymhdavies
Copy link

lucymhdavies commented Jan 25, 2018

For use with gosu or su-exec, e.g.
https://denibertovic.com/posts/handling-permissions-with-docker-volumes/

On a linux host, if you bind a volume from the host into the container, but then run as root in the container, any new files will be owned by root, both inside the container and outside.

To work around this, you need a specially crafted docker image.
e.g.
lucymhdavies/docker-images@b4f9b85

And you need to set environment vars when running the container

e.g. in lucli I have used HOST_USER_ID and HOST_GROUP_ID
https://github.com/LMHD/lucli/blob/52fd09f647706af0939e90d9620e4a52b8a8204f/cmd/vim.go#L16-L21

	u, err := user.Current()
	if err != nil {
		log.Fatalf("Failed to find uid for user: %s", err)
	}
	task.AddEnv("HOST_USER_ID", u.Uid)
	task.AddEnv("HOST_GROUP_ID", u.Gid)

Fix:

While we can't 'fix' the issue, we can standardise what appears to be the popular workaround.

By default, Cali should set HOST_USER_ID and HOST_GROUP_ID environment variables for all containers it runs.

These environment variables should also be configurable in the app which uses the Cali library, and configurable on each individual task too.

@lucymhdavies
Copy link
Author

lucymhdavies commented Feb 11, 2018

Some overlap with #6
Need to do something with the Git image when doing this too.

e.g.

lucli vim -g [email protected]:lucymhdavies/skybet.github.io.git CNAME

Running :shell to get to a sh session, I see that /tmp/workspace and the files in there are owned by root, which is the user that the git container used to clone the repo.

So... Need to make sure that any standard solution to this issue work both when $PWD is bound to /tmp/workspace, as well as when /tmp/workspace comes from a sidekick container.

Potentially the default solution here is that when you're running a container against a git repo, do not pass in user ids into either the git container or the task container. Rather, just require that the uid inside both containers match.

Again though, this should be configurable globally to the app, as well as to the individual task.

@lucymhdavies
Copy link
Author

Also, kinda related, but we seriously need a better way of doing this than passing in environment variables.

For example:
https://docs.docker.com/engine/security/userns-remap/
(which may or may not be an appropriate solution for all users)

@lucymhdavies
Copy link
Author

Also worth considering not passing in any uid at all on OSs which do not require it. e.g. Docker for Mac does this uid mapping for you, so Cali should (optionally) detect that and refrain from passing in UID/GID in this case.

@lucymhdavies lucymhdavies changed the title Standarise HOST_USER_ID and HOST_GROUP_ID environment variables Standardise method for mapping uid/gid inside the container with uid/gid outside the container Feb 13, 2018
@edupo
Copy link

edupo commented Mar 26, 2018

I have a possible fix to this issue. My code has been heavily refactored, but the fix is only one additional function.

The logic is simple since is something I already did as bash scripts a while ago:

  • Run the container with a void process such as sleep infinite
  • Exec a fixing script generated by cali inside the running container.
    The script looks pretty much as some repeated scripts I saw in other repos, but it can be easily generalized.
  • Exec the user command in the fixed container.

Right now the fix applies every execution of cali but I think a future improvement may be committing the container and only rebuild the container when an upstream update happens...

@lucymhdavies
Copy link
Author

lucymhdavies commented Mar 26, 2018

Oh, that's cool.

That script looks similar to what I'm doing in my Cali app.
https://github.com/lucymhdavies/docker-images/blob/latest/docker.io/lucymhdavies/vim/entrypoint.sh
https://github.com/LMHD/lucli/blob/master/cmd/vim.go#L16-L21

Think we got that idea from the same place :)

I would say we probably don't want to include that in every container by default (given a container could contain anything), but maybe have it as an option.

Such a script would need customising based on the base OS of the container, e.g. alpine vs centos vs debian vs whatever (alpine:latest for example doesn't have usermod or groupadd until you install shadow). But even so, this should be fairly common to most things.

I like the idea of committing the container, for future uses on the same machine too.

@edupo
Copy link

edupo commented Mar 27, 2018

I got it from staticli, I guess you and @wheresalice are working together in this? :)

As I see it, cali is a framework to develop CLI's, and from every CLI I use I expect it to run as my own user. Use case: Creating reports as root is not doing any good to our CI xD
That is why I developed it as the default behavior.

Another solution is to offer it as an additional entrypoint of the API so you can do something like

task.FixUser()

Of course the script should fix several types of platforms, but that shouldn't be hard and is easy to expand.

@edupo
Copy link

edupo commented Mar 27, 2018

Is not the most elegant solutions but works now for me:
A general fix script template:
https://github.com/edupo/cali/blob/fix-image/static/fix.sh.template
A fix function that injects it into any container:
https://github.com/edupo/cali/blob/3eddce060ececa2790fe6908b9f7441aac40fc3f/docker/container.go#L109

@edupo
Copy link

edupo commented Apr 3, 2018

The users remapping flag involves touching the docker daemon. I think is not an option for this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants