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

Feature request: CLI flag for running container as the current user #3498

Open
kponichtera opened this issue Mar 26, 2022 · 17 comments · May be fixed by #3499
Open

Feature request: CLI flag for running container as the current user #3498

kponichtera opened this issue Mar 26, 2022 · 17 comments · May be fixed by #3499

Comments

@kponichtera
Copy link

There are many use cases where Docker containers are used as development environments to bundle dependencies and provide consistent development experience. Docker daemon uses such an approach as well). In order to allow developers to modify the files in the IDE outside the container, the project’s working directory is often mounted inside the container like this:

$ docker run -v $(pwd):/mount my_container /bin/bash

However, this creates a problem where all the files, created by the process inside the container, are owned by this process’ user, which is often root. Removing root-owned files from the project directory requires juggling sudo commands around and is tedious in general.

Often it’s not even necessary to run the containerized process as root at all, and the following command can be executed to run it as the current user:

$ docker run --user $(id -u):$(id -g) -v $(pwd):/mount my_container /bin/bash

However, this is cumbersome to type in the terminal and bloats the scripts in which it’s used. It would be neat to have a dedicated boolean flag, for instance --current-user, which would fetch the current user’s UID and GID (using Go’s standard library) and use them for running the container’s process.

This would shorten the above example to:

$ docker run --current-user -v $(pwd):/mount my_container /bin/bash
@thaJeztah
Copy link
Member

Thanks for opening this ticket; generally, I can see this being useful (especially for development situations)

I want to discuss with other maintainers on the design/feature.

I was also considering if we add this feature if it would perhaps make sense to define a special case for this on the --user flag. Doing so would prevent the (already long) list of flags to grow, but also simplifies handling (no need to take into account situations where both --user and --current-user are set).

Of course if we go that route, we need to think of that "special" value, and make sure it doesn't (easily) conflict with actual names that someone could use), e.g.

--user=current
--user=current-user
--user=keep

I think BuildKit uses local as special value for docker build --platform (/cc @crazy-max), which makes the build match the local OS and Architecture, so perhaps that could make sense as well as a name for that

--user=local

@thaJeztah
Copy link
Member

/cc @ndeloof @rumpl @tonistiigi perhaps you have ideas as well

@ndeloof
Copy link
Contributor

ndeloof commented Mar 27, 2022

I like this feature and would like we align with buildx and compose on this topic.
Main challenge is to find a reasonable special value to support this use case while not introducing a risk for conflict with some user name in container. Typically I wonder about using local.
My preference goes for --user=current

@thaJeztah
Copy link
Member

Yes, I like current as well (mostly mentioned local as I knew it was used elsewhere, but otherwise current would have my preference as well)

@thaJeztah
Copy link
Member

We should also think about possible combinations of windows cli + linux daemon, windows cli + windows daemon, linux cli + windows daemon etc. (Not sure from the top of my head if Windows daemon support the --user flag, but if it does, it will be in a different format (and most likely will require either an existing user within the container, or a pre-defined user)

@kponichtera
Copy link
Author

Thanks for the suggestion! I didn't think about extending the --user flag mainly due to the potential backwards compatibility issues.

I also find the value current the most fitting here. local sounds more like bound to the local machine, which makes sense for the docker build and local architecture/OS, but not necessarily the user. After all, there might be many local users on the machine. Also, from the perspective of breaking backwards compatibility, I can imagine more people have users called local than current.

I've updated my pull request: #3499. Or should I rather close it and open a new one with a new description?

@thaJeztah
Copy link
Member

I've updated my pull request: #3499. Or should I rather close it and open a new one with a new description?

No, it's perfectly fine to continue on the existing PR; we can review the changes, and make updates on it while we come to consensus on design.

@albers
Copy link
Collaborator

albers commented Mar 28, 2022

Should we consider that current is a valid user name?
Giving this username a special treatment violates the principle of least surprise.

I'd prefer a solution that clearly communicates that we have a special "username" / "username:group" value here.
How about @current?

@ndeloof
Copy link
Contributor

ndeloof commented Mar 28, 2022

@albers I also think this would be better this feature is triggered by some special syntax that can't conflict with an actual user name. --user=$ID would make much sense to me but would obviously be expanded by shell. So maybe --user=@ or something comparable would be an acceptable option

@thaJeztah
Copy link
Member

Yes, it's tricky; to detect if there's a conflict we'd have to create the container first to check if there's a user with that name inside the container.

@ndeloof
Copy link
Contributor

ndeloof commented Mar 28, 2022

so we need to define this special value as something that can't be a user name by POSIX standard.

@albers
Copy link
Collaborator

albers commented Mar 28, 2022

@thaJeztah My concerns are not about a user in the continer. I was thinking about a user "Fred Current" with the user name "current" trying to issue docker run --user=current ..., expecting that this call would only change the uid and not the gid of the container.

I'm against overloading specific legal argument values with special meanings. If an option triggers an alternative operating mode of a command, that fact should be made obvious by a distinct syntax variant.

@ndeloof
Copy link
Contributor

ndeloof commented Mar 28, 2022

I'm not against a new --current-user (or anything else) flag. The number of existing run flags is not an issue imho

@kponichtera
Copy link
Author

The simplest solution without introducing a new flag would be treating --user as boolean when no value is specified and as string whenever it is specified. In the former case it could fetch current user's UID and send it to the daemon, in the latter it would use the provided value instead. I haven't seen something like that done before though and I don't think that's even possible - wouldn't it lead to the ambiguity in the command line grammar?

The @current and @ are still valid usernames from what I tested. I think trying to look for a value that doesn't fall into this category will only give us a weird-looking argument. In case of docker build --platform it was a bit easier, since the platform strings all seem to follow the <OS>/<ARCH> syntax and even if they didn't, the domain of possible inputs is also much smaller.

Also, which do you think would be better: --current-user or --user-current? The first one makes more sense, but the second one will nicely end up next to the --user flag in documentation and when --help is called.

@ndeloof
Copy link
Contributor

ndeloof commented Mar 28, 2022

The simplest solution without introducing a new flag would be treating --user as boolean
Unfortunately the Cobra library we use as command line parser doesn't support this AFAIK

@albers
Copy link
Collaborator

albers commented Mar 28, 2022

treating --user as boolean when no value is specified and as string whenever it is specified.

IMHO, an option should either be boolean or have a value. Mixing both concepts leads to user interfaces that are hard to comprehend.
How should a GUI implement such an option? How should shell completion deal with it?

@kponichtera
Copy link
Author

Unfortunately the Cobra library we use as command line parser doesn't support this AFAIK

It's probably by design, due to that ambiguity - if I was to execute docker run --user container app, how would the CLI know if container is an input to the --user or already a name of the container to run?

How should a GUI implement such an option? How should shell completion deal with it?

It would likely require handling on the GUI side in a way that wouldn't directly correspond to the CLI parameter, which is a bit messy. As for the shell completion - to be honest, I don't know. 😂

I initially liked the idea of reusing --user flag, but seeing how many potential issues it could introduce, I would rather go with --current-user or --user-current. I think it wouldn't introduce that much noise into the CLI. It doesn't even require a shorthand, since it's mostly for specific use cases like development scripts. Also, making --user take precedence whenever both are used would eliminate ambiguities. I can also imagine scripts that dynamically inject the --user flag to the CLI invocation that includes --current-user and otherwise leave just that one option.

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

Successfully merging a pull request may close this issue.

4 participants