archive: store the override xattr with the inode type#2183
archive: store the override xattr with the inode type#2183openshift-merge-bot[bot] merged 6 commits intocontainers:mainfrom
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 |
3e313eb to
f5153fb
Compare
f5153fb to
18c73a2
Compare
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?
18c73a2 to
054a550
Compare
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
98ff561 to
b2ddf4f
Compare
|
pushed a new version, and verified manually both partial pulls and with regular pulls |
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) .
b2ddf4f to
6a69308
Compare
mtrmac
left a comment
There was a problem hiding this comment.
Don’t SafeChown/SafeLchown still need a bit of an update?
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. |
45adba0 to
89b73d0
Compare
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Fixes: containers#2174 Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
89b73d0 to
84e7502
Compare
|
@rhatdan PTAL |
|
/lgtm |
Closes: #2174