Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix leaving unreleased references behind after SBOM generation #5511

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, false)
}

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
}