Merge pull request #1459 from luhring/sbom-fixes

More SBOM logic improvements

.github/workflows/wolfi-presubmit.yaml

@@ -63,7 +63,7 @@ jobs:
           - tini
           - lzo
           - bubblewrap
-          - gdk-pixbuf
+#          - gdk-pixbuf # Looks like this is broken again, see: https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/515
           - gitsign
           - guac
           - mdbook

pkg/build/build.go

@@ -497,6 +497,10 @@ func (b *Build) IsBuildLess() bool {
 // ConfigFileExternalRef calculates ExternalRef for the melange config
 // file itself.
 func (b *Build) ConfigFileExternalRef() (*purl.PackageURL, error) {
+	// TODO(luhring): This is now the second implementation of finding the commit
+	//  for the config file (the first being the "detectedCommit" logic. We should
+	//  unify these.
+
 	// configFile must exist
 	configpath, err := filepath.Abs(b.ConfigFile)
 	if err != nil {
@@ -508,6 +512,11 @@ func (b *Build) ConfigFileExternalRef() (*purl.PackageURL, error) {
 	if err != nil {
 		return nil, nil
 	}
+
+	// TODO(luhring): This is brittle and assumes a specific git remote configuration.
+	//  We should consider a more general approach, and this may be moot when we
+	//  unify our git state detection mechanisms.
+
 	// If no remote origin, skip (local git repo)
 	remote, err := r.Remote("origin")
 	if err != nil {
@@ -871,8 +880,9 @@ func (b *Build) BuildPackage(ctx context.Context) error {
 		sp := sp
 
 		log.Infof("generating SBOM for subpackage %s", sp.Name)
-		if err := sbom.Generate(ctx, &sbom.Spec{
-			Path:            filepath.Join(b.WorkspaceDir, "melange-out", sp.Name),
+
+		apkFSPath := filepath.Join(b.WorkspaceDir, "melange-out", sp.Name)
+		if err := sbom.GenerateAndWrite(ctx, apkFSPath, &sbom.Spec{
 			PackageName:     sp.Name,
 			PackageVersion:  fmt.Sprintf("%s-r%d", b.Configuration.Package.Version, b.Configuration.Package.Epoch),
 			License:         b.Configuration.Package.LicenseExpression(),
@@ -888,8 +898,9 @@ func (b *Build) BuildPackage(ctx context.Context) error {
 	}
 
 	log.Infof("generating SBOM for %s", b.Configuration.Package.Name)
-	if err := sbom.Generate(ctx, &sbom.Spec{
-		Path:            filepath.Join(b.WorkspaceDir, "melange-out", b.Configuration.Package.Name),
+
+	apkFSPath := filepath.Join(b.WorkspaceDir, "melange-out", b.Configuration.Package.Name)
+	if err := sbom.GenerateAndWrite(ctx, apkFSPath, &sbom.Spec{
 		PackageName:     b.Configuration.Package.Name,
 		PackageVersion:  fmt.Sprintf("%s-r%d", b.Configuration.Package.Version, b.Configuration.Package.Epoch),
 		License:         b.Configuration.Package.LicenseExpression(),

pkg/config/config.go

@@ -721,6 +721,9 @@ func WithVarsFileForParsing(path string) ConfigurationParsingOption {
 }
 
 func detectCommit(ctx context.Context, dirPath string) string {
+	// TODO(luhring): Heads up, a similar implementation was added after this one.
+	//  We should unify these implementations. See Build.ConfigFileExternalRef.
+
 	log := clog.FromContext(ctx)
 	// Best-effort detection of current commit, to be used when not specified in the config file
 

pkg/sbom/bom.go

@@ -23,10 +23,6 @@ import (
 	purl "github.com/package-url/packageurl-go"
 )
 
-type bom struct {
-	Packages []pkg
-}
-
 type element interface {
 	ID() string
 }

pkg/sbom/generator.go

@@ -19,14 +19,15 @@ import (
 	"fmt"
 	"time"
 
+	"chainguard.dev/apko/pkg/sbom/generator/spdx"
 	"github.com/chainguard-dev/clog"
 	purl "github.com/package-url/packageurl-go"
 	"go.opentelemetry.io/otel"
 )
 
-// Spec is the input specification for generating an SBOM.
+// Spec describes the metadata of an APK package for which an SBOM should be
+// created.
 type Spec struct {
-	Path            string
 	PackageName     string
 	PackageVersion  string
 	License         string // Full SPDX license expression
@@ -38,36 +39,43 @@ type Spec struct {
 	SourceDateEpoch time.Time
 }
 
-// Generate runs the main SBOM generation process, by analyzing the APK package
-// from the given spec, creating an SPDX SBOM document, and writing that
-// document to disk.
-func Generate(ctx context.Context, spec *Spec) error {
+// GenerateAndWrite creates an SBOM for the APK package described by the given
+// Spec and writes the SBOM to the APK's filesystem.
+func GenerateAndWrite(ctx context.Context, apkFSPath string, spec *Spec) error {
 	_, span := otel.Tracer("melange").Start(ctx, "GenerateSBOM")
 	defer span.End()
 	log := clog.FromContext(ctx)
 
-	if shouldRun, err := checkEnvironment(spec); err != nil {
+	if shouldRun, err := checkPathExists(apkFSPath); err != nil {
 		return fmt.Errorf("checking SBOM environment: %w", err)
 	} else if !shouldRun {
 		log.Warnf("working directory not found, apk is empty")
 		return nil
 	}
 
-	p, err := generateAPKPackage(spec)
+	document, err := GenerateSPDX(ctx, spec)
 	if err != nil {
-		return fmt.Errorf("generating main APK package: %w", err)
+		return fmt.Errorf("generating SPDX document: %w", err)
 	}
 
-	sbomDoc := &bom{
-		Packages: []pkg{
-			p,
-		},
-	}
-
-	// Finally, write the SBOM data to disk
-	if err := writeSBOM(ctx, spec, sbomDoc); err != nil {
+	if err := writeSBOM(apkFSPath, spec.PackageName, spec.PackageVersion, document); err != nil {
 		return fmt.Errorf("writing sbom to disk: %w", err)
 	}
 
 	return nil
 }
+
+// GenerateSPDX creates an SPDX 2.3 document from the given Spec.
+func GenerateSPDX(ctx context.Context, spec *Spec) (*spdx.Document, error) {
+	p, err := generateSBOMDataForAPKPackage(spec)
+	if err != nil {
+		return nil, fmt.Errorf("generating main APK package: %w", err)
+	}
+
+	doc, err := newSPDXDocument(ctx, spec, p)
+	if err != nil {
+		return nil, fmt.Errorf("creating SPDX document: %w", err)
+	}
+
+	return doc, nil
+}

pkg/sbom/implementation.go

@@ -42,6 +42,8 @@ import (
 	"chainguard.dev/apko/pkg/sbom/generator/spdx"
 )
 
+const extRefCatPackageManager = "PACKAGE-MANAGER"
+
 // invalidIDCharsRe is a regular expression that matches characters not
 // considered valid in SPDX identifiers.
 var invalidIDCharsRe = regexp.MustCompile(`[^a-zA-Z0-9-.]+`)
@@ -75,34 +77,35 @@ func encodeInvalidRune(r rune) string {
 	return "C" + strconv.Itoa(int(r))
 }
 
-// checkEnvironment returns a bool indicating if Spec's Path exists. If the path
-// does not exist, it returns false and a nil error. If an error occurs while
-// checking the directory, it returns false and the error.
-func checkEnvironment(spec *Spec) (bool, error) {
-	dirPath, err := filepath.Abs(spec.Path)
+// checkPathExists returns a bool indicating if the specified path exists. If
+// the path does not exist, it returns false and a nil error. If an error occurs
+// while checking the directory, it returns false and the error.
+func checkPathExists(p string) (bool, error) {
+	dirPath, err := filepath.Abs(p)
 	if err != nil {
-		return false, fmt.Errorf("getting absolute directory path: %w", err)
+		return false, fmt.Errorf("getting absolute path: %w", err)
 	}
 
 	// Check if directory exists
 	if _, err := os.Stat(dirPath); err != nil {
 		if os.IsNotExist(err) {
 			return false, nil
 		}
-		return false, fmt.Errorf("checking if working directory exists: %w", err)
+		return false, fmt.Errorf("stat: %w", err)
 	}
 
 	return true, nil
 }
 
-// generateAPKPackage generates the sbom package representing the apk
-func generateAPKPackage(spec *Spec) (pkg, error) {
+// generateSBOMDataForAPKPackage puts together the normalized SBOM
+// representation for the APK package.
+func generateSBOMDataForAPKPackage(spec *Spec) (*pkg, error) {
 	if spec.PackageName == "" {
-		return pkg{}, errors.New("unable to generate package, name not specified")
+		return nil, errors.New("package name not specified")
 	}
 
 	supplier := "Organization: " + cases.Title(language.English).String(spec.Namespace)
-	newPackage := pkg{
+	newPackage := &pkg{
 		id:               stringToIdentifier(fmt.Sprintf("%s-%s", spec.PackageName, spec.PackageVersion)),
 		FilesAnalyzed:    false,
 		Name:             spec.PackageName,
@@ -155,7 +158,6 @@ func addPackage(doc *spdx.Document, p *pkg) {
 	}
 
 	// Add the purl to the package
-	const extRefCatPackageManager = "PACKAGE_MANAGER"
 	if p.Namespace != "" {
 		var q purl.Qualifiers
 		if p.Arch != "" {
@@ -209,8 +211,8 @@ func sbomHasRelationship(spdxDoc *spdx.Document, bomRel relationship) bool {
 	return false
 }
 
-// buildDocumentSPDX creates an SPDX 2.3 document from our generic representation
-func buildDocumentSPDX(ctx context.Context, spec *Spec, doc *bom) (*spdx.Document, error) {
+// newSPDXDocument creates an SPDX 2.3 document from our generic representation.
+func newSPDXDocument(ctx context.Context, spec *Spec, p *pkg) (*spdx.Document, error) {
 	log := clog.FromContext(ctx)
 
 	h := sha1.New()
@@ -228,9 +230,11 @@ func buildDocumentSPDX(ctx context.Context, spec *Spec, doc *bom) (*spdx.Documen
 			},
 			LicenseListVersion: "3.22", // https://spdx.org/licenses/
 		},
-		DataLicense:          "CC0-1.0",
-		Namespace:            "https://spdx.org/spdxdocs/chainguard/melange/" + hex.EncodeToString(h.Sum(nil)),
-		DocumentDescribes:    []string{},
+		DataLicense: "CC0-1.0",
+		Namespace:   "https://spdx.org/spdxdocs/chainguard/melange/" + hex.EncodeToString(h.Sum(nil)),
+		DocumentDescribes: []string{
+			stringToIdentifier(p.ID()),
+		},
 		Packages:             []spdx.Package{},
 		Relationships:        []spdx.Relationship{},
 		ExternalDocumentRefs: []spdx.ExternalDocumentRef{},
@@ -254,39 +258,28 @@ func buildDocumentSPDX(ctx context.Context, spec *Spec, doc *bom) (*spdx.Documen
 		}
 	}
 
-	for _, p := range doc.Packages {
-		spdxDoc.DocumentDescribes = append(spdxDoc.DocumentDescribes, stringToIdentifier(p.ID()))
-		addPackage(&spdxDoc, &p)
-	}
+	addPackage(&spdxDoc, p)
 
 	return &spdxDoc, nil
 }
 
-// writeSBOM constructs an SPDX document from the given bom, encodes the
-// document to JSON, and writes it to the filesystem in the directory
-// `/var/lib/db/sbom`.
-func writeSBOM(ctx context.Context, spec *Spec, doc *bom) error {
-	spdxDoc, err := buildDocumentSPDX(ctx, spec, doc)
-	if err != nil {
-		return fmt.Errorf("building SPDX document: %w", err)
-	}
-
-	dirPath, err := filepath.Abs(spec.Path)
-	if err != nil {
-		return fmt.Errorf("getting absolute directory path: %w", err)
-	}
+func getPathForPackageSBOM(sbomDirPath, pkgName, pkgVersion string) string {
+	return filepath.Join(
+		sbomDirPath,
+		fmt.Sprintf("%s-%s.spdx.json", pkgName, pkgVersion),
+	)
+}
 
-	const apkSBOMDir = "/var/lib/db/sbom"
-	if err := os.MkdirAll(filepath.Join(dirPath, apkSBOMDir), os.FileMode(0755)); err != nil {
-		return fmt.Errorf("creating SBOM directory in apk filesystem: %w", err)
+// writeSBOM encodes the given SPDX document to JSON and writes it to the
+// filesystem in the directory `/var/lib/db/sbom`.
+func writeSBOM(apkFSPath, pkgName, pkgVersion string, spdxDoc *spdx.Document) error {
+	sbomDirPath := filepath.Join(apkFSPath, "/var/lib/db/sbom")
+	if err := os.MkdirAll(sbomDirPath, os.FileMode(0755)); err != nil {
+		return fmt.Errorf("creating SBOM directory: %w", err)
 	}
 
-	apkSBOMPath := filepath.Join(
-		dirPath,
-		apkSBOMDir,
-		fmt.Sprintf("%s-%s.spdx.json", spec.PackageName, spec.PackageVersion),
-	)
-	f, err := os.Create(apkSBOMPath)
+	sbomPath := getPathForPackageSBOM(sbomDirPath, pkgName, pkgVersion)
+	f, err := os.Create(sbomPath)
 	if err != nil {
 		return fmt.Errorf("opening SBOM file for writing: %w", err)
 	}