Skip to content

Commit

Permalink
fix leaving unreleased references behind after SBOM generation
Browse files Browse the repository at this point in the history
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
  • Loading branch information
tonistiigi committed Nov 12, 2024
1 parent 0655923 commit c9ac4fc
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 22 deletions.
2 changes: 2 additions & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, true)
}

func testMultipleCacheExports(t *testing.T, sb integration.Sandbox) {
Expand Down
49 changes: 27 additions & 22 deletions exporter/containerimage/attestations.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/fs"
"path/filepath"
"slices"
"strings"

intoto "github.com/in-toto/in-toto-golang/in_toto"
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}

Expand All @@ -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]

Expand All @@ -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
}

0 comments on commit c9ac4fc

Please sign in to comment.