Use container user details to create new volumes. Fixes #5698 #6262
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sujil02 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
sujil02
force-pushed
the
user-kepid
branch
2 times, most recently
from
746db38 to
addd7ee
Compare
| if err != nil { | ||
| volOptions = []VolumeCreateOption{WithVolumeName(vol.Name), WithVolumeUID(int(uid)), WithVolumeGID(int(gid))} | ||
| } else { | ||
| volOptions = []VolumeCreateOption{WithVolumeName(vol.Name), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())} |
There was a problem hiding this comment.
Why the fallback here? Do we expect this to fail?
There was a problem hiding this comment.
It seems like this might be to handle cases where ctr.config.User is unset, but we should explicitly handle that case, not do a catch-all for errors.
sujil02
force-pushed
the
user-kepid
branch
2 times, most recently
from
5ed8d26 to
c49c434
Compare
Adds the Use container user details to create new volume while running a container Also Adds relevant test cases. Signed-off-by: Sujil02 <[email protected]>
| if ctr.config.User == "" { | ||
| volOptions = []VolumeCreateOption{WithVolumeName(vol.Name), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())} | ||
| } else { | ||
| uid, gid, _, err := chrootuser.GetUser(ctr.state.Mountpoint, ctr.config.User) |
There was a problem hiding this comment.
Looking at this further, we aren't actually mounted when this call happens. We need to mount the container first, then immediately unmount after this finishes.
|
@sujil02 What is the state of this PR? Lets get this finished, to close the issue. |
Yes i am working on this |
|
Friendly ping. @sujil02, are you willing to tackle this beyond the internship? |
|
@vrothberg Could you take this over, since it fixes a bug. We need to get it done. |
Will do. Thanks a lot for your great work, @sujil02 ! |
@vrothberg thank you for picking it. Sorry could not get this in time. Basically the issue that held this was that the container is not mounted when the volume was created hence picking details using chroot users might be the best solution. |
github-actions
bot
added
the
locked - please file new issue/PR
Use container user details to create new volumes.
Adds relevant test cases.
Adds relevant test cases.
Fixes #5698
Signed-off-by: Sujil02 [email protected]