c8d: Fix image commit with userns mapping (carry)#46972
c8d: Fix image commit with userns mapping (carry)#46972thaJeztah merged 2 commits intomoby:masterfrom
Conversation
The remapping in the commit code was in the wrong place, we would create a diff and then remap the snapshot, but the descriptor created in "CreateDiff" was still pointing to the old snapshot, we now remap the snapshot before creating a diff. Also make sure we don't lose any capabilities, they used to be lost after the chown. Signed-off-by: Djordje Lukic <[email protected]>
dmcgowan
force-pushed
the
fix-userns-capabilities
branch
from
856f624 to
711b01c
Compare
dmcgowan
changed the title
|
Marking this one as ready to review, I don't see any problems in the tests and any optimizations can come later if necessary. |
thaJeztah
added
containerd-integration
|
Looks like there's a compile failure on Windows; |
dmcgowan
force-pushed
the
fix-userns-capabilities
branch
from
711b01c to
fc25b1b
Compare
| xattrs[xattr] = data | ||
| } | ||
|
|
||
| if err := os.Lchown(path, uid, gid); err != nil { |
There was a problem hiding this comment.
Can be in a follow-up, but considering to add some details about why these attributes are stripped in the first place (I was initially scratching my head on that), and why we have to go against that. Perhaps it's considered "common knowledge" but I wasn't aware (🙈) Djordje posted a link to https://lwn.net/Articles/244747/ on Slack, which made that clear to me.
There was a problem hiding this comment.
That sounds useful and would help me understand it as well.
Signed-off-by: Derek McGowan <[email protected]>
dmcgowan
force-pushed
the
fix-userns-capabilities
branch
from
fc25b1b to
f74ca4e
Compare
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM let's bring this one in
3 participants
Alternative approach to #46938 as mentioned here.