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

Add ability to preserve host user groups inside container #212

Merged
merged 3 commits into from
May 4, 2023

Conversation

miguelprada
Copy link
Contributor

I have an extension in a personal repository which mounts the host's docker daemon socket into the container and installs the docker CLI. I can then issue docker commands against the host's daemon. However, to use it I need to either use sudo or manually add the user inside the container to the right group matching the host.

With this additional argument, I can automate adding the user inside the container to the same groups the host user belongs to.

Currently:

  • If there's no group in the container with the same name or gid, it will create a new group in the container and add the user to it.
  • It's relatively easy to add the user to groups for which both name and gid match in the host and the container, but I'm not sure whether this makes sense.
  • The case where either a group with the same name or the same gid exits, but not both, is even more difficult for me to understand how to handle.

@miguelprada
Copy link
Contributor Author

Somewhat related to #211, but the approach used there for --group-add does not work in the cases where the group does not already exist inside the image.

@tfoote
Copy link
Collaborator

tfoote commented Apr 10, 2023

Thanks I'd like to take this approach instead of #211 They've verified that this solves their problem.

In the case of the mismatch of groupname and GID I would suggest that we bias towards using the host's GID. We could print a small warning to the console saying there's a potential conflict. And as an advanced feature fix all the file permissions already existing. It's a moderately scriptable process, but potentially expensive computationally/resource wise: https://superuser.com/questions/1736609/how-to-reassign-uidgid-to-all-users-and-automatically-fix-all-file-ownerships-a or https://www.cyberciti.biz/faq/linux-change-user-group-uid-gid-for-all-owned-files/

miguelprada and others added 3 commits April 16, 2023 17:35
…roups on the host that aren't permissible on the target but you'd like best-effort.

Signed-off-by: Tully Foote <tullyfoote@intrinsic.ai>
@miguelprada
Copy link
Contributor Author

I think I hadn't considered all the options in detail in my first comment. Suppose a user belongs to a group mygroup(123). Then two cases are currently handled:

  • There's no group named mygroup or with GID 123 inside the image. This is the case handled in my first commit: a mygroup(123) group is created inside the image and the user is assigned to it.
  • There's a matching mygroup(123) inside the image. Here the user is assigned to this group inside the container. Just added this in 2e6466f

These already solve my use-case with docker and the use-cases of video or dialout discussed in #211. But there three potential cases still remain:

a. There's both othergroup(123) and mygroup(456)
b. There's a othergroup(123) but no mygroup(456).
c. There's a mygroup(456) but no othergroup(123).

I can see adding the user to othergroup(123) in the a and b cases. But should the user be added to someinventedname(123) in c case? How would we go about inventing a group name being sure it doesn't exist?

Or maybe just error out in those three cases? I'm not sure how likely is any of them to happen.

I also don't quite follow your point about fixing file permissions. Care to elaborate?

@miguelprada
Copy link
Contributor Author

miguelprada commented Apr 20, 2023

Oh, and I noticed your branch with the permissive addition and decided to work on top of that and force push. I hope that's ok.

@tfoote
Copy link
Collaborator

tfoote commented Apr 26, 2023

Thanks for picking that up. I made progress but then got distracted by the CI upgrade in #219 and hadn't come back.

My comment about fixing the file permissions would be the case if the image already had the username allocated but we change the GID of the group. To be fully compatible, we would change all files with that group access to use the new GID. But if we leave that as an error case we could skip that for now.

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

Successfully merging this pull request may close these issues.

2 participants