skybet / cali

Cali Automation Layout Initialiser
MIT License
32 stars 7 forks source link

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

Open lucymhdavies opened 6 years ago

lucymhdavies commented 6 years ago

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. https://github.com/lucymhdavies/docker-images/commit/b4f9b85aec3e507aa719c346bcffa808f88599e4

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 commented 6 years ago

Some overlap with https://github.com/skybet/cali/issues/6 Need to do something with the Git image when doing this too.

e.g.

lucli vim -g git@github.com: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 commented 6 years ago

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 commented 6 years ago

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.

edupo commented 6 years ago

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:

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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