diff --git a/internal/cmptest/common_options.go b/internal/cmptest/common_options.go index ecfb54edf..45962d46b 100644 --- a/internal/cmptest/common_options.go +++ b/internal/cmptest/common_options.go @@ -8,11 +8,15 @@ import ( "github.com/anchore/syft/syft/pkg" ) -func DefaultCommonOptions() []cmp.Option { - return CommonOptions(nil, nil) +func DefaultOptions() []cmp.Option { + return BuildOptions(nil, nil) } -func CommonOptions(licenseCmp LicenseComparer, locationCmp LocationComparer) []cmp.Option { +func DefaultIgnoreLocationLayerOptions() []cmp.Option { + return BuildOptions(LicenseComparerWithoutLocationLayer, LocationComparerWithoutLayer) +} + +func BuildOptions(licenseCmp LicenseComparer, locationCmp LocationComparer) []cmp.Option { if licenseCmp == nil { licenseCmp = DefaultLicenseComparer } @@ -25,47 +29,9 @@ func CommonOptions(licenseCmp LicenseComparer, locationCmp LocationComparer) []c cmpopts.IgnoreFields(pkg.Package{}, "id"), // note: ID is not deterministic for test purposes cmpopts.SortSlices(pkg.Less), cmpopts.SortSlices(DefaultRelationshipComparer), - cmp.Comparer( - func(x, y file.LocationSet) bool { - xs := x.ToSlice() - ys := y.ToSlice() - - if len(xs) != len(ys) { - return false - } - for i, xe := range xs { - ye := ys[i] - if !locationCmp(xe, ye) { - return false - } - } - - return true - }, - ), - cmp.Comparer( - func(x, y pkg.LicenseSet) bool { - xs := x.ToSlice() - ys := y.ToSlice() - - if len(xs) != len(ys) { - return false - } - for i, xe := range xs { - ye := ys[i] - if !licenseCmp(xe, ye) { - return false - } - } - - return true - }, - ), - cmp.Comparer( - locationCmp, - ), - cmp.Comparer( - licenseCmp, - ), + cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](locationCmp)), + cmp.Comparer(buildSetComparer[pkg.License, pkg.LicenseSet](licenseCmp)), + cmp.Comparer(locationCmp), + cmp.Comparer(licenseCmp), } } diff --git a/internal/cmptest/license.go b/internal/cmptest/license.go index f63cda296..8ab5fe840 100644 --- a/internal/cmptest/license.go +++ b/internal/cmptest/license.go @@ -10,20 +10,17 @@ import ( type LicenseComparer func(x, y pkg.License) bool func DefaultLicenseComparer(x, y pkg.License) bool { - return cmp.Equal(x, y, cmp.Comparer(DefaultLocationComparer), cmp.Comparer( - func(x, y file.LocationSet) bool { - xs := x.ToSlice() - ys := y.ToSlice() - if len(xs) != len(ys) { - return false - } - for i, xe := range xs { - ye := ys[i] - if !DefaultLocationComparer(xe, ye) { - return false - } - } - return true - }, - )) + return cmp.Equal( + x, y, + cmp.Comparer(DefaultLocationComparer), + cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](DefaultLocationComparer)), + ) +} + +func LicenseComparerWithoutLocationLayer(x, y pkg.License) bool { + return cmp.Equal( + x, y, + cmp.Comparer(LocationComparerWithoutLayer), + cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](LocationComparerWithoutLayer)), + ) } diff --git a/internal/cmptest/location.go b/internal/cmptest/location.go index b7654ab46..039136531 100644 --- a/internal/cmptest/location.go +++ b/internal/cmptest/location.go @@ -11,3 +11,7 @@ type LocationComparer func(x, y file.Location) bool func DefaultLocationComparer(x, y file.Location) bool { return cmp.Equal(x.Coordinates, y.Coordinates) && cmp.Equal(x.AccessPath, y.AccessPath) } + +func LocationComparerWithoutLayer(x, y file.Location) bool { + return cmp.Equal(x.Coordinates.RealPath, y.Coordinates.RealPath) && cmp.Equal(x.AccessPath, y.AccessPath) +} diff --git a/internal/cmptest/set.go b/internal/cmptest/set.go new file mode 100644 index 000000000..ab65823b0 --- /dev/null +++ b/internal/cmptest/set.go @@ -0,0 +1,24 @@ +package cmptest + +type slicer[T any] interface { + ToSlice() []T +} + +func buildSetComparer[T any, S slicer[T]](l func(x, y T) bool) func(x, y S) bool { + return func(x, y S) bool { + xs := x.ToSlice() + ys := y.ToSlice() + + if len(xs) != len(ys) { + return false + } + for i, xe := range xs { + ye := ys[i] + if !l(xe, ye) { + return false + } + } + + return true + } +} diff --git a/internal/evidence/constants.go b/internal/evidence/constants.go new file mode 100644 index 000000000..97f4bded6 --- /dev/null +++ b/internal/evidence/constants.go @@ -0,0 +1,9 @@ +package evidence + +// this package exists so that the file package can reference package evidence in tests without creating a circular dependency. + +const ( + AnnotationKey = "evidence" + PrimaryAnnotation = "primary" + SupportingAnnotation = "supporting" +) diff --git a/internal/relationship/by_file_ownership_test.go b/internal/relationship/by_file_ownership_test.go index f87b228a4..73cf4e482 100644 --- a/internal/relationship/by_file_ownership_test.go +++ b/internal/relationship/by_file_ownership_test.go @@ -145,7 +145,7 @@ func TestOwnershipByFilesRelationship(t *testing.T) { assert.Len(t, relationships, len(expectedRelations)) for idx, expectedRelationship := range expectedRelations { actualRelationship := relationships[idx] - if d := cmp.Diff(expectedRelationship, actualRelationship, cmptest.DefaultCommonOptions()...); d != "" { + if d := cmp.Diff(expectedRelationship, actualRelationship, cmptest.DefaultOptions()...); d != "" { t.Errorf("unexpected relationship (-want, +got): %s", d) } } diff --git a/syft/file/location_set_test.go b/syft/file/location_set_test.go index fabe3240c..49a37da11 100644 --- a/syft/file/location_set_test.go +++ b/syft/file/location_set_test.go @@ -1,15 +1,17 @@ package file import ( + "sort" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/anchore/syft/internal/evidence" "github.com/anchore/syft/syft/artifact" ) -func TestLocationSet(t *testing.T) { +func TestLocationSet_SortPaths(t *testing.T) { etcHostsLinkVar := Location{ LocationData: LocationData{ @@ -93,6 +95,117 @@ func TestLocationSet(t *testing.T) { } } +func TestLocationSet_SortEvidence(t *testing.T) { + primaryEvidence := map[string]string{evidence.AnnotationKey: evidence.PrimaryAnnotation} + secondaryEvidence := map[string]string{evidence.AnnotationKey: evidence.SupportingAnnotation} + + binPrimary := Location{ + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/bin", + FileSystemID: "a", + }, + AccessPath: "/usr/bin", + }, + LocationMetadata: LocationMetadata{ + Annotations: primaryEvidence, + }, + } + + binSecondary := Location{ + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/bin", + FileSystemID: "a", + }, + AccessPath: "/usr/bin", + }, + LocationMetadata: LocationMetadata{ + Annotations: secondaryEvidence, + }, + } + + binNoEvidence := Location{ + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/bin", + FileSystemID: "a", + }, + AccessPath: "/usr/bin", + }, + } + + etcHostsPrimary := Location{ + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/etc/hosts", + FileSystemID: "a", + }, + AccessPath: "/var/etc/hosts", + }, + LocationMetadata: LocationMetadata{ + Annotations: primaryEvidence, + }, + } + + etcHostsSecondary := Location{ + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/etc/hosts", + FileSystemID: "a", + }, + AccessPath: "/var/etc/hosts", + }, + LocationMetadata: LocationMetadata{ + Annotations: secondaryEvidence, + }, + } + + etcHostsNoEvidence := Location{ + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/etc/hosts", + FileSystemID: "a", + }, + AccessPath: "/var/etc/hosts", + }, + } + + tests := []struct { + name string + input []Location + expected []Location + }{ + { + name: "sort primary, secondary, tertiary, no evidence", + input: []Location{ + binNoEvidence, binPrimary, binSecondary, + }, + expected: []Location{ + binPrimary, binSecondary, binNoEvidence, + }, + }, + { + name: "sort by evidence, then path", + input: []Location{ + etcHostsNoEvidence, etcHostsSecondary, + binSecondary, binNoEvidence, + binPrimary, etcHostsPrimary, + }, + expected: []Location{ + binPrimary, etcHostsPrimary, binSecondary, etcHostsSecondary, binNoEvidence, etcHostsNoEvidence, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + sort.Sort(Locations(test.input)) + assert.Equal(t, test.expected, test.input) + }) + } +} + func TestLocationSet_Hash(t *testing.T) { etcAlink := Location{ LocationData: LocationData{ diff --git a/syft/file/locations.go b/syft/file/locations.go index 80a4503cb..013eb798d 100644 --- a/syft/file/locations.go +++ b/syft/file/locations.go @@ -1,5 +1,7 @@ package file +import "github.com/anchore/syft/internal/evidence" + type Locations []Location func (l Locations) Len() int { @@ -7,13 +9,25 @@ func (l Locations) Len() int { } func (l Locations) Less(i, j int) bool { - if l[i].RealPath == l[j].RealPath { - if l[i].AccessPath == l[j].AccessPath { - return l[i].FileSystemID < l[j].FileSystemID + liEvidence := l[i].Annotations[evidence.AnnotationKey] + ljEvidence := l[j].Annotations[evidence.AnnotationKey] + if liEvidence == ljEvidence { + if l[i].RealPath == l[j].RealPath { + if l[i].AccessPath == l[j].AccessPath { + return l[i].FileSystemID < l[j].FileSystemID + } + return l[i].AccessPath < l[j].AccessPath } - return l[i].AccessPath < l[j].AccessPath + return l[i].RealPath < l[j].RealPath } - return l[i].RealPath < l[j].RealPath + if liEvidence == evidence.PrimaryAnnotation { + return true + } + if ljEvidence == evidence.PrimaryAnnotation { + return false + } + + return liEvidence > ljEvidence } func (l Locations) Swap(i, j int) { diff --git a/syft/pkg/cataloger/debian/cataloger_test.go b/syft/pkg/cataloger/debian/cataloger_test.go index 299d7338d..478e52e36 100644 --- a/syft/pkg/cataloger/debian/cataloger_test.go +++ b/syft/pkg/cataloger/debian/cataloger_test.go @@ -30,11 +30,11 @@ func TestDpkgCataloger(t *testing.T) { pkg.NewLicenseFromLocations("LGPL-2.1", file.NewLocation("/usr/share/doc/libpam-runtime/copyright")), ), Locations: file.NewLocationSet( - file.NewLocation("/var/lib/dpkg/status"), - file.NewLocation("/var/lib/dpkg/info/libpam-runtime.preinst"), - file.NewLocation("/var/lib/dpkg/info/libpam-runtime.md5sums"), - file.NewLocation("/var/lib/dpkg/info/libpam-runtime.conffiles"), - file.NewLocation("/usr/share/doc/libpam-runtime/copyright"), + file.NewLocation("/var/lib/dpkg/status").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.PrimaryEvidenceAnnotation), + file.NewLocation("/var/lib/dpkg/info/libpam-runtime.preinst").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation), + file.NewLocation("/var/lib/dpkg/info/libpam-runtime.md5sums").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation), + file.NewLocation("/var/lib/dpkg/info/libpam-runtime.conffiles").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation), + file.NewLocation("/usr/share/doc/libpam-runtime/copyright").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation), ), Type: pkg.DebPkg, Metadata: pkg.DpkgDBEntry{ @@ -104,10 +104,10 @@ func TestDpkgCataloger(t *testing.T) { pkg.NewLicenseFromLocations("GPL-2", file.NewLocation("/usr/share/doc/libsqlite3-0/copyright")), ), Locations: file.NewLocationSet( - file.NewLocation("/var/lib/dpkg/status.d/libsqlite3-0"), - file.NewLocation("/var/lib/dpkg/status.d/libsqlite3-0.md5sums"), - file.NewLocation("/var/lib/dpkg/status.d/libsqlite3-0.preinst"), - file.NewLocation("/usr/share/doc/libsqlite3-0/copyright"), + file.NewLocation("/var/lib/dpkg/status.d/libsqlite3-0").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.PrimaryEvidenceAnnotation), + file.NewLocation("/var/lib/dpkg/status.d/libsqlite3-0.md5sums").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation), + file.NewLocation("/var/lib/dpkg/status.d/libsqlite3-0.preinst").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation), + file.NewLocation("/usr/share/doc/libsqlite3-0/copyright").WithAnnotation(pkg.EvidenceAnnotationKey, pkg.SupportingEvidenceAnnotation), ), Type: pkg.DebPkg, Metadata: pkg.DpkgDBEntry{ diff --git a/syft/pkg/cataloger/golang/stdlib_package_test.go b/syft/pkg/cataloger/golang/stdlib_package_test.go index 295266d33..0ced95b04 100644 --- a/syft/pkg/cataloger/golang/stdlib_package_test.go +++ b/syft/pkg/cataloger/golang/stdlib_package_test.go @@ -139,14 +139,14 @@ func Test_stdlibPackageAndRelationships_values(t *testing.T) { require.Len(t, gotPkgs, 1) gotPkg := gotPkgs[0] - if d := cmp.Diff(expectedPkg, gotPkg, cmptest.DefaultCommonOptions()...); d != "" { + if d := cmp.Diff(expectedPkg, gotPkg, cmptest.DefaultOptions()...); d != "" { t.Errorf("unexpected package (-want +got): %s", d) } require.Len(t, gotRels, 1) gotRel := gotRels[0] - if d := cmp.Diff(expectedRel, gotRel, cmptest.DefaultCommonOptions()...); d != "" { + if d := cmp.Diff(expectedRel, gotRel, cmptest.DefaultOptions()...); d != "" { t.Errorf("unexpected relationship (-want +got): %s", d) } diff --git a/syft/pkg/cataloger/internal/pkgtest/test_generic_parser.go b/syft/pkg/cataloger/internal/pkgtest/test_generic_parser.go index 96bd79450..6397d4d04 100644 --- a/syft/pkg/cataloger/internal/pkgtest/test_generic_parser.go +++ b/syft/pkg/cataloger/internal/pkgtest/test_generic_parser.go @@ -146,29 +146,8 @@ func (p *CatalogTester) ExpectsAssertion(a func(t *testing.T, pkgs []pkg.Package } func (p *CatalogTester) IgnoreLocationLayer() *CatalogTester { - p.locationComparer = func(x, y file.Location) bool { - return cmp.Equal(x.Coordinates.RealPath, y.Coordinates.RealPath) && cmp.Equal(x.AccessPath, y.AccessPath) - } - - // we need to update the license comparer to use the ignored location layer - p.licenseComparer = func(x, y pkg.License) bool { - return cmp.Equal(x, y, cmp.Comparer(p.locationComparer), cmp.Comparer( - func(x, y file.LocationSet) bool { - xs := x.ToSlice() - ys := y.ToSlice() - if len(xs) != len(ys) { - return false - } - for i, xe := range xs { - ye := ys[i] - if !p.locationComparer(xe, ye) { - return false - } - } - - return true - })) - } + p.locationComparer = cmptest.LocationComparerWithoutLayer + p.licenseComparer = cmptest.LicenseComparerWithoutLocationLayer return p } @@ -272,7 +251,7 @@ func (p *CatalogTester) TestCataloger(t *testing.T, cataloger pkg.Cataloger) { func (p *CatalogTester) assertPkgs(t *testing.T, pkgs []pkg.Package, relationships []artifact.Relationship) { t.Helper() - p.compareOptions = append(p.compareOptions, cmptest.CommonOptions(p.licenseComparer, p.locationComparer)...) + p.compareOptions = append(p.compareOptions, cmptest.BuildOptions(p.licenseComparer, p.locationComparer)...) { r := cmptest.NewDiffReporter() @@ -325,53 +304,20 @@ func TestFileParserWithEnv(t *testing.T, fixturePath string, parser generic.Pars NewCatalogTester().FromFile(t, fixturePath).WithEnv(env).Expects(expectedPkgs, expectedRelationships).TestParser(t, parser) } -func AssertPackagesEqual(t *testing.T, a, b pkg.Package) { +func AssertPackagesEqual(t *testing.T, a, b pkg.Package, userOpts ...cmp.Option) { t.Helper() - opts := []cmp.Option{ - cmpopts.IgnoreFields(pkg.Package{}, "id"), // note: ID is not deterministic for test purposes - cmp.Comparer( - func(x, y file.LocationSet) bool { - xs := x.ToSlice() - ys := y.ToSlice() + opts := cmptest.DefaultOptions() + opts = append(opts, userOpts...) - if len(xs) != len(ys) { - return false - } - for i, xe := range xs { - ye := ys[i] - if !cmptest.DefaultLocationComparer(xe, ye) { - return false - } - } - - return true - }, - ), - cmp.Comparer( - func(x, y pkg.LicenseSet) bool { - xs := x.ToSlice() - ys := y.ToSlice() - - if len(xs) != len(ys) { - return false - } - for i, xe := range xs { - ye := ys[i] - if !cmptest.DefaultLicenseComparer(xe, ye) { - return false - } - } - - return true - }, - ), - cmp.Comparer( - cmptest.DefaultLocationComparer, - ), - cmp.Comparer( - cmptest.DefaultLicenseComparer, - ), + if diff := cmp.Diff(a, b, opts...); diff != "" { + t.Errorf("unexpected packages from parsing (-expected +actual)\n%s", diff) } +} + +func AssertPackagesEqualIgnoreLayers(t *testing.T, a, b pkg.Package, userOpts ...cmp.Option) { + t.Helper() + opts := cmptest.DefaultIgnoreLocationLayerOptions() + opts = append(opts, userOpts...) if diff := cmp.Diff(a, b, opts...); diff != "" { t.Errorf("unexpected packages from parsing (-expected +actual)\n%s", diff) diff --git a/syft/pkg/cataloger/python/parse_wheel_egg_metadata_test.go b/syft/pkg/cataloger/python/parse_wheel_egg_metadata_test.go index 2ebc771e3..57fbf65ee 100644 --- a/syft/pkg/cataloger/python/parse_wheel_egg_metadata_test.go +++ b/syft/pkg/cataloger/python/parse_wheel_egg_metadata_test.go @@ -76,7 +76,7 @@ func TestParseWheelEggMetadata(t *testing.T) { t.Fatalf("failed to parse: %+v", err) } - if d := cmp.Diff(test.ExpectedMetadata, actual, cmptest.DefaultCommonOptions()...); d != "" { + if d := cmp.Diff(test.ExpectedMetadata, actual, cmptest.DefaultOptions()...); d != "" { t.Errorf("metadata mismatch (-want +got):\n%s", d) } }) diff --git a/syft/pkg/evidence.go b/syft/pkg/evidence.go index 9a7ae5f52..76daeee40 100644 --- a/syft/pkg/evidence.go +++ b/syft/pkg/evidence.go @@ -1,7 +1,9 @@ package pkg +import "github.com/anchore/syft/internal/evidence" + const ( - EvidenceAnnotationKey = "evidence" - PrimaryEvidenceAnnotation = "primary" - SupportingEvidenceAnnotation = "supporting" + EvidenceAnnotationKey = evidence.AnnotationKey + PrimaryEvidenceAnnotation = evidence.PrimaryAnnotation + SupportingEvidenceAnnotation = evidence.SupportingAnnotation )