[RFC] overlay: make sure directories omitted from layers have the right permissions #1653
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,106 @@ | ||||||
| package unionbackfill | ||||||
|
|
||||||
| import ( | ||||||
| "archive/tar" | ||||||
| "io/fs" | ||||||
| "os" | ||||||
| "path" | ||||||
| "path/filepath" | ||||||
| "strings" | ||||||
|
|
||||||
| "github.com/containers/storage/pkg/archive" | ||||||
| "github.com/containers/storage/pkg/idtools" | ||||||
| "github.com/containers/storage/pkg/system" | ||||||
| ) | ||||||
|
|
||||||
| // NewBackfiller supplies a backfiller whose Backfill method provides the | ||||||
| // ownership/permissions/attributes of a directory from a lower layer so that | ||||||
| // we don't have to create it in an upper layer using default values that will | ||||||
| // be mistaken for a reason that the directory was pulled up to that layer. | ||||||
| func NewBackfiller(idmap *idtools.IDMappings, lowerDiffDirs []string) *backfiller { | ||||||
|
There was a problem hiding this comment. Should this be a public c/storage API, or would something like (I don’t have a strong opinion, I just want this to be an intentional decision.) There was a problem hiding this comment. If |
||||||
| if idmap != nil { | ||||||
| uidMaps, gidMaps := idmap.UIDs(), idmap.GIDs() | ||||||
| if len(uidMaps) > 0 || len(gidMaps) > 0 { | ||||||
| idmap = idtools.NewIDMappingsFromMaps(append([]idtools.IDMap{}, uidMaps...), append([]idtools.IDMap{}, gidMaps...)) | ||||||
| } | ||||||
| } | ||||||
| return &backfiller{idmap: idmap, lowerDiffDirs: append([]string{}, lowerDiffDirs...)} | ||||||
|
Comment on lines
+24
to
+27
There was a problem hiding this comment. (Absolutely non-blocking: This might be a tiny bit more readable when using |
||||||
| } | ||||||
|
|
||||||
| type backfiller struct { | ||||||
| idmap *idtools.IDMappings | ||||||
| lowerDiffDirs []string | ||||||
| } | ||||||
|
|
||||||
| // Backfill supplies the ownership/permissions/attributes of a directory from a | ||||||
| // lower layer so that we don't have to create it in an upper layer using | ||||||
| // default values that will be mistaken for a reason that the directory was | ||||||
| // pulled up to that layer. | ||||||
| func (b *backfiller) Backfill(pathname string) (*tar.Header, error) { | ||||||
| for _, lowerDiffDir := range b.lowerDiffDirs { | ||||||
| candidate := filepath.Join(lowerDiffDir, pathname) | ||||||
|
There was a problem hiding this comment. At least … actually even with a |
||||||
| // if the asked-for path is in this lower, return a tar header for it | ||||||
|
There was a problem hiding this comment. I can’t think of anything that would rule out us finding a whiteout. I’m not sure that needs a special handling, but at least I think this can exit early because using any of the whiteout’s, or pre-whiteout directory’s, permissions would not be beneficial. |
||||||
| if st, err := os.Lstat(candidate); err == nil { | ||||||
| var linkTarget string | ||||||
| if st.Mode()&fs.ModeType == fs.ModeSymlink { | ||||||
| target, err := os.Readlink(candidate) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| linkTarget = target | ||||||
| } | ||||||
| hdr, err := tar.FileInfoHeader(st, linkTarget) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| // this is where we'd delete "opaque" from the header, if FileInfoHeader read xattrs | ||||||
| hdr.Name = strings.Trim(filepath.ToSlash(pathname), "/") | ||||||
| if st.Mode()&fs.ModeType == fs.ModeDir { | ||||||
| hdr.Name += "/" | ||||||
| } | ||||||
| if b.idmap != nil && !b.idmap.Empty() { | ||||||
| if uid, gid, err := b.idmap.ToContainer(idtools.IDPair{UID: hdr.Uid, GID: hdr.Gid}); err == nil { | ||||||
| hdr.Uid, hdr.Gid = uid, gid | ||||||
| } | ||||||
| } | ||||||
| return hdr, nil | ||||||
|
Comment on lines
+44
to
+66
There was a problem hiding this comment. Again a vague feeling this probably should be shared. At least |
||||||
| } | ||||||
| // if the directory or any of its parents is marked opaque, we're done looking at lowers | ||||||
| p := strings.Trim(pathname, "/") | ||||||
| subpathname := "" | ||||||
| for { | ||||||
| dir, subdir := filepath.Split(p) | ||||||
| dir = strings.Trim(dir, "/") | ||||||
| if dir == p { | ||||||
| break | ||||||
| } | ||||||
| // kernel overlay style | ||||||
| xval, err := system.Lgetxattr(filepath.Join(lowerDiffDir, dir), archive.GetOverlayXattrName("opaque")) | ||||||
| if err == nil && len(xval) == 1 && xval[0] == 'y' { | ||||||
| return nil, nil | ||||||
| } | ||||||
| // aufs or fuse-overlayfs using aufs-like whiteouts | ||||||
| if _, err := os.Stat(filepath.Join(lowerDiffDir, dir, archive.WhiteoutOpaqueDir)); err == nil { | ||||||
| return nil, nil | ||||||
| } | ||||||
| // kernel overlay "redirect" - starting with the next lower layer, we'll need to look elsewhere | ||||||
| subpathname = strings.Trim(path.Join(subdir, subpathname), "/") | ||||||
| xval, err = system.Lgetxattr(filepath.Join(lowerDiffDir, dir), archive.GetOverlayXattrName("redirect")) | ||||||
|
There was a problem hiding this comment. Do we need to worry about images shipping this attribute in layers? At least from a quick check of (Ultimately, AFAICS the path always goes through [1] Not actually, but that can be fixed elsewhere. |
||||||
| if err == nil && len(xval) > 0 { | ||||||
| subdir := string(xval) | ||||||
|
There was a problem hiding this comment.
Suggested change
or some other name expressing the relationship to |
||||||
| if path.IsAbs(subdir) { | ||||||
| // path is relative to the root of the mount point | ||||||
| pathname = path.Join(subdir, subpathname) | ||||||
| } else { | ||||||
| // path is relative to the current directory | ||||||
| parent, _ := filepath.Split(dir) | ||||||
| parent = strings.Trim(parent, "/") | ||||||
| pathname = path.Join(parent, subdir, subpathname) | ||||||
| } | ||||||
| break | ||||||
| } | ||||||
| p = dir | ||||||
| } | ||||||
| } | ||||||
| return nil, nil | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some documentation of what this field contains would be nice.