archive: store the override xattr with the inode type #2183
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
related fuse-overlayfs change: containers/fuse-overlayfs#434 |
giuseppe
force-pushed
the
extend-override-xattr
branch
4 times, most recently
from
3e313eb to
f5153fb
Compare
giuseppe
force-pushed
the
extend-override-xattr
branch
from
f5153fb to
18c73a2
Compare
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
ACK conceptually, but there seems even more to be done.
- symbolic links are now not handled consistently.
check.gointerpretsforce_mask; it needs to be taught about this- Doesn’t the code in
overlay.Driver.createaround creatingdiffneed to also setos.ModeDirnow? - The parser in
pkg/archive.remapIDsseems to need updating (preferably consolidate all parsers to one function)
(It gets confusing… if only the OS had separate types for “permission bits” and “the full mode including a ModeType.)
pkg/idtools/idtools.go
Outdated
| } | ||
| } | ||
| value := Stat{IDPair{uid, gid}, mode} | ||
| value := Stat{IDPair{uid, gid}, mode, 0, 0} |
There was a problem hiding this comment.
Does this need to handle file types?
pkg/idtools/idtools.go
Outdated
| } | ||
| } | ||
| value := Stat{IDPair{uid, gid}, mode} | ||
| value := Stat{IDPair{uid, gid}, mode, 0, 0} |
There was a problem hiding this comment.
Does this need to handle file types?
giuseppe
force-pushed
the
extend-override-xattr
branch
from
18c73a2 to
054a550
Compare
Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
extend-override-xattr
branch
4 times, most recently
from
98ff561 to
b2ddf4f
Compare
|
pushed a new version, and verified manually both partial pulls and with regular pulls |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
ACK to the symlink semantics change.
Note also the outstanding items in the top-level review comment in #2183 (review) .
giuseppe
force-pushed
the
extend-override-xattr
branch
from
b2ddf4f to
6a69308
Compare
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Don’t SafeChown/SafeLchown still need a bit of an update?
giuseppe
force-pushed
the
extend-override-xattr
branch
2 times, most recently
from
c11fa65 to
45adba0
Compare
I've changed them further. I don't really know how they are used on darwin. I see the same data is also set on the file itself. |
|
IIRC the general idea has been to allow unprivileged macOS users to extract volumes, but I’d need to look up the git history to confirm. |
giuseppe
force-pushed
the
extend-override-xattr
branch
from
45adba0 to
89b73d0
Compare
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Fixes: containers#2174 Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
extend-override-xattr
branch
from
89b73d0 to
84e7502
Compare
|
@rhatdan PTAL |
|
/lgtm |
3 participants
Closes: #2174