Merge pull request moby#5511 from tonistiigi/sbom-release-fix

fix leaving unreleased references behind after SBOM generation

Author: tonistiigi

client/client_test.go

@@ -9879,6 +9879,8 @@ func testSBOMSupplements(t *testing.T, sb integration.Sandbox) {
 	require.Regexp(t, "^layerID: sha256:", attest.Predicate.Files[0].FileComment)
 	require.Equal(t, "/bar", attest.Predicate.Files[1].FileName)
 	require.Empty(t, attest.Predicate.Files[1].FileComment)
+
+	checkAllReleasable(t, c, sb, false)
 }
 
 func testMultipleCacheExports(t *testing.T, sb integration.Sandbox) {

exporter/containerimage/attestations.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/fs"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	intoto "github.com/in-toto/in-toto-golang/in_toto"
@@ -63,6 +64,7 @@ func supplementSBOM(ctx context.Context, s session.Group, target cache.Immutable
 	if err != nil {
 		return att, err
 	}
+	defer layers.release(context.WithoutCancel(ctx))
 	modifyFile := func(f *spdx.File) error {
 		if f == nil {
 			// Skip over nil entries - this is likely a bug in the SPDX parser,
@@ -77,7 +79,7 @@ func supplementSBOM(ctx context.Context, s session.Group, target cache.Immutable
 			return nil
 		}
 
-		_, desc, err := layers.find(ctx, s, f.FileName)
+		desc, err := layers.find(ctx, s, f.FileName)
 		if err != nil {
 			if !errors.Is(err, fs.ErrNotExist) {
 				return err
@@ -144,13 +146,9 @@ func encodeSPDX(s *spdx.Document) (dt []byte, err error) {
 // fileLayerFinder finds the layer that contains a file, with caching to avoid
 // repeated FileList lookups.
 type fileLayerFinder struct {
-	pending []fileLayerEntry
-	cache   map[string]fileLayerEntry
-}
-
-type fileLayerEntry struct {
-	ref  cache.ImmutableRef
-	desc ocispecs.Descriptor
+	refs    []cache.ImmutableRef
+	pending []ocispecs.Descriptor
+	cache   map[string]ocispecs.Descriptor
 }
 
 func newFileLayerFinder(target cache.ImmutableRef, remote *solver.Remote) (fileLayerFinder, error) {
@@ -159,14 +157,10 @@ func newFileLayerFinder(target cache.ImmutableRef, remote *solver.Remote) (fileL
 	if len(chain) != len(descs) {
 		return fileLayerFinder{}, errors.New("layer chain and descriptor list are not the same length")
 	}
-
-	pending := make([]fileLayerEntry, len(chain))
-	for i, ref := range chain {
-		pending[i] = fileLayerEntry{ref: ref, desc: descs[i]}
-	}
 	return fileLayerFinder{
-		pending: pending,
-		cache:   map[string]fileLayerEntry{},
+		pending: slices.Clone(descs),
+		refs:    chain,
+		cache:   map[string]ocispecs.Descriptor{},
 	}, nil
 }
 
@@ -175,20 +169,21 @@ func newFileLayerFinder(target cache.ImmutableRef, remote *solver.Remote) (fileL
 // the layer that created the file, not the one that deleted it.
 //
 // find is not concurrency-safe.
-func (c *fileLayerFinder) find(ctx context.Context, s session.Group, filename string) (cache.ImmutableRef, *ocispecs.Descriptor, error) {
+func (c *fileLayerFinder) find(ctx context.Context, s session.Group, filename string) (*ocispecs.Descriptor, error) {
 	filename = filepath.Join("/", filename)
 
 	// return immediately if we've already found the layer containing filename
 	if cache, ok := c.cache[filename]; ok {
-		return cache.ref, &cache.desc, nil
+		return &cache, nil
 	}
 
 	for len(c.pending) > 0 {
 		// pop the last entry off the pending list (we traverse the layers backwards)
-		pending := c.pending[len(c.pending)-1]
-		files, err := pending.ref.FileList(ctx, s)
+		idx := len(c.pending) - 1
+		pending := c.pending[idx]
+		files, err := c.refs[idx].FileList(ctx, s)
 		if err != nil {
-			return nil, nil, err
+			return nil, err
 		}
 		c.pending = c.pending[:len(c.pending)-1]
 
@@ -213,8 +208,18 @@ func (c *fileLayerFinder) find(ctx context.Context, s session.Group, filename st
 			}
 		}
 		if found {
-			return pending.ref, &pending.desc, nil
+			return &pending, nil
+		}
+	}
+	return nil, fs.ErrNotExist
+}
+
+func (c *fileLayerFinder) release(ctx context.Context) error {
+	var err error
+	for _, ref := range c.refs {
+		if err1 := ref.Release(ctx); err1 != nil && err == nil {
+			err = err1
 		}
 	}
-	return nil, nil, fs.ErrNotExist
+	return err
 }