From f9896deae21a0accfee8044f7ab96294c66ed46c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 23 Apr 2026 13:50:55 +0200 Subject: [PATCH 1/6] Collect constants in drivers/overlay by purpose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- storage/drivers/overlay/overlay.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 7647bf2454..de8b55c450 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -48,8 +48,7 @@ import ( var untar = chrootarchive.UntarUncompressed const ( - defaultPerms = os.FileMode(0o555) - mountProgramFlagFile = ".has-mount-program" + defaultPerms = os.FileMode(0o555) ) // This backend uses the overlay union filesystem for containers @@ -79,24 +78,32 @@ const ( // syscall. A hard upper limit of 500 lower layers is enforced to ensure // that mounts do not fail due to length. -const ( - linkDir = "l" - stagingDir = "staging" - tempDirName = "tempdirs" - lowerFile = "lower" +const ( // Paths within the driver’s home directory + mountProgramFlagFile = ".has-mount-program" + linkDir = "l" + stagingDir = "staging" + tempDirName = "tempdirs" +) + +const ( // Paths within a per-layer directory + lowerFile = "lower" // lowerLayersFile references lower layers directly by layer ID // instead of going through the l/ symlinks. The code appends // "/diff" itself when consuming entries. It is preferred over // lowerFile when present. The old lowerFile is still written // for backward compatibility with older tools. lowerLayersFile = "lower-layers" - maxDepth = 500 - - stagingLockFile = "staging.lock" +) +const ( // Keys within DriverWithDifferOutput.Artifacts tocArtifact = "toc" fsVerityDigestsArtifact = "fs-verity-digests" +) +const stagingLockFile = "staging.lock" + +const ( + maxDepth = 500 // idLength represents the number of random characters // which can be used to create the unique link identifier // for every layer. If this value is too long then the From 4f46ac63355bce6579b539dab02d4cbefa852421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 23 Apr 2026 13:54:41 +0200 Subject: [PATCH 2/6] Document getLower and rename it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... to be less confusing with a future getLowerLayerIDs. Should not change behavior. Signed-off-by: Miloslav Trmač --- storage/drivers/overlay/overlay.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index de8b55c450..9395fd2574 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -1152,7 +1152,7 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl return idtools.MkdirAndChown(path.Join(dir, "empty"), 0o700, forcedSt.IDs) } - lower, err := d.getLower(parent) + lower, err := d.getLowerForParent(parent) if err != nil { return err } @@ -1238,7 +1238,11 @@ func (d *Driver) parseStorageOpt(opts *graphdriver.CreateOpts, readOnly bool) (q return res, nil } -func (d *Driver) getLower(parent string) (string, error) { +// getLowerForParent returns the contents of lowerFile for a child layer of parent. +// +// This should only be used to construct a lowerFile for compatibility; +// new code should rely on lowerLayersFile instead. +func (d *Driver) getLowerForParent(parent string) (string, error) { parentDir := d.dir(parent) // Ensure parent exists From 727feeeb701dec19c0314c0b78a7280247f257a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 23 Apr 2026 14:34:27 +0200 Subject: [PATCH 3/6] Remove an always-true condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getLowerForParent never returns "" on success (and we are on a branch where parent != ""). Should not change behavior. Signed-off-by: Miloslav Trmač --- storage/drivers/overlay/overlay.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 9395fd2574..63c52c4025 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -1156,10 +1156,8 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl if err != nil { return err } - if lower != "" { - if err := os.WriteFile(path.Join(dir, lowerFile), []byte(lower), 0o666); err != nil { - return err - } + if err := os.WriteFile(path.Join(dir, lowerFile), []byte(lower), 0o666); err != nil { + return err } // Write a lower-layers file referencing layers by ID instead of From 7fac8c23e36d557fc5655e5752c1ecebc225cd89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 23 Apr 2026 15:19:16 +0200 Subject: [PATCH 4/6] Consolidate the lowerFile fallback to getLowerLayerIDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids Readlink() on layer IDs we _know_ are not links (and a risk of Readlink() actually finding something at that relative path), and simplifies users to again use unambiguous data contents. Then we can also use ordinary d.dir() to look up the directory to Stat(). Signed-off-by: Miloslav Trmač --- storage/drivers/overlay/overlay.go | 138 +++++++++++------------------ 1 file changed, 51 insertions(+), 87 deletions(-) diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 63c52c4025..1b5f0ee7b1 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -1162,30 +1162,12 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl // Write a lower-layers file referencing layers by ID instead of // l/ symlink references. The reading side appends "/diff" itself. - parentDir := d.dir(parent) - layerLower := parent - parentLower, err := os.ReadFile(path.Join(parentDir, lowerLayersFile)) - if err == nil { - layerLower += ":" + string(parentLower) - } else if !errors.Is(err, unix.ENOENT) { + parentLowerLayerIDs, err := d.getLowerLayerIDs(parent) + if err != nil { return err - } else { - // Parent has no lower-layers file. Convert old-format lower - // entries (l/ symlinks) to layer IDs. - oldLower, err := os.ReadFile(path.Join(parentDir, lowerFile)) - if err == nil { - for _, s := range strings.Split(string(oldLower), ":") { - target, err := os.Readlink(d.dir(s)) - if err != nil { - return fmt.Errorf("reading symlink for lower %q: %w", s, err) - } - layerLower += ":" + filepath.Base(filepath.Dir(target)) - } - } else if !errors.Is(err, unix.ENOENT) { - return err - } } - if err := os.WriteFile(path.Join(dir, lowerLayersFile), []byte(layerLower), 0o666); err != nil { + layerLowerLayerIDs := strings.Join(append([]string{parent}, parentLowerLayerIDs...), ":") + if err := os.WriteFile(path.Join(dir, lowerLayersFile), []byte(layerLowerLayerIDs), 0o666); err != nil { return err } @@ -1303,33 +1285,51 @@ func (d *Driver) dir2(id string, useImageStore bool) (string, string, bool) { return newpath, homedir, false } -func (d *Driver) getLowerDirs(id string) ([]string, error) { - var lowersArray []string +// getLowerLayerIDs returns a list of lower layer IDs for a layer id; +// typically the contents of lowerLayersFile, falling back to lowerFile. +// If the layer has neither of the files, returns an empty list without reporting an error. +func (d *Driver) getLowerLayerIDs(id string) ([]string, error) { dir := d.dir(id) - lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile)) - if err != nil { - if !errors.Is(err, unix.ENOENT) { + lowerLayers, err := os.ReadFile(path.Join(dir, lowerLayersFile)) + switch { + case err == nil: + return strings.Split(string(lowerLayers), ":"), nil + + case errors.Is(err, fs.ErrNotExist): + lowers, err := os.ReadFile(path.Join(dir, lowerFile)) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } return nil, err } - lowers, err = os.ReadFile(path.Join(dir, lowerFile)) - } - if err == nil { - for s := range strings.SplitSeq(string(lowers), ":") { - lower := d.dir(s) - lp, err := os.Readlink(lower) + var res []string + for relLowerLink := range strings.SplitSeq(string(lowers), ":") { + lowerLink := d.dir(relLowerLink) // This is an invalid use of dir() (the input is supposed to be a layer ID) but pre-existing + lp, err := os.Readlink(lowerLink) if err != nil { - if errors.Is(err, syscall.EINVAL) { - // Not a symlink: layer ID, append /diff. - lowersArray = append(lowersArray, path.Join(lower, "diff")) - continue - } return nil, err } - lowersArray = append(lowersArray, path.Clean(d.dir(path.Join("link", lp)))) + lowerID := filepath.Base(filepath.Dir(lp)) + res = append(res, lowerID) } - } else if !errors.Is(err, fs.ErrNotExist) { + return res, nil + + default: return nil, err } +} + +func (d *Driver) getLowerDirs(id string) ([]string, error) { + lowerLayerIDs, err := d.getLowerLayerIDs(id) + if err != nil { + return nil, err + } + lowersArray := make([]string, 0, len(lowerLayerIDs)) + for _, lowerID := range lowerLayerIDs { + lowerDir := d.dir(lowerID) + lowersArray = append(lowersArray, path.Join(lowerDir, "diff")) + } return lowersArray, nil } @@ -1529,18 +1529,11 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO readWrite = false } - lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile)) + lowerLayerIDs, err := d.getLowerLayerIDs(id) if err != nil { - if !errors.Is(err, unix.ENOENT) { - return "", err - } - lowers, err = os.ReadFile(path.Join(dir, lowerFile)) - if err != nil && !os.IsNotExist(err) { - return "", err - } + return "", err } - splitLowers := strings.Split(string(lowers), ":") - if len(splitLowers) > maxDepth { + if len(lowerLayerIDs) > maxDepth { return "", errors.New("max depth exceeded") } @@ -1630,46 +1623,17 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO // For each lower, resolve its path, and append it and any additional diffN // directories to the lowers list. - for i, l := range splitLowers { - if l == "" { - continue + for i, lowerID := range lowerLayerIDs { + lower := filepath.Join(d.dir(lowerID), "diff") + st, err := os.Stat(lower) + if err != nil { + return "", fmt.Errorf("can't stat (or find?) lower layer %q: %w", lower, err) } - - lower := "" - newpath := path.Join(d.home, l) - if st, err := os.Stat(newpath); err != nil { - for _, p := range d.getAllImageStores() { - lower = path.Join(p, d.name, l) - if st2, err2 := os.Stat(lower); err2 == nil { - if !permsKnown { - perms = st2.Mode() - permsKnown = true - } - break - } - lower = "" - } - if lower == "" { - return "", fmt.Errorf("can't stat lower layer %q: %w", newpath, err) - } - } else { - if !permsKnown { - perms = st.Mode() - permsKnown = true - } - lower = newpath + if !permsKnown { + perms = st.Mode() + permsKnown = true } - linkContent, err := os.Readlink(lower) - if err != nil { - if !errors.Is(err, syscall.EINVAL) { - return "", err - } - // Not a symlink: layer ID from lower-layers, append /diff. - lower = path.Join(lower, "diff") - linkContent = lower - } - lowerID := filepath.Base(filepath.Dir(linkContent)) composefsMount, err := maybeAddComposefsMount(lowerID, i+1, readWrite) if err != nil { return "", err From 5c397106878d25365381f3b224c41970a59fbb8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 23 Apr 2026 15:21:09 +0200 Subject: [PATCH 5/6] Simplify and optimize isParent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't bother with the d.dir() filesystem lookups; they will be equal iff the layer IDs are equal. Signed-off-by: Miloslav Trmač --- storage/drivers/overlay/overlay.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 1b5f0ee7b1..4b5f5b122e 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -2003,23 +2003,15 @@ func (d *Driver) ListLayers() ([]string, error) { // isParent returns if the passed in parent is the direct parent of the passed in layer func (d *Driver) isParent(id, parent string) bool { - lowers, err := d.getLowerDirs(id) + lowerLayerIDs, err := d.getLowerLayerIDs(id) if err != nil { return false } - if parent == "" && len(lowers) > 0 { - return false - } - - parentDir := d.dir(parent) - var ld string - if len(lowers) > 0 { - ld = filepath.Dir(lowers[0]) - } - if ld == "" && parent == "" { - return true + actualParent := "" + if len(lowerLayerIDs) > 0 { + actualParent = lowerLayerIDs[0] } - return ld == parentDir + return parent == actualParent } func (d *Driver) getWhiteoutFormat() archive.WhiteoutFormat { From e4ee402a45784910d073d91779156c76d72d4808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 23 Apr 2026 15:26:09 +0200 Subject: [PATCH 6/6] Move all the "lower"-related functions to single place. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only moves unchanged code, should not change behavior. Also document the difference between getLowerDirs and getLowerDiffPaths. Signed-off-by: Miloslav Trmač --- storage/drivers/overlay/overlay.go | 84 ++++++++++++++++-------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 4b5f5b122e..98f1be3d35 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -1218,32 +1218,6 @@ func (d *Driver) parseStorageOpt(opts *graphdriver.CreateOpts, readOnly bool) (q return res, nil } -// getLowerForParent returns the contents of lowerFile for a child layer of parent. -// -// This should only be used to construct a lowerFile for compatibility; -// new code should rely on lowerLayersFile instead. -func (d *Driver) getLowerForParent(parent string) (string, error) { - parentDir := d.dir(parent) - - // Ensure parent exists - if err := fileutils.Lexists(parentDir); err != nil { - return "", err - } - - parentLink, err := os.ReadFile(path.Join(parentDir, "link")) - if err != nil { - return "", err - } - lowers := []string{path.Join(linkDir, string(parentLink))} - - parentLower, err := os.ReadFile(path.Join(parentDir, lowerFile)) - if err == nil { - parentLowers := strings.SplitSeq(string(parentLower), ":") - lowers = slices.AppendSeq(lowers, parentLowers) - } - return strings.Join(lowers, ":"), nil -} - func (d *Driver) dir(id string) string { p, _, _ := d.dir2(id, false) return p @@ -1285,6 +1259,32 @@ func (d *Driver) dir2(id string, useImageStore bool) (string, string, bool) { return newpath, homedir, false } +// getLowerForParent returns the contents of lowerFile for a child layer of parent. +// +// This should only be used to construct a lowerFile for compatibility; +// new code should rely on lowerLayersFile instead. +func (d *Driver) getLowerForParent(parent string) (string, error) { + parentDir := d.dir(parent) + + // Ensure parent exists + if err := fileutils.Lexists(parentDir); err != nil { + return "", err + } + + parentLink, err := os.ReadFile(path.Join(parentDir, "link")) + if err != nil { + return "", err + } + lowers := []string{path.Join(linkDir, string(parentLink))} + + parentLower, err := os.ReadFile(path.Join(parentDir, lowerFile)) + if err == nil { + parentLowers := strings.SplitSeq(string(parentLower), ":") + lowers = slices.AppendSeq(lowers, parentLowers) + } + return strings.Join(lowers, ":"), nil +} + // getLowerLayerIDs returns a list of lower layer IDs for a layer id; // typically the contents of lowerLayersFile, falling back to lowerFile. // If the layer has neither of the files, returns an empty list without reporting an error. @@ -1320,6 +1320,8 @@ func (d *Driver) getLowerLayerIDs(id string) ([]string, error) { } } +// getLowerDirs returns a list of lower directories for a layer id; +// the directories may be symbolic links (do not call redirectDiffIfAdditionalLayer). func (d *Driver) getLowerDirs(id string) ([]string, error) { lowerLayerIDs, err := d.getLowerLayerIDs(id) if err != nil { @@ -1333,6 +1335,22 @@ func (d *Driver) getLowerDirs(id string) ([]string, error) { return lowersArray, nil } +// getLowerDiffPaths returns a list of lower diff paths for a layer id; +// the paths have redirectDiffIfAdditionalLayer applied. +func (d *Driver) getLowerDiffPaths(id string) ([]string, error) { + layers, err := d.getLowerDirs(id) + if err != nil { + return nil, err + } + for i, l := range layers { + layers[i], err = redirectDiffIfAdditionalLayer(l, false) + if err != nil { + return nil, err + } + } + return layers, nil +} + func (d *Driver) optsAppendMappings(opts string, uidMaps, gidMaps []idtools.IDMap) string { if uidMaps != nil { var uids, gids bytes.Buffer @@ -2395,20 +2413,6 @@ func (d *Driver) getDiffPath(id string) (string, error) { return redirectDiffIfAdditionalLayer(path.Join(dir, "diff"), false) } -func (d *Driver) getLowerDiffPaths(id string) ([]string, error) { - layers, err := d.getLowerDirs(id) - if err != nil { - return nil, err - } - for i, l := range layers { - layers[i], err = redirectDiffIfAdditionalLayer(l, false) - if err != nil { - return nil, err - } - } - return layers, nil -} - // DiffSize calculates the changes between the specified id // and its parent and returns the size in bytes of the changes // relative to its base filesystem directory.