Skip to content

Conversation

@rumpl
Copy link
Owner

@rumpl rumpl commented Sep 8, 2022

- What I did

Fixed exec when specifiying a user:

$ docker run -d --rm -it ubuntu /bin/bash 
fb183991d3b1a2fbcea9d97b49a01cfefd7869ca6a4d428cdd0843853468ec25
$ docker exec -u 1000 fb183991d3b1a2fbcea9d97b49a01cfefd7869ca6a4d428cdd0843853468ec25 id
uid=1000 gid=0(root) groups=0(root)

- How I did it

By getting the container from containerd and asking it to populate the spec with the right user.

@rumpl rumpl force-pushed the exec-user branch 6 times, most recently from d459f61 to 62a2e2d Compare September 8, 2022 14:09
return err
}
opts := []oci.SpecOpts{
coci.WithUser(ec.User),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder this should be guarded by ec.User != nil. Otherwise, if no explicit user is set, this will reset additionalGIDs without any replacement

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's guarded by if len(ec.User) > 0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my bad. 👍

Copy link
Collaborator

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with an open question on nil ec.User

return err
}
opts := []oci.SpecOpts{
coci.WithUser(ec.User),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my bad. 👍

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants