Merge pull request #427 from anchore/epoch-fix

Allow for RPM package epoch to be optionally provided in the version field

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/adrg/xdg v0.2.1
 	github.com/alecthomas/jsonschema v0.0.0-20210301060011-54c507b6f074
 	github.com/anchore/client-go v0.0.0-20210222170800-9c70f9b80bcf
-	github.com/anchore/go-rpmdb v0.0.0-20210415132930-2460011e83c6
+	github.com/anchore/go-rpmdb v0.0.0-20210602151223-1f0f707a2894
 	github.com/anchore/go-testutils v0.0.0-20200925183923-d5f45b0d3c04
 	github.com/anchore/go-version v1.2.2-0.20200701162849-18adb9c92b9b
 	github.com/anchore/stereoscope v0.0.0-20210524175238-3b7662f3a66f

go.sum

@@ -109,8 +109,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF
 github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho=
 github.com/anchore/client-go v0.0.0-20210222170800-9c70f9b80bcf h1:DYssiUV1pBmKqzKsm4mqXx8artqC0Q8HgZsVI3lMsAg=
 github.com/anchore/client-go v0.0.0-20210222170800-9c70f9b80bcf/go.mod h1:FaODhIA06mxO1E6R32JE0TL1JWZZkmjRIAd4ULvHUKk=
-github.com/anchore/go-rpmdb v0.0.0-20210415132930-2460011e83c6 h1:wEN3HXc3VuC4wo7Cz27YCpeQ4gaB5ASKwMwM5GdFsew=
+github.com/anchore/go-rpmdb v0.0.0-20210602151223-1f0f707a2894 h1:VvCq7fFNU8dwWkCTGUykm4p64nVaDCYkKrj87x350Sk=
-github.com/anchore/go-rpmdb v0.0.0-20210415132930-2460011e83c6/go.mod h1:8jNYOxCJC5kyD/Ct4MbzsDN2hOhRoCAzQcb/7KdYYGw=
+github.com/anchore/go-rpmdb v0.0.0-20210602151223-1f0f707a2894/go.mod h1:8jNYOxCJC5kyD/Ct4MbzsDN2hOhRoCAzQcb/7KdYYGw=
 github.com/anchore/go-testutils v0.0.0-20200925183923-d5f45b0d3c04 h1:VzprUTpc0vW0nnNKJfJieyH/TZ9UYAnTZs5/gHTdAe8=
 github.com/anchore/go-testutils v0.0.0-20200925183923-d5f45b0d3c04/go.mod h1:6dK64g27Qi1qGQZ67gFmBFvEHScy0/C8qhQhNe5B5pQ=
 github.com/anchore/go-version v1.2.2-0.20200701162849-18adb9c92b9b h1:e1bmaoJfZVsCYMrIZBpFxwV26CbsuoEh5muXD5I1Ods=

syft/pkg/cataloger/package_url_test.go

@@ -30,30 +30,6 @@ func TestPackageURL(t *testing.T) {
 			},
 			expected: "pkg:pypi/name@v0.1.0",
 		},
-		{
-			pkg: pkg.Package{
-				Name:    "name",
-				Version: "v0.1.0",
-				Type:    pkg.PythonPkg,
-			},
-			expected: "pkg:pypi/name@v0.1.0",
-		},
-		{
-			pkg: pkg.Package{
-				Name:    "name",
-				Version: "v0.1.0",
-				Type:    pkg.PythonPkg,
-			},
-			expected: "pkg:pypi/name@v0.1.0",
-		},
-		{
-			pkg: pkg.Package{
-				Name:    "name",
-				Version: "v0.1.0",
-				Type:    pkg.PythonPkg,
-			},
-			expected: "pkg:pypi/name@v0.1.0",
-		},
 		{
 			pkg: pkg.Package{
 				Name:    "name",
@@ -96,13 +72,31 @@ func TestPackageURL(t *testing.T) {
 				Type:    pkg.RpmPkg,
 				Metadata: pkg.RpmdbMetadata{
 					Name:    "name",
-					Version: "v0.1.0",
+					Version: "0.1.0",
-					Epoch:   2,
+					Epoch:   intRef(2),
 					Arch:    "amd64",
 					Release: "3",
 				},
 			},
-			expected: "pkg:rpm/centos/name@2:v0.1.0-3?arch=amd64",
+			expected: "pkg:rpm/centos/name@0.1.0-3?arch=amd64&epoch=2",
+		},
+		{
+			distro: &distro.Distro{
+				Type: distro.CentOS,
+			},
+			pkg: pkg.Package{
+				Name:    "bad-name",
+				Version: "bad-v0.1.0",
+				Type:    pkg.RpmPkg,
+				Metadata: pkg.RpmdbMetadata{
+					Name:    "name",
+					Version: "0.1.0",
+					Epoch:   nil,
+					Arch:    "amd64",
+					Release: "3",
+				},
+			},
+			expected: "pkg:rpm/centos/name@0.1.0-3?arch=amd64",
 		},
 		{
 			distro: &distro.Distro{
@@ -136,3 +130,7 @@ func TestPackageURL(t *testing.T) {
 		})
 	}
 }
+
+func intRef(i int) *int {
+	return &i
+}

syft/pkg/cataloger/rpmdb/parse_rpmdb.go

@@ -47,25 +47,27 @@ func parseRpmDB(resolver source.FilePathResolver, dbLocation source.Location, re
 	allPkgs := make([]pkg.Package, 0)
 
 	for _, entry := range pkgList {
+		metadata := pkg.RpmdbMetadata{
+			Name:      entry.Name,
+			Version:   entry.Version,
+			Epoch:     entry.Epoch,
+			Arch:      entry.Arch,
+			Release:   entry.Release,
+			SourceRpm: entry.SourceRpm,
+			Vendor:    entry.Vendor,
+			License:   entry.License,
+			Size:      entry.Size,
+			Files:     extractRpmdbFileRecords(resolver, entry),
+		}
+
 		p := pkg.Package{
 			Name:         entry.Name,
-			Version:      fmt.Sprintf("%s-%s", entry.Version, entry.Release), // this is what engine does, instead of fmt.Sprintf("%d:%s-%s.%s", entry.Epoch, entry.Version, entry.Release, entry.Arch)
+			Version:      toELVersion(metadata),
 			Locations:    []source.Location{dbLocation},
 			FoundBy:      catalogerName,
 			Type:         pkg.RpmPkg,
 			MetadataType: pkg.RpmdbMetadataType,
-			Metadata: pkg.RpmdbMetadata{
+			Metadata:     metadata,
-				Name:      entry.Name,
-				Version:   entry.Version,
-				Epoch:     entry.Epoch,
-				Arch:      entry.Arch,
-				Release:   entry.Release,
-				SourceRpm: entry.SourceRpm,
-				Vendor:    entry.Vendor,
-				License:   entry.License,
-				Size:      entry.Size,
-				Files:     extractRpmdbFileRecords(resolver, entry),
-			},
 		}
 
 		allPkgs = append(allPkgs, p)
@@ -74,6 +76,19 @@ func parseRpmDB(resolver source.FilePathResolver, dbLocation source.Location, re
 	return allPkgs, nil
 }
 
+// The RPM naming scheme is [name]-[version]-[release]-[arch], where version is implicitly expands to [epoch]:[version].
+// RPM version comparison depends on comparing at least the version and release fields together as a subset of the
+// naming scheme. This toELVersion function takes a RPM DB package information and converts it into a minimally comparable
+// version string, containing epoch (optional), version, and release information. Epoch is an optional field and can be
+// assumed to be 0 when not provided for comparison purposes, however, if the underlying RPM DB entry does not have
+// an epoch specified it would be slightly disingenuous to display a value of 0.
+func toELVersion(metadata pkg.RpmdbMetadata) string {
+	if metadata.Epoch != nil {
+		return fmt.Sprintf("%d:%s-%s", *metadata.Epoch, metadata.Version, metadata.Release)
+	}
+	return fmt.Sprintf("%s-%s", metadata.Version, metadata.Release)
+}
+
 func extractRpmdbFileRecords(resolver source.FilePathResolver, entry *rpmdb.PackageInfo) []pkg.RpmdbFileRecord {
 	var records = make([]pkg.RpmdbFileRecord, 0)
 

syft/pkg/cataloger/rpmdb/parse_rpmdb_test.go

@@ -5,6 +5,8 @@ import (
 	"os"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/anchore/syft/syft/file"
 
 	"github.com/anchore/syft/syft/source"
@@ -70,7 +72,7 @@ func TestParseRpmDB(t *testing.T) {
 					MetadataType: pkg.RpmdbMetadataType,
 					Metadata: pkg.RpmdbMetadata{
 						Name:      "dive",
-						Epoch:     0,
+						Epoch:     nil,
 						Arch:      "x86_64",
 						Release:   "1",
 						Version:   "0.9.2",
@@ -97,7 +99,7 @@ func TestParseRpmDB(t *testing.T) {
 					MetadataType: pkg.RpmdbMetadataType,
 					Metadata: pkg.RpmdbMetadata{
 						Name:      "dive",
-						Epoch:     0,
+						Epoch:     nil,
 						Arch:      "x86_64",
 						Release:   "1",
 						Version:   "0.9.2",
@@ -157,3 +159,51 @@ func TestParseRpmDB(t *testing.T) {
 	}
 
 }
+
+func TestToElVersion(t *testing.T) {
+	tests := []struct {
+		name     string
+		entry    pkg.RpmdbMetadata
+		expected string
+	}{
+		{
+			name: "no epoch",
+			entry: pkg.RpmdbMetadata{
+				Version: "1.2.3-4",
+				Release: "el7",
+				Arch:    "x86-64",
+			},
+			expected: "1.2.3-4-el7",
+		},
+		{
+			name: "with 0 epoch",
+			entry: pkg.RpmdbMetadata{
+				Version: "1.2.3-4",
+				Release: "el7",
+				Arch:    "x86-64",
+				Epoch:   intRef(0),
+			},
+			expected: "0:1.2.3-4-el7",
+		},
+		{
+			name: "with non-zero epoch",
+			entry: pkg.RpmdbMetadata{
+				Version: "1.2.3-4",
+				Release: "el7",
+				Arch:    "x86-64",
+				Epoch:   intRef(12),
+			},
+			expected: "12:1.2.3-4-el7",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			assert.Equal(t, test.expected, toELVersion(test.entry))
+		})
+	}
+}
+
+func intRef(i int) *int {
+	return &i
+}

syft/pkg/rpmdb_metadata.go

@@ -3,6 +3,7 @@ package pkg
 import (
 	"fmt"
 	"sort"
+	"strconv"
 
 	"github.com/anchore/syft/syft/file"
 
@@ -20,7 +21,7 @@ var _ fileOwner = (*RpmdbMetadata)(nil)
 type RpmdbMetadata struct {
 	Name      string            `json:"name"`
 	Version   string            `json:"version"`
-	Epoch     int               `json:"epoch"`
+	Epoch     *int              `json:"epoch"`
 	Arch      string            `json:"architecture"`
 	Release   string            `json:"release"`
 	SourceRpm string            `json:"sourceRpm"`
@@ -50,17 +51,30 @@ func (m RpmdbMetadata) PackageURL(d *distro.Distro) string {
 		return ""
 	}
 
+	qualifiers := packageurl.Qualifiers{
+		{
+			Key:   "arch",
+			Value: m.Arch,
+		},
+	}
+
+	if m.Epoch != nil {
+		qualifiers = append(qualifiers,
+			packageurl.Qualifier{
+				Key:   "epoch",
+				Value: strconv.Itoa(*m.Epoch),
+			},
+		)
+	}
+
 	pURL := packageurl.NewPackageURL(
 		packageurl.TypeRPM,
 		d.Type.String(),
 		m.Name,
-		fmt.Sprintf("%d:%s-%s", m.Epoch, m.Version, m.Release),
+		// for purl the epoch is a qualifier, not part of the version
-		packageurl.Qualifiers{
+		// see https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst under the RPM section
-			{
+		fmt.Sprintf("%s-%s", m.Version, m.Release),
-				Key:   "arch",
+		qualifiers,
-				Value: m.Arch,
-			},
-		},
 		"")
 	return pURL.ToString()
 }

syft/pkg/rpmdb_metadata_test.go

@@ -25,9 +25,9 @@ func TestRpmMetadata_pURL(t *testing.T) {
 				Version: "v",
 				Arch:    "a",
 				Release: "r",
-				Epoch:   1,
+				Epoch:   intRef(1),
 			},
-			expected: "pkg:rpm/centos/p@1:v-r?arch=a",
+			expected: "pkg:rpm/centos/p@v-r?arch=a&epoch=1",
 		},
 		{
 			distro: distro.Distro{
@@ -38,9 +38,9 @@ func TestRpmMetadata_pURL(t *testing.T) {
 				Version: "v",
 				Arch:    "a",
 				Release: "r",
-				Epoch:   1,
+				Epoch:   nil,
 			},
-			expected: "pkg:rpm/redhat/p@1:v-r?arch=a",
+			expected: "pkg:rpm/redhat/p@v-r?arch=a",
 		},
 	}
 
@@ -97,3 +97,7 @@ func TestRpmMetadata_fileOwner(t *testing.T) {
 		})
 	}
 }
+
+func intRef(i int) *int {
+	return &i
+}