port apk cataloger to new generic cataloger pattern (#1283)

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

syft/pkg/apk_metadata.go

@@ -5,17 +5,12 @@ import (
 
 	"github.com/scylladb/go-set/strset"
 
-	"github.com/anchore/packageurl-go"
 	"github.com/anchore/syft/syft/file"
-	"github.com/anchore/syft/syft/linux"
 )
 
 const ApkDBGlob = "**/lib/apk/db/installed"
 
-var (
+var _ FileOwner = (*ApkMetadata)(nil)
-	_ FileOwner     = (*ApkMetadata)(nil)
-	_ urlIdentifier = (*ApkMetadata)(nil)
-)
 
 // ApkMetadata represents all captured data for a Alpine DB package entry.
 // See the following sources for more information:
@@ -48,31 +43,6 @@ type ApkFileRecord struct {
 	Digest      *file.Digest `json:"digest,omitempty"`
 }
 
-// PackageURL returns the PURL for the specific Alpine package (see https://github.com/package-url/purl-spec)
-func (m ApkMetadata) PackageURL(distro *linux.Release) string {
-	qualifiers := map[string]string{
-		PURLQualifierArch: m.Architecture,
-	}
-
-	if m.OriginPackage != "" {
-		qualifiers[PURLQualifierUpstream] = m.OriginPackage
-	}
-
-	return packageurl.NewPackageURL(
-		// note: this is currently a candidate and not technically within spec
-		// see https://github.com/package-url/purl-spec#other-candidate-types-to-define
-		"alpine",
-		"",
-		m.Package,
-		m.Version,
-		PURLQualifiers(
-			qualifiers,
-			distro,
-		),
-		"",
-	).ToString()
-}
-
 func (m ApkMetadata) OwnedFiles() (result []string) {
 	s := strset.New()
 	for _, f := range m.Files {

syft/pkg/cataloger/apkdb/cataloger.go

@@ -5,14 +5,13 @@ package apkdb
 
 import (
 	"github.com/anchore/syft/syft/pkg"
-	"github.com/anchore/syft/syft/pkg/cataloger/common"
+	"github.com/anchore/syft/syft/pkg/cataloger/generic"
 )
 
-// NewApkdbCataloger returns a new Alpine DB cataloger object.
+const catalogerName = "apkdb-cataloger"
-func NewApkdbCataloger() *common.GenericCataloger {
-	globParsers := map[string]common.ParserFn{
-		pkg.ApkDBGlob: parseApkDB,
-	}
 
-	return common.NewGenericCataloger(nil, globParsers, "apkdb-cataloger")
+// NewApkdbCataloger returns a new Alpine DB cataloger object.
+func NewApkdbCataloger() *generic.Cataloger {
+	return generic.NewCataloger(catalogerName).
+		WithParserByGlobs(parseApkDB, pkg.ApkDBGlob)
 }

syft/pkg/cataloger/apkdb/package.go

@@ -0,0 +1,57 @@
+package apkdb
+
+import (
+	"strings"
+
+	"github.com/anchore/packageurl-go"
+	"github.com/anchore/syft/syft/linux"
+	"github.com/anchore/syft/syft/pkg"
+	"github.com/anchore/syft/syft/source"
+)
+
+func newPackage(d pkg.ApkMetadata, release *linux.Release, locations ...source.Location) pkg.Package {
+	p := pkg.Package{
+		Name:         d.Package,
+		Version:      d.Version,
+		Locations:    source.NewLocationSet(locations...),
+		Licenses:     strings.Split(d.License, " "),
+		PURL:         packageURL(d, release),
+		Type:         pkg.ApkPkg,
+		MetadataType: pkg.ApkMetadataType,
+		Metadata:     d,
+	}
+
+	p.SetID()
+
+	return p
+}
+
+// packageURL returns the PURL for the specific Alpine package (see https://github.com/package-url/purl-spec)
+func packageURL(m pkg.ApkMetadata, distro *linux.Release) string {
+	if distro == nil || distro.ID != "alpine" {
+		// note: there is no namespace variation (like with debian ID_LIKE for ubuntu ID, for example)
+		return ""
+	}
+
+	qualifiers := map[string]string{
+		pkg.PURLQualifierArch: m.Architecture,
+	}
+
+	if m.OriginPackage != "" {
+		qualifiers[pkg.PURLQualifierUpstream] = m.OriginPackage
+	}
+
+	return packageurl.NewPackageURL(
+		// note: this is currently a candidate and not technically within spec
+		// see https://github.com/package-url/purl-spec#other-candidate-types-to-define
+		"alpine",
+		"",
+		m.Package,
+		m.Version,
+		pkg.PURLQualifiers(
+			qualifiers,
+			distro,
+		),
+		"",
+	).ToString()
+}

syft/pkg/cataloger/apkdb/package_test.go

@@ -1,4 +1,4 @@
-package pkg
+package apkdb
 
 import (
 	"strings"
@@ -9,18 +9,32 @@ import (
 
 	"github.com/anchore/packageurl-go"
 	"github.com/anchore/syft/syft/linux"
+	"github.com/anchore/syft/syft/pkg"
 )
 
-func TestApkMetadata_pURL(t *testing.T) {
+func Test_PackageURL(t *testing.T) {
 	tests := []struct {
 		name     string
-		metadata ApkMetadata
+		metadata pkg.ApkMetadata
 		distro   linux.Release
 		expected string
 	}{
+		{
+			name: "bad distro",
+			metadata: pkg.ApkMetadata{
+				Package:      "p",
+				Version:      "v",
+				Architecture: "a",
+			},
+			distro: linux.Release{
+				ID:        "something else",
+				VersionID: "3.4.6",
+			},
+			expected: "",
+		},
 		{
 			name: "gocase",
-			metadata: ApkMetadata{
+			metadata: pkg.ApkMetadata{
 				Package:      "p",
 				Version:      "v",
 				Architecture: "a",
@@ -33,7 +47,7 @@ func TestApkMetadata_pURL(t *testing.T) {
 		},
 		{
 			name: "missing architecture",
-			metadata: ApkMetadata{
+			metadata: pkg.ApkMetadata{
 				Package: "p",
 				Version: "v",
 			},
@@ -45,7 +59,7 @@ func TestApkMetadata_pURL(t *testing.T) {
 		},
 		// verify #351
 		{
-			metadata: ApkMetadata{
+			metadata: pkg.ApkMetadata{
 				Package:      "g++",
 				Version:      "v84",
 				Architecture: "am86",
@@ -57,7 +71,7 @@ func TestApkMetadata_pURL(t *testing.T) {
 			expected: "pkg:alpine/g++@v84?arch=am86&distro=alpine-3.4.6",
 		},
 		{
-			metadata: ApkMetadata{
+			metadata: pkg.ApkMetadata{
 				Package:      "g plus plus",
 				Version:      "v84",
 				Architecture: "am86",
@@ -70,7 +84,7 @@ func TestApkMetadata_pURL(t *testing.T) {
 		},
 		{
 			name: "add source information as qualifier",
-			metadata: ApkMetadata{
+			metadata: pkg.ApkMetadata{
 				Package:       "p",
 				Version:       "v",
 				Architecture:  "a",
@@ -86,12 +100,17 @@ func TestApkMetadata_pURL(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			actual := test.metadata.PackageURL(&test.distro)
+			actual := packageURL(test.metadata, &test.distro)
 			if actual != test.expected {
 				dmp := diffmatchpatch.New()
 				diffs := dmp.DiffMain(test.expected, actual, true)
 				t.Errorf("diff: %s", dmp.DiffPrettyText(diffs))
 			}
+
+			if test.expected == "" {
+				return
+			}
+
 			// verify packageurl can parse
 			purl, err := packageurl.FromString(actual)
 			if err != nil {
@@ -118,12 +137,12 @@ func TestApkMetadata_pURL(t *testing.T) {
 
 func TestApkMetadata_FileOwner(t *testing.T) {
 	tests := []struct {
-		metadata ApkMetadata
+		metadata pkg.ApkMetadata
 		expected []string
 	}{
 		{
-			metadata: ApkMetadata{
+			metadata: pkg.ApkMetadata{
-				Files: []ApkFileRecord{
+				Files: []pkg.ApkFileRecord{
 					{Path: "/somewhere"},
 					{Path: "/else"},
 				},
@@ -134,8 +153,8 @@ func TestApkMetadata_FileOwner(t *testing.T) {
 			},
 		},
 		{
-			metadata: ApkMetadata{
+			metadata: pkg.ApkMetadata{
-				Files: []ApkFileRecord{
+				Files: []pkg.ApkFileRecord{
 					{Path: "/somewhere"},
 					{Path: ""},
 				},

syft/pkg/cataloger/apkdb/parse_apk_db.go

@@ -13,32 +13,23 @@ import (
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/artifact"
 	"github.com/anchore/syft/syft/file"
+	"github.com/anchore/syft/syft/linux"
 	"github.com/anchore/syft/syft/pkg"
-	"github.com/anchore/syft/syft/pkg/cataloger/common"
+	"github.com/anchore/syft/syft/pkg/cataloger/generic"
+	"github.com/anchore/syft/syft/source"
 )
 
 // integrity check
-var _ common.ParserFn = parseApkDB
+var _ generic.Parser = parseApkDB
-
-func newApkDBPackage(d *pkg.ApkMetadata) *pkg.Package {
-	return &pkg.Package{
-		Name:         d.Package,
-		Version:      d.Version,
-		Licenses:     strings.Split(d.License, " "),
-		Type:         pkg.ApkPkg,
-		MetadataType: pkg.ApkMetadataType,
-		Metadata:     *d,
-	}
-}
 
 // parseApkDb parses individual packages from a given Alpine DB file. For more information on specific fields
 // see https://wiki.alpinelinux.org/wiki/Apk_spec .
-func parseApkDB(_ string, reader io.Reader) ([]*pkg.Package, []artifact.Relationship, error) {
+func parseApkDB(_ source.FileResolver, env *generic.Environment, reader source.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) {
 	// larger capacity for the scanner.
 	const maxScannerCapacity = 1024 * 1024
 	// a new larger buffer for the scanner
 	bufScan := make([]byte, maxScannerCapacity)
-	packages := make([]*pkg.Package, 0)
+	pkgs := make([]pkg.Package, 0)
 
 	scanner := bufio.NewScanner(reader)
 	scanner.Buffer(bufScan, maxScannerCapacity)
@@ -55,6 +46,11 @@ func parseApkDB(_ string, reader io.Reader) ([]*pkg.Package, []artifact.Relation
 		return 0, data, bufio.ErrFinalToken
 	}
 
+	var r *linux.Release
+	if env != nil {
+		r = env.LinuxRelease
+	}
+
 	scanner.Split(onDoubleLF)
 	for scanner.Scan() {
 		metadata, err := parseApkDBEntry(strings.NewReader(scanner.Text()))
@@ -62,7 +58,7 @@ func parseApkDB(_ string, reader io.Reader) ([]*pkg.Package, []artifact.Relation
 			return nil, nil, err
 		}
 		if metadata != nil {
-			packages = append(packages, newApkDBPackage(metadata))
+			pkgs = append(pkgs, newPackage(*metadata, r, reader.Location))
 		}
 	}
 
@@ -70,7 +66,7 @@ func parseApkDB(_ string, reader io.Reader) ([]*pkg.Package, []artifact.Relation
 		return nil, nil, fmt.Errorf("failed to parse APK DB file: %w", err)
 	}
 
-	return packages, nil, nil
+	return pkgs, nil, nil
 }
 
 // parseApkDBEntry reads and parses a single pkg.ApkMetadata element from the stream, returning nil if their are no more entries.

syft/pkg/cataloger/apkdb/parse_apk_db_test.go

@@ -7,9 +7,13 @@ import (
 
 	"github.com/go-test/deep"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/anchore/syft/syft/file"
+	"github.com/anchore/syft/syft/linux"
 	"github.com/anchore/syft/syft/pkg"
+	"github.com/anchore/syft/syft/pkg/cataloger/generic"
+	"github.com/anchore/syft/syft/source"
 )
 
 func TestExtraFileAttributes(t *testing.T) {
@@ -617,23 +621,14 @@ func TestSinglePackageDetails(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.fixture, func(t *testing.T) {
-			file, err := os.Open(test.fixture)
+			f, err := os.Open(test.fixture)
-			if err != nil {
+			require.NoError(t, err)
-				t.Fatal("Unable to read fixture: ", err)
+			t.Cleanup(func() { f.Close() })
-			}
-			defer func() {
-				err := file.Close()
-				if err != nil {
-					t.Fatal("closing file failed:", err)
-				}
-			}()
 
-			reader := bufio.NewReader(file)
+			reader := bufio.NewReader(f)
 
 			entry, err := parseApkDBEntry(reader)
-			if err != nil {
+			require.NoError(t, err)
-				t.Fatal("Unable to read file contents: ", err)
-			}
 
 			if diff := deep.Equal(*entry, test.expected); diff != nil {
 				for _, d := range diff {
@@ -647,16 +642,17 @@ func TestSinglePackageDetails(t *testing.T) {
 func TestMultiplePackages(t *testing.T) {
 	tests := []struct {
 		fixture  string
-		expected []*pkg.Package
+		expected []pkg.Package
 	}{
 		{
 			fixture: "test-fixtures/multiple",
-			expected: []*pkg.Package{
+			expected: []pkg.Package{
 				{
 					Name:         "libc-utils",
 					Version:      "0.7.2-r0",
 					Licenses:     []string{"BSD"},
 					Type:         pkg.ApkPkg,
+					PURL:         "pkg:alpine/libc-utils@0.7.2-r0?arch=x86_64&upstream=libc-dev&distro=alpine",
 					MetadataType: pkg.ApkMetadataType,
 					Metadata: pkg.ApkMetadata{
 						Package:          "libc-utils",
@@ -680,6 +676,7 @@ func TestMultiplePackages(t *testing.T) {
 					Version:      "1.1.24-r2",
 					Licenses:     []string{"MIT", "BSD", "GPL2+"},
 					Type:         pkg.ApkPkg,
+					PURL:         "pkg:alpine/musl-utils@1.1.24-r2?arch=x86_64&upstream=musl&distro=alpine",
 					MetadataType: pkg.ApkMetadataType,
 					Metadata: pkg.ApkMetadata{
 						Package:          "musl-utils",
@@ -764,22 +761,20 @@ func TestMultiplePackages(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.fixture, func(t *testing.T) {
-			file, err := os.Open(test.fixture)
+			f, err := os.Open(test.fixture)
-			if err != nil {
+			require.NoError(t, err)
-				t.Fatal("Unable to read: ", err)
+			t.Cleanup(func() { f.Close() })
-			}
-			defer func() {
-				err := file.Close()
-				if err != nil {
-					t.Fatal("closing file failed:", err)
-				}
-			}()
 
 			// TODO: no relationships are under test yet
-			pkgs, _, err := parseApkDB(file.Name(), file)
+			pkgs, _, err := parseApkDB(nil, &generic.Environment{
-			if err != nil {
+				LinuxRelease: &linux.Release{
-				t.Fatal("Unable to read file contents: ", err)
+					ID: "alpine",
-			}
+				},
+			}, source.LocationReadCloser{
+				Location:   source.NewLocation(f.Name()),
+				ReadCloser: f,
+			})
+			require.NoError(t, err)
 
 			if len(pkgs) != 2 {
 				t.Fatalf("unexpected number of entries: %d", len(pkgs))

syft/pkg/url_test.go

@@ -141,24 +141,6 @@ func TestPackageURL(t *testing.T) {
 			},
 			expected: "pkg:cargo/name@v0.1.0",
 		},
-		{
-			name: "apk",
-			distro: &linux.Release{
-				ID:        "alpine",
-				VersionID: "3.4.6",
-			},
-			pkg: Package{
-				Name:    "bad-name",
-				Version: "bad-v0.1.0",
-				Type:    ApkPkg,
-				Metadata: ApkMetadata{
-					Package:      "name",
-					Version:      "v0.1.0",
-					Architecture: "amd64",
-				},
-			},
-			expected: "pkg:alpine/name@v0.1.0?arch=amd64&distro=alpine-3.4.6",
-		},
 		{
 			name: "php-composer",
 			pkg: Package{
@@ -271,6 +253,7 @@ func TestPackageURL(t *testing.T) {
 	expectedTypes.Remove(string(KbPkg))
 	expectedTypes.Remove(string(PortagePkg))
 	expectedTypes.Remove(string(AlpmPkg))
+	expectedTypes.Remove(string(ApkPkg))
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {