c8d: Set the process user on exec #45267
Conversation
rumpl
added
area/runtime
|
Converting to draft for now, this one needs this too rumpl#92 |
|
I opened a PR in containerd containerd/containerd#8351 |
This change makes is possible to run `docker exec -u <UID> ...` when the containerd integration is activated. Signed-off-by: Djordje Lukic <[email protected]>
Uses containerd from release/1.6 commit containerd/containerd@c0efc63 Signed-off-by: Djordje Lukic <[email protected]>
|
The readonly mount in containerd was merged in master and 1.6 and 1.7 release branches, I updated the dependency to containerd/containerd@c0efc63, exec-ing into a container with a user now works and doesn't put the mounts into an undefined state. |
| if err != nil { | ||
| return err | ||
| if daemon.UsesSnapshotter() { | ||
| p.User, err = getUserFromContainerd(ctx, daemon.containerdCli, ec) |
There was a problem hiding this comment.
Could this one also work for !daemon.UsesSnapshotter() branch?
There was a problem hiding this comment.
Ok just tested and indeed this can work for both cases, will update
There was a problem hiding this comment.
I'd rather we didn't actually, we can do it in the future once we are sure that the two ways of getting the users and groups are equivalent, I can add a TODO though
There was a problem hiding this comment.
Yeah it might be better not to touch the non-snapshotter case. I was just asking out of curiosity 😄
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM (I guess technically the commits should have been in the reverse order, but the first one "builds", so I guess it's fine)
3 participants
- What I did
Set the process user on exec
This change makes is possible to run
docker exec -u <UID> ...when the containerd integration is activated.- How I did it
We load the container spec and change its user and then set the exec process user.
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)