Repository: chainguard-dev/apko
Pull request #1611 (2025-04-10 17:52:15)

For multi-layer images, include all layers
    Our image SBOMs currently embeds "the" (singular) layer digest and its
    mediaType as a purl. For multi-layer images, this is no longer
    reasonable. Instead, we'll include all the layers in the SBOM.
    
    This is a bit tricky because the code in written to assume there is
    always exactly one layer, but most of the tests pass exactly 0 layers,
    and we want to support greater than one layer.
    
    I've fixed up the tests and code to make the tests keep passing, but
    we'll probably want to revisit what SBOMs look like when we start
    shipping multiple layers (and write tests to enforce that).


Patch:
diff --git a/pkg/apk/apk/repo.go b/pkg/apk/apk/repo.go
index 9da904f..b46679f 100644
--- a/pkg/apk/apk/repo.go
+++ b/pkg/apk/apk/repo.go
@@ -787,6 +787,33 @@ func (p *PkgResolver) getPackageDependencies(ctx context.Context, pkg *Repositor
 					return nil, nil, err
 				}
 
+				// See if any virtual packages of packages we've already
+				// selected satisfy this contraint.
+				satisfiedByProvide := false
+				for _, provide := range picked.Provides {
+					prostraint := cachedResolvePackageNameVersionPin(provide)
+					pname, pversion, pcompare := prostraint.name, prostraint.version, prostraint.dep
+					if pname != name {
+						continue
+					}
+					// If this virtual package is unversioned, it can't
+					// satisfy a versioned constraint
+					if pversion == "" {
+						continue
+					}
+					prover, err := cachedParseVersion(pversion)
+					if err != nil {
+						return nil, nil, err
+					}
+					if pcompare.satisfies(prover, requiredVersion) {
+						satisfiedByProvide = true
+						break
+					}
+				}
+				if satisfiedByProvide {
+					continue
+				}
+
 				// We do care which version and they match.
 				if compare.satisfies(actualVersion, requiredVersion) {
 					continue
diff --git a/pkg/build/sbom.go b/pkg/build/sbom.go
index 557bf13..331ecf8 100644
--- a/pkg/build/sbom.go
+++ b/pkg/build/sbom.go
@@ -95,14 +95,10 @@ func (bc *Context) GenerateImageSBOM(ctx context.Context, arch types.Architectur
 		return nil, fmt.Errorf("getting %s manifest: %w", arch, err)
 	}
 
-	if bc.baseimg == nil && len(m.Layers) != 1 {
-		return nil, fmt.Errorf("unexpected layers in %s manifest: %d", arch, len(m.Layers))
-	}
-
 	s := newSBOM(ctx, bc.fs, bc.o, bc.ic, bde)
 	log.Debug("Generating image SBOM")
 
-	s.ImageInfo.LayerDigest = m.Layers[0].Digest.String()
+	s.ImageInfo.Layers = m.Layers
 
 	info, err := readReleaseData(bc.fs)
 	if err != nil {
diff --git a/pkg/sbom/generator/spdx/spdx.go b/pkg/sbom/generator/spdx/spdx.go
index 23c441f..f1fe895 100644
--- a/pkg/sbom/generator/spdx/spdx.go
+++ b/pkg/sbom/generator/spdx/spdx.go
@@ -25,6 +25,7 @@ import (
 	"unicode/utf8"
 
 	"github.com/charmbracelet/log"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 	purl "github.com/package-url/packageurl-go"
 	"sigs.k8s.io/release-utils/version"
 
@@ -71,13 +72,21 @@ func stringToIdentifier(in string) (out string) {
 	})
 }
 
+// Returns ":" otherwise :(
+func hashToString(h v1.Hash) string {
+	if h == (v1.Hash{}) {
+		return ""
+	}
+	return h.String()
+}
+
 // Generate writes an SPDX SBOM in path
 func (sx *SPDX) Generate(opts *options.Options, path string) error {
 	// The default document name makes no attempt to avoid
 	// clashes. Ensuring a unique name requires a digest
 	documentName := "sbom"
-	if opts.ImageInfo.LayerDigest != "" {
-		documentName += "-" + opts.ImageInfo.LayerDigest
+	if hash := hashToString(opts.ImageInfo.Layers[0].Digest); hash != "" {
+		documentName += "-" + hash
 	}
 	doc := &Document{
 		ID:      "SPDXRef-DOCUMENT",
@@ -97,21 +106,32 @@ func (sx *SPDX) Generate(opts *options.Options, path string) error {
 		Relationships:  []Relationship{},
 		LicensingInfos: []LicensingInfo{},
 	}
-	var imagePackage *Package
-	layerPackage := sx.layerPackage(opts)
-
-	doc.DocumentDescribes = []string{layerPackage.ID}
 
+	var imagePackage *Package
 	if opts.ImageInfo.ImageDigest != "" {
 		imagePackage = sx.imagePackage(opts)
-		doc.DocumentDescribes = []string{imagePackage.ID}
 		doc.Packages = append(doc.Packages, *imagePackage)
+	}
+
+	for _, layer := range opts.ImageInfo.Layers {
+		layerPackage := sx.layerPackage(opts, layer)
+
 		// Add to the relationships list
-		doc.Relationships = append(doc.Relationships, Relationship{
-			Element: imagePackage.ID,
-			Type:    "CONTAINS",
-			Related: layerPackage.ID,
-		})
+		if imagePackage != nil {
+			doc.Relationships = append(doc.Relationships, Relationship{
+				Element: imagePackage.ID,
+				Type:    "CONTAINS",
+				Related: layerPackage.ID,
+			})
+		} else {
+			doc.DocumentDescribes = []string{layerPackage.ID}
+		}
+
+		doc.Packages = append(doc.Packages, *layerPackage)
+	}
+
+	if imagePackage != nil {
+		doc.DocumentDescribes = []string{imagePackage.ID}
 	}
 
 	if opts.ImageInfo.VCSUrl != "" {
@@ -120,14 +140,26 @@ func (sx *SPDX) Generate(opts *options.Options, path string) error {
 		}
 	}
 
-	doc.Packages = append(doc.Packages, *layerPackage)
-
 	for _, pkg := range opts.Packages {
 		// add the package
 		p := sx.apkPackage(opts, pkg)
-		// Add the layer to the ID to avoid clashes
+
+		nonce := "thismakestestspass"
+
+		// Add the image ID to the package ID to avoid clashes.
+		// NB: imagePackage.ID here is actually not important.
+		// The important thing is that p.ID is different from the package ID we
+		// would find in the corresponding APK SBOM. The ProcessInternalApkSBOM
+		// logic does a "JOIN" against the APK SBOM and replaces references to
+		// this ID with references to the APK's SBOM's Package ID, which means
+		// that logic falls over if they match. Removing imagePackage.ID makes
+		// them match, so we have to have _something_. You could replace
+		// imagePackage.ID with a UUID or "" and this would still be correct.
+		if imagePackage != nil {
+			nonce = imagePackage.ID
+		}
 		p.ID = stringToIdentifier(fmt.Sprintf(
-			"SPDXRef-Package-%s-%s-%s", layerPackage.ID, pkg.Name, pkg.Version,
+			"SPDXRef-Package-%s-%s-%s", nonce, pkg.Name, pkg.Version,
 		))
 
 		doc.Packages = append(doc.Packages, p)
@@ -495,8 +527,8 @@ func (sx *SPDX) apkPackage(opts *options.Options, pkg *apk.InstalledPackage) Pac
 }
 
 // LayerPackage returns a package describing the layer
-func (sx *SPDX) layerPackage(opts *options.Options) *Package {
-	layerPackageName := opts.ImageInfo.LayerDigest
+func (sx *SPDX) layerPackage(opts *options.Options, layer v1.Descriptor) *Package {
+	layerPackageName := hashToString(layer.Digest)
 	mainPkgID := stringToIdentifier(layerPackageName)
 
 	return &Package{
@@ -514,9 +546,9 @@ func (sx *SPDX) layerPackage(opts *options.Options) *Package {
 				Category: ExtRefPackageManager,
 				Type:     ExtRefTypePurl,
 				Locator: purl.NewPackageURL(
-					purl.TypeOCI, "", opts.ImagePurlName(), opts.ImageInfo.LayerDigest,
+					purl.TypeOCI, "", opts.ImagePurlName(), hashToString(layer.Digest),
 					nil, "",
-				).String() + "?" + opts.LayerPurlQualifiers().String(),
+				).String() + "?" + opts.LayerPurlQualifiers(layer).String(),
 			},
 		},
 	}
diff --git a/pkg/sbom/options/options.go b/pkg/sbom/options/options.go
index a74a26f..1ed0033 100644
--- a/pkg/sbom/options/options.go
+++ b/pkg/sbom/options/options.go
@@ -68,8 +68,8 @@ type ImageInfo struct {
 	Tag             string
 	Name            string
 	Repository      string
-	LayerDigest     string
 	ImageDigest     string
+	Layers          []v1.Descriptor
 	VCSUrl          string
 	IndexMediaType  ggcrtypes.MediaType
 	ImageMediaType  ggcrtypes.MediaType
@@ -149,16 +149,9 @@ func (pq PurlQualifiers) String() string {
 
 // LayerPurlQualifiers reurns the qualifiers for the purl, they are based
 // on the image with the corresponding mediatype
-func (o *Options) LayerPurlQualifiers() (qualifiers PurlQualifiers) {
+func (o *Options) LayerPurlQualifiers(layer v1.Descriptor) (qualifiers PurlQualifiers) {
 	qualifiers = o.ImagePurlQualifiers()
-	switch o.ImageInfo.ImageMediaType {
-	case ggcrtypes.OCIManifestSchema1:
-		qualifiers["mediaType"] = string(ggcrtypes.OCILayer)
-	case ggcrtypes.DockerManifestSchema2:
-		qualifiers["mediaType"] = string(ggcrtypes.DockerLayer)
-	default:
-		qualifiers["mediaType"] = ""
-	}
+	qualifiers["mediaType"] = string(layer.MediaType)
 	return qualifiers
 }
 


Test patch:
diff --git a/pkg/apk/apk/repo_test.go b/pkg/apk/apk/repo_test.go
index 146ae3b..f12fe52 100644
--- a/pkg/apk/apk/repo_test.go
+++ b/pkg/apk/apk/repo_test.go
@@ -870,6 +870,39 @@ func TestSameProvidedVersion(t *testing.T) {
 	}
 }
 
+// If a package has a provides with a different version than the real
+// package (here libcurl-openssl4=8.12.1-r1 provides libcurl-abi=8.12.1),
+// both should be allowed in the same DAG
+func TestDepWithProvidesOfDiffVersion(t *testing.T) {
+	providers := map[string][]string{
+		"libcurl-openssl4=8.12.1-r1": {
+			"libcurl-abi=8.12.1", "so:libcurl.so.4=4",
+		},
+	}
+	dependers := map[string][]string{
+		"foo=1":                      {}, // an extra dep level needed to trigger bug
+		"libcurl-openssl4=8.12.1-r1": {"foo"},
+		"curl=8.12.1-r1":             {"libcurl-abi=8.12.1", "so:libcurl.so.4"},
+		"curl-dev=8.12.1-r1":         {"so:libcurl.so.4"},
+	}
+
+	resolver := makeResolver(providers, dependers)
+	pkgs, _, err := resolver.GetPackagesWithDependencies(context.Background(), []string{"curl-dev", "curl"}, nil)
+	require.NoError(t, err)
+
+	wantPkgs := []string{
+		"foo-1.apk",
+		"libcurl-openssl4-8.12.1-r1.apk",
+		"curl-dev-8.12.1-r1.apk",
+		"curl-8.12.1-r1.apk",
+	}
+
+	require.Len(t, pkgs, len(wantPkgs))
+	for i, pkg := range pkgs {
+		require.Equal(t, pkg.Filename(), wantPkgs[i])
+	}
+}
+
 func TestHigherProvidedVersion(t *testing.T) {
 	providers := map[string][]string{
 		"ld-linux=2.38-r10": {"so:ld-linux-aarch64.so.1=1.1"},
diff --git a/pkg/sbom/generator/spdx/spdx_test.go b/pkg/sbom/generator/spdx/spdx_test.go
index 99c4fe0..2de2f82 100644
--- a/pkg/sbom/generator/spdx/spdx_test.go
+++ b/pkg/sbom/generator/spdx/spdx_test.go
@@ -24,6 +24,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/stretchr/testify/require"
 	"sigs.k8s.io/release-utils/command"
 
@@ -34,6 +35,9 @@ import (
 
 // TODO: clean this up and make consistent with the other test cases
 var testOpts = &options.Options{
+	ImageInfo: options.ImageInfo{
+		Layers: []v1.Descriptor{{}},
+	},
 	OS: options.OSInfo{
 		Name:    "unknown",
 		ID:      "unknown",
@@ -78,6 +82,9 @@ func TestSPDX_Generate(t *testing.T) {
 		{
 			name: "custom-license",
 			opts: &options.Options{
+				ImageInfo: options.ImageInfo{
+					Layers: []v1.Descriptor{{}},
+				},
 				OS: options.OSInfo{
 					Name:    "unknown",
 					ID:      "unknown",
@@ -97,6 +104,9 @@ func TestSPDX_Generate(t *testing.T) {
 		{
 			name: "no-supplier",
 			opts: &options.Options{
+				ImageInfo: options.ImageInfo{
+					Layers: []v1.Descriptor{{}},
+				},
 				OS: options.OSInfo{
 					Name:    "Apko Images, Plc",
 					ID:      "apko-images",
@@ -116,6 +126,9 @@ func TestSPDX_Generate(t *testing.T) {
 		{
 			name: "package-deduplicating",
 			opts: &options.Options{
+				ImageInfo: options.ImageInfo{
+					Layers: []v1.Descriptor{{}},
+				},
 				OS: options.OSInfo{
 					Name:    "unknown",
 					ID:      "unknown",