From 59b880f26a15b88a2fc0b04f8e04185a095e5a27 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Tue, 13 May 2025 00:02:07 -0400 Subject: [PATCH] order locations by container layer order (#3858) Signed-off-by: Alex Goodman --- internal/cmptest/common_options.go | 37 +- internal/cmptest/license.go | 4 +- internal/cmptest/set.go | 8 +- syft/file/coordinate_set.go | 35 +- syft/file/location_set.go | 25 +- syft/file/location_set_test.go | 4 +- syft/file/locations.go | 127 +++- syft/file/locations_test.go | 557 ++++++++++++++ .../cyclonedxhelpers/to_format_model.go | 22 +- .../common/spdxhelpers/to_format_model.go | 24 +- .../cyclonedxutil/helpers/component.go | 4 +- .../cyclonedxutil/helpers/component_test.go | 4 +- syft/format/internal/location_sorter.go | 17 + syft/format/syftjson/to_format_model.go | 55 +- syft/format/syftjson/to_format_model_test.go | 700 +++++++++++++++++- .../container_image_squash_test.go | 16 +- syft/pkg/license_set.go | 31 +- syft/pkg/license_set_test.go | 2 +- 18 files changed, 1589 insertions(+), 83 deletions(-) create mode 100644 syft/file/locations_test.go create mode 100644 syft/format/internal/location_sorter.go diff --git a/internal/cmptest/common_options.go b/internal/cmptest/common_options.go index 45962d46b..fed53a6f5 100644 --- a/internal/cmptest/common_options.go +++ b/internal/cmptest/common_options.go @@ -1,9 +1,12 @@ package cmptest import ( + "strings" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/anchore/syft/internal/evidence" "github.com/anchore/syft/syft/file" "github.com/anchore/syft/syft/pkg" ) @@ -29,9 +32,41 @@ func BuildOptions(licenseCmp LicenseComparer, locationCmp LocationComparer) []cm cmpopts.IgnoreFields(pkg.Package{}, "id"), // note: ID is not deterministic for test purposes cmpopts.SortSlices(pkg.Less), cmpopts.SortSlices(DefaultRelationshipComparer), - cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](locationCmp)), + cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](locationCmp, locationSorter)), cmp.Comparer(buildSetComparer[pkg.License, pkg.LicenseSet](licenseCmp)), cmp.Comparer(locationCmp), cmp.Comparer(licenseCmp), } } + +// LocationSorter always sorts by evidence annotations first, then by access path, then by real path. +// This intentionally does not consider layer details since some test fixtures have no layer information +// on the left side of the comparison (expected) and does on the right side (actual). +func locationSorter(a, b file.Location) int { + // compare by evidence annotations first... + aEvidence := a.Annotations[evidence.AnnotationKey] + bEvidence := b.Annotations[evidence.AnnotationKey] + + if aEvidence != bEvidence { + if aEvidence == evidence.PrimaryAnnotation { + return -1 + } + if bEvidence == evidence.PrimaryAnnotation { + return 1 + } + + if aEvidence > bEvidence { + return -1 + } + if bEvidence > aEvidence { + return 1 + } + } + + // ...then by paths + if a.AccessPath != b.AccessPath { + return strings.Compare(a.AccessPath, b.AccessPath) + } + + return strings.Compare(a.RealPath, b.RealPath) +} diff --git a/internal/cmptest/license.go b/internal/cmptest/license.go index 8ab5fe840..186e835fa 100644 --- a/internal/cmptest/license.go +++ b/internal/cmptest/license.go @@ -13,7 +13,7 @@ func DefaultLicenseComparer(x, y pkg.License) bool { return cmp.Equal( x, y, cmp.Comparer(DefaultLocationComparer), - cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](DefaultLocationComparer)), + cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](DefaultLocationComparer, locationSorter)), ) } @@ -21,6 +21,6 @@ func LicenseComparerWithoutLocationLayer(x, y pkg.License) bool { return cmp.Equal( x, y, cmp.Comparer(LocationComparerWithoutLayer), - cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](LocationComparerWithoutLayer)), + cmp.Comparer(buildSetComparer[file.Location, file.LocationSet](LocationComparerWithoutLayer, locationSorter)), ) } diff --git a/internal/cmptest/set.go b/internal/cmptest/set.go index ab65823b0..b5de85c29 100644 --- a/internal/cmptest/set.go +++ b/internal/cmptest/set.go @@ -1,13 +1,13 @@ package cmptest type slicer[T any] interface { - ToSlice() []T + ToSlice(sorter ...func(a, b T) int) []T } -func buildSetComparer[T any, S slicer[T]](l func(x, y T) bool) func(x, y S) bool { +func buildSetComparer[T any, S slicer[T]](l func(x, y T) bool, sorters ...func(a, b T) int) func(x, y S) bool { return func(x, y S) bool { - xs := x.ToSlice() - ys := y.ToSlice() + xs := x.ToSlice(sorters...) + ys := y.ToSlice(sorters...) if len(xs) != len(ys) { return false diff --git a/syft/file/coordinate_set.go b/syft/file/coordinate_set.go index f84ea7206..c5ea0be94 100644 --- a/syft/file/coordinate_set.go +++ b/syft/file/coordinate_set.go @@ -59,7 +59,34 @@ func (s CoordinateSet) Paths() []string { return pathSlice } -func (s CoordinateSet) ToSlice() []Coordinates { +func (s CoordinateSet) ToSlice(sorters ...func(a, b Coordinates) int) []Coordinates { + coordinates := s.ToUnorderedSlice() + + var sorted bool + for _, sorter := range sorters { + if sorter == nil { + continue + } + sort.Slice(coordinates, func(i, j int) bool { + return sorter(coordinates[i], coordinates[j]) < 0 + }) + sorted = true + break + } + + if !sorted { + sort.SliceStable(coordinates, func(i, j int) bool { + if coordinates[i].FileSystemID == coordinates[j].FileSystemID { + return coordinates[i].RealPath < coordinates[j].RealPath + } + return coordinates[i].FileSystemID < coordinates[j].FileSystemID + }) + } + + return coordinates +} + +func (s CoordinateSet) ToUnorderedSlice() []Coordinates { if s.set == nil { return nil } @@ -69,12 +96,6 @@ func (s CoordinateSet) ToSlice() []Coordinates { coordinates[idx] = v idx++ } - sort.SliceStable(coordinates, func(i, j int) bool { - if coordinates[i].RealPath == coordinates[j].RealPath { - return coordinates[i].FileSystemID < coordinates[j].FileSystemID - } - return coordinates[i].RealPath < coordinates[j].RealPath - }) return coordinates } diff --git a/syft/file/location_set.go b/syft/file/location_set.go index a49142b7c..9ee3a236a 100644 --- a/syft/file/location_set.go +++ b/syft/file/location_set.go @@ -1,6 +1,7 @@ package file import ( + "slices" "sort" "github.com/gohugoio/hashstructure" @@ -54,7 +55,28 @@ func (s LocationSet) Contains(l Location) bool { return ok } -func (s LocationSet) ToSlice() []Location { +func (s LocationSet) ToSlice(sorters ...func(a, b Location) int) []Location { + locations := s.ToUnorderedSlice() + + var sorted bool + for _, sorter := range sorters { + if sorter == nil { + continue + } + slices.SortFunc(locations, sorter) + sorted = true + break + } + + if !sorted { + // though no sorter was passed, we need to guarantee a stable ordering between calls + sort.Sort(Locations(locations)) + } + + return locations +} + +func (s LocationSet) ToUnorderedSlice() []Location { if s.set == nil { return nil } @@ -67,7 +89,6 @@ func (s LocationSet) ToSlice() []Location { } idx++ } - sort.Sort(Locations(locations)) return locations } diff --git a/syft/file/location_set_test.go b/syft/file/location_set_test.go index 49a37da11..b80028325 100644 --- a/syft/file/location_set_test.go +++ b/syft/file/location_set_test.go @@ -89,8 +89,8 @@ func TestLocationSet_SortPaths(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - set := NewLocationSet(test.input...) - assert.Equal(t, test.expected, set.ToSlice()) + actual := NewLocationSet(test.input...).ToSlice() + assert.Equal(t, test.expected, actual) }) } } diff --git a/syft/file/locations.go b/syft/file/locations.go index 013eb798d..f7836b5ea 100644 --- a/syft/file/locations.go +++ b/syft/file/locations.go @@ -1,6 +1,12 @@ package file -import "github.com/anchore/syft/internal/evidence" +import ( + "strings" + + "github.com/anchore/syft/internal/evidence" +) + +var locationSorterWithoutLayers = LocationSorter(nil) type Locations []Location @@ -9,27 +15,108 @@ func (l Locations) Len() int { } func (l Locations) Less(i, j int) bool { - 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].RealPath < l[j].RealPath - } - if liEvidence == evidence.PrimaryAnnotation { - return true - } - if ljEvidence == evidence.PrimaryAnnotation { - return false - } - - return liEvidence > ljEvidence + return locationSorterWithoutLayers(l[i], l[j]) < 0 } func (l Locations) Swap(i, j int) { l[i], l[j] = l[j], l[i] } + +// LocationSorter creates a comparison function (slices.SortFunc) for Location objects based on layer order +func LocationSorter(layers []string) func(a, b Location) int { //nolint:gocognit + var layerOrderByDigest map[string]int + if len(layers) > 0 { + layerOrderByDigest = make(map[string]int) + for i, digest := range layers { + layerOrderByDigest[digest] = i + } + } + + return func(a, b Location) int { + // compare by evidence annotations first... + aEvidence := a.Annotations[evidence.AnnotationKey] + bEvidence := b.Annotations[evidence.AnnotationKey] + + if aEvidence != bEvidence { + if aEvidence == evidence.PrimaryAnnotation { + return -1 + } + if bEvidence == evidence.PrimaryAnnotation { + return 1 + } + + if aEvidence > bEvidence { + return -1 + } + if bEvidence > aEvidence { + return 1 + } + } + + // ...then by layer order + if layerOrderByDigest != nil { + // we're given layer order details + aLayerIdx, aExists := layerOrderByDigest[a.FileSystemID] + bLayerIdx, bExists := layerOrderByDigest[b.FileSystemID] + + if aLayerIdx != bLayerIdx { + if !aExists && !bExists { + return strings.Compare(a.FileSystemID, b.FileSystemID) + } + if !aExists { + return 1 + } + if !bExists { + return -1 + } + + return aLayerIdx - bLayerIdx + } + } else if a.FileSystemID != b.FileSystemID { + // no layer info given, legacy behavior is to sort lexicographically + return strings.Compare(a.FileSystemID, b.FileSystemID) + } + + // ...then by paths + if a.AccessPath != b.AccessPath { + return strings.Compare(a.AccessPath, b.AccessPath) + } + + return strings.Compare(a.RealPath, b.RealPath) + } +} + +// CoordinatesSorter creates a comparison function (slices.SortFunc) for Coordinate objects based on layer order +func CoordinatesSorter(layers []string) func(a, b Coordinates) int { + var layerOrderByDigest map[string]int + if len(layers) > 0 { + layerOrderByDigest = make(map[string]int) + for i, digest := range layers { + layerOrderByDigest[digest] = i + } + } + + return func(a, b Coordinates) int { + // ...then by layer order + if layerOrderByDigest != nil { + aLayerIdx, aExists := layerOrderByDigest[a.FileSystemID] + bLayerIdx, bExists := layerOrderByDigest[b.FileSystemID] + + if aLayerIdx != bLayerIdx { + if !aExists && !bExists { + return strings.Compare(a.FileSystemID, b.FileSystemID) + } + if !aExists { + return 1 + } + if !bExists { + return -1 + } + + return aLayerIdx - bLayerIdx + } + } + + return strings.Compare(a.RealPath, b.RealPath) + } +} diff --git a/syft/file/locations_test.go b/syft/file/locations_test.go new file mode 100644 index 000000000..07e903d90 --- /dev/null +++ b/syft/file/locations_test.go @@ -0,0 +1,557 @@ +package file + +import ( + "fmt" + "slices" + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/anchore/syft/internal/evidence" +) + +func TestLocationAndCoordinatesSorters(t *testing.T) { + tests := []struct { + name string + layers []string + locs []Location + coords []Coordinates + wantLocs []string + wantCoords []string + }{ + { + name: "empty location slice", + layers: []string{"fsid-1"}, + locs: []Location{}, + coords: []Coordinates{}, + wantLocs: []string{}, + wantCoords: []string{}, + }, + { + name: "nil layer list", + layers: nil, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + AccessPath: "/a", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{}, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-2", + }, + AccessPath: "/b", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{}, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/a", + FileSystemID: "fsid-1", + }, + { + RealPath: "/b", + FileSystemID: "fsid-2", + }, + }, + wantLocs: []string{ + "/a (/a) @ fsid-1 map[]", + "/b (/b) @ fsid-2 map[]", + }, + wantCoords: []string{ + "/a @ fsid-1", + "/b @ fsid-2", + }, + }, + { + name: "sort by evidence type only", + layers: []string{"fsid-1"}, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + AccessPath: "/a", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: "", + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + AccessPath: "/b", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.SupportingAnnotation, + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-1", + }, + AccessPath: "/c", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/a", + FileSystemID: "fsid-1", + }, + { + RealPath: "/b", + FileSystemID: "fsid-1", + }, + { + RealPath: "/c", + FileSystemID: "fsid-1", + }, + }, + wantLocs: []string{ + "/c (/c) @ fsid-1 map[evidence:primary]", + "/b (/b) @ fsid-1 map[evidence:supporting]", + "/a (/a) @ fsid-1 map[evidence:]", + }, + wantCoords: []string{ + "/a @ fsid-1", + "/b @ fsid-1", + "/c @ fsid-1", + }, + }, + { + name: "same evidence type, sort by layer", + layers: []string{"fsid-1", "fsid-2", "fsid-3"}, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-3", + }, + AccessPath: "/a", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + AccessPath: "/b", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-2", + }, + AccessPath: "/c", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/a", + FileSystemID: "fsid-3", + }, + { + RealPath: "/b", + FileSystemID: "fsid-1", + }, + { + RealPath: "/c", + FileSystemID: "fsid-2", + }, + }, + wantLocs: []string{ + "/b (/b) @ fsid-1 map[evidence:primary]", + "/c (/c) @ fsid-2 map[evidence:primary]", + "/a (/a) @ fsid-3 map[evidence:primary]", + }, + wantCoords: []string{ + "/b @ fsid-1", + "/c @ fsid-2", + "/a @ fsid-3", + }, + }, + { + name: "same evidence and layer, sort by access path", + layers: []string{"fsid-1"}, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/x", + FileSystemID: "fsid-1", + }, + AccessPath: "/c", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/y", + FileSystemID: "fsid-1", + }, + AccessPath: "/a", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/z", + FileSystemID: "fsid-1", + }, + AccessPath: "/b", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/x", + FileSystemID: "fsid-1", + }, + { + RealPath: "/y", + FileSystemID: "fsid-1", + }, + { + RealPath: "/z", + FileSystemID: "fsid-1", + }, + }, + wantLocs: []string{ + "/y (/a) @ fsid-1 map[evidence:primary]", + "/z (/b) @ fsid-1 map[evidence:primary]", + "/x (/c) @ fsid-1 map[evidence:primary]", + }, + wantCoords: []string{ + "/x @ fsid-1", + "/y @ fsid-1", + "/z @ fsid-1", + }, + }, + { + name: "same evidence, layer, and access path - sort by real path", + layers: []string{"fsid-1"}, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-1", + }, + AccessPath: "/same", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + AccessPath: "/same", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + AccessPath: "/same", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/c", + FileSystemID: "fsid-1", + }, + { + RealPath: "/a", + FileSystemID: "fsid-1", + }, + { + RealPath: "/b", + FileSystemID: "fsid-1", + }, + }, + wantLocs: []string{ + "/a (/same) @ fsid-1 map[evidence:primary]", + "/b (/same) @ fsid-1 map[evidence:primary]", + "/c (/same) @ fsid-1 map[evidence:primary]", + }, + wantCoords: []string{ + "/a @ fsid-1", + "/b @ fsid-1", + "/c @ fsid-1", + }, + }, + { + name: "unknown layers", + layers: []string{"fsid-1", "fsid-2"}, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/a", + FileSystemID: "unknown-1", + }, + AccessPath: "/a", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{}, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/b", + FileSystemID: "unknown-2", + }, + AccessPath: "/b", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{}, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/a", + FileSystemID: "unknown-1", + }, + { + RealPath: "/b", + FileSystemID: "unknown-2", + }, + }, + wantLocs: []string{ + "/a (/a) @ unknown-1 map[]", + "/b (/b) @ unknown-2 map[]", + }, + wantCoords: []string{ + "/a @ unknown-1", + "/b @ unknown-2", + }, + }, + { + name: "mixed known and unknown layers", + layers: []string{"fsid-1", "fsid-2"}, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + AccessPath: "/a", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{}, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/b", + FileSystemID: "unknown", + }, + AccessPath: "/b", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{}, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/a", + FileSystemID: "fsid-1", + }, + { + RealPath: "/b", + FileSystemID: "unknown", + }, + }, + wantLocs: []string{ + "/a (/a) @ fsid-1 map[]", + "/b (/b) @ unknown map[]", + }, + wantCoords: []string{ + "/a @ fsid-1", + "/b @ unknown", + }, + }, + { + name: "evidence comparison when one has none", + layers: []string{"fsid-1"}, + locs: []Location{ + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + AccessPath: "/a", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + // No evidence.AnnotationKey + }, + }, + }, + { + LocationData: LocationData{ + Coordinates: Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + AccessPath: "/b", + }, + LocationMetadata: LocationMetadata{ + Annotations: map[string]string{ + evidence.AnnotationKey: evidence.PrimaryAnnotation, + }, + }, + }, + }, + coords: []Coordinates{ + { + RealPath: "/a", + FileSystemID: "fsid-1", + }, + { + RealPath: "/b", + FileSystemID: "fsid-1", + }, + }, + wantLocs: []string{ + "/b (/b) @ fsid-1 map[evidence:primary]", + "/a (/a) @ fsid-1 map[]", + }, + wantCoords: []string{ + "/a @ fsid-1", + "/b @ fsid-1", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run("Location", func(t *testing.T) { + locs := make([]Location, len(tt.locs)) + copy(locs, tt.locs) + + slices.SortFunc(locs, LocationSorter(tt.layers)) + + got := make([]string, len(locs)) + for i, loc := range locs { + got[i] = fmt.Sprintf("%s (%s) @ %s %s", + loc.RealPath, + loc.AccessPath, + loc.FileSystemID, + loc.LocationMetadata.Annotations) + } + + if d := cmp.Diff(tt.wantLocs, got); d != "" { + t.Errorf("LocationSorter() mismatch (-want +got):\n%s", d) + } + }) + + t.Run("Coordinates", func(t *testing.T) { + coords := make([]Coordinates, len(tt.coords)) + copy(coords, tt.coords) + + slices.SortFunc(coords, CoordinatesSorter(tt.layers)) + + got := make([]string, len(coords)) + for i, coord := range coords { + got[i] = fmt.Sprintf("%s @ %s", + coord.RealPath, + coord.FileSystemID) + } + + if d := cmp.Diff(tt.wantCoords, got); d != "" { + t.Errorf("CoordinatesSorter() mismatch (-want +got):\n%s", d) + } + }) + }) + } +} diff --git a/syft/format/common/cyclonedxhelpers/to_format_model.go b/syft/format/common/cyclonedxhelpers/to_format_model.go index d3dc1b776..32986608f 100644 --- a/syft/format/common/cyclonedxhelpers/to_format_model.go +++ b/syft/format/common/cyclonedxhelpers/to_format_model.go @@ -42,17 +42,18 @@ func ToFormatModel(s sbom.SBOM) *cyclonedx.BOM { cdxBOM.SerialNumber = uuid.New().URN() cdxBOM.Metadata = toBomDescriptor(s.Descriptor.Name, s.Descriptor.Version, s.Source) + coordinates, locationSorter := getCoordinates(s) + // Packages packages := s.Artifacts.Packages.Sorted() components := make([]cyclonedx.Component, len(packages)) for i, p := range packages { - components[i] = helpers.EncodeComponent(p) + components[i] = helpers.EncodeComponent(p, locationSorter) } components = append(components, toOSComponent(s.Artifacts.LinuxDistribution)...) - // Files artifacts := s.Artifacts - coordinates := s.AllCoordinates() + for _, coordinate := range coordinates { var metadata *file.Metadata // File Info @@ -95,6 +96,21 @@ func ToFormatModel(s sbom.SBOM) *cyclonedx.BOM { return cdxBOM } +func getCoordinates(s sbom.SBOM) ([]file.Coordinates, func(a, b file.Location) int) { + var layers []string + if m, ok := s.Source.Metadata.(source.ImageMetadata); ok { + for _, l := range m.Layers { + layers = append(layers, l.Digest) + } + } + + coordSorter := file.CoordinatesSorter(layers) + coordinates := s.AllCoordinates() + + slices.SortFunc(coordinates, coordSorter) + return coordinates, file.LocationSorter(layers) +} + func digestsToHashes(digests []file.Digest) []cyclonedx.Hash { var hashes []cyclonedx.Hash for _, digest := range digests { diff --git a/syft/format/common/spdxhelpers/to_format_model.go b/syft/format/common/spdxhelpers/to_format_model.go index eca71d866..9e05ca380 100644 --- a/syft/format/common/spdxhelpers/to_format_model.go +++ b/syft/format/common/spdxhelpers/to_format_model.go @@ -22,6 +22,7 @@ import ( "github.com/anchore/syft/internal/spdxlicense" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" + formatInternal "github.com/anchore/syft/syft/format/internal" "github.com/anchore/syft/syft/format/internal/spdxutil/helpers" "github.com/anchore/syft/syft/pkg" "github.com/anchore/syft/syft/sbom" @@ -613,14 +614,19 @@ func lookupRelationship(ty artifact.RelationshipType) (bool, helpers.Relationshi func toFiles(s sbom.SBOM) (results []*spdx.File) { artifacts := s.Artifacts - for _, coordinates := range s.AllCoordinates() { + _, coordinateSorter := formatInternal.GetLocationSorters(s) + + coordinates := s.AllCoordinates() + slices.SortFunc(coordinates, coordinateSorter) + + for _, c := range coordinates { var metadata *file.Metadata - if metadataForLocation, exists := artifacts.FileMetadata[coordinates]; exists { + if metadataForLocation, exists := artifacts.FileMetadata[c]; exists { metadata = &metadataForLocation } var digests []file.Digest - if digestsForLocation, exists := artifacts.FileDigests[coordinates]; exists { + if digestsForLocation, exists := artifacts.FileDigests[c]; exists { digests = digestsForLocation } @@ -636,18 +642,18 @@ func toFiles(s sbom.SBOM) (results []*spdx.File) { // TODO: add file classifications (?) and content as a snippet var comment string - if coordinates.FileSystemID != "" { - comment = fmt.Sprintf("layerID: %s", coordinates.FileSystemID) + if c.FileSystemID != "" { + comment = fmt.Sprintf("layerID: %s", c.FileSystemID) } - relativePath, err := convertAbsoluteToRelative(coordinates.RealPath) + relativePath, err := convertAbsoluteToRelative(c.RealPath) if err != nil { - log.Debugf("unable to convert relative path '%s' to absolute path: %s", coordinates.RealPath, err) - relativePath = coordinates.RealPath + log.Debugf("unable to convert relative path '%s' to absolute path: %s", c.RealPath, err) + relativePath = c.RealPath } results = append(results, &spdx.File{ - FileSPDXIdentifier: toSPDXID(coordinates), + FileSPDXIdentifier: toSPDXID(c), FileComment: comment, // required, no attempt made to determine license information LicenseConcluded: noAssertion, diff --git a/syft/format/internal/cyclonedxutil/helpers/component.go b/syft/format/internal/cyclonedxutil/helpers/component.go index ce060d3af..aae3488e2 100644 --- a/syft/format/internal/cyclonedxutil/helpers/component.go +++ b/syft/format/internal/cyclonedxutil/helpers/component.go @@ -13,7 +13,7 @@ import ( "github.com/anchore/syft/syft/pkg" ) -func EncodeComponent(p pkg.Package) cyclonedx.Component { +func EncodeComponent(p pkg.Package, locationSorter func(a, b file.Location) int) cyclonedx.Component { props := EncodeProperties(p, "syft:package") if p.Metadata != nil { @@ -25,7 +25,7 @@ func EncodeComponent(p pkg.Package) cyclonedx.Component { } props = append(props, encodeCPEs(p)...) - locations := p.Locations.ToSlice() + locations := p.Locations.ToSlice(locationSorter) if len(locations) > 0 { props = append(props, EncodeProperties(locations, "syft:location")...) } diff --git a/syft/format/internal/cyclonedxutil/helpers/component_test.go b/syft/format/internal/cyclonedxutil/helpers/component_test.go index 7f2b9d245..c48f61f27 100644 --- a/syft/format/internal/cyclonedxutil/helpers/component_test.go +++ b/syft/format/internal/cyclonedxutil/helpers/component_test.go @@ -152,7 +152,7 @@ func Test_encodeComponentProperties(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - c := EncodeComponent(test.input) + c := EncodeComponent(test.input, file.LocationSorter(nil)) if test.expected == nil { if c.Properties != nil { t.Fatalf("expected no properties, got: %+v", *c.Properties) @@ -212,7 +212,7 @@ func Test_encodeCompomentType(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tt.pkg.ID() - p := EncodeComponent(tt.pkg) + p := EncodeComponent(tt.pkg, file.LocationSorter(nil)) assert.Equal(t, tt.want, p) }) } diff --git a/syft/format/internal/location_sorter.go b/syft/format/internal/location_sorter.go new file mode 100644 index 000000000..c40afe6dc --- /dev/null +++ b/syft/format/internal/location_sorter.go @@ -0,0 +1,17 @@ +package internal + +import ( + "github.com/anchore/syft/syft/file" + "github.com/anchore/syft/syft/sbom" + "github.com/anchore/syft/syft/source" +) + +func GetLocationSorters(s sbom.SBOM) (func(a, b file.Location) int, func(a, b file.Coordinates) int) { + var layers []string + if m, ok := s.Source.Metadata.(source.ImageMetadata); ok { + for _, l := range m.Layers { + layers = append(layers, l.Digest) + } + } + return file.LocationSorter(layers), file.CoordinatesSorter(layers) +} diff --git a/syft/format/syftjson/to_format_model.go b/syft/format/syftjson/to_format_model.go index 7cb264bef..79a9a57e1 100644 --- a/syft/format/syftjson/to_format_model.go +++ b/syft/format/syftjson/to_format_model.go @@ -10,6 +10,7 @@ import ( "github.com/anchore/syft/internal/log" "github.com/anchore/syft/syft/artifact" "github.com/anchore/syft/syft/file" + formatInternal "github.com/anchore/syft/syft/format/internal" "github.com/anchore/syft/syft/format/syftjson/model" "github.com/anchore/syft/syft/internal/packagemetadata" "github.com/anchore/syft/syft/internal/sourcemetadata" @@ -33,10 +34,12 @@ func metadataType(metadata interface{}, legacy bool) string { // ToFormatModel transforms the sbom import a format-specific model. func ToFormatModel(s sbom.SBOM, cfg EncoderConfig) model.Document { + locationSorter, coordinateSorter := formatInternal.GetLocationSorters(s) + return model.Document{ - Artifacts: toPackageModels(s.Artifacts.Packages, cfg), + Artifacts: toPackageModels(s.Artifacts.Packages, locationSorter, cfg), ArtifactRelationships: toRelationshipModel(s.Relationships), - Files: toFile(s), + Files: toFile(s, coordinateSorter), Source: toSourceModel(s.Source), Distro: toLinuxReleaser(s.Artifacts.LinuxDistribution), Descriptor: toDescriptor(s.Descriptor), @@ -81,7 +84,7 @@ func toDescriptor(d sbom.Descriptor) model.Descriptor { } } -func toFile(s sbom.SBOM) []model.File { +func toFile(s sbom.SBOM, coordinateSorter func(a, b file.Coordinates) int) []model.File { results := make([]model.File, 0) artifacts := s.Artifacts @@ -141,13 +144,21 @@ func toFile(s sbom.SBOM) []model.File { }) } - // sort by real path then virtual path to ensure the result is stable across multiple runs - sort.SliceStable(results, func(i, j int) bool { - return results[i].Location.RealPath < results[j].Location.RealPath - }) + // sort to ensure we're stable across multiple runs + // should order by the layer order from the container image then by real path + sortFiles(results, coordinateSorter) return results } +func sortFiles(files []model.File, coordinateSorter func(a, b file.Coordinates) int) { + fileSorter := func(a, b model.File) int { + return coordinateSorter(a.Location, b.Location) + } + sort.SliceStable(files, func(i, j int) bool { + return fileSorter(files[i], files[j]) < 0 + }) +} + func toFileMetadataEntry(coordinates file.Coordinates, metadata *file.Metadata) *model.FileMetadataEntry { if metadata == nil { return nil @@ -155,7 +166,7 @@ func toFileMetadataEntry(coordinates file.Coordinates, metadata *file.Metadata) var mode int var size int64 - if metadata != nil && metadata.FileInfo != nil { + if metadata.FileInfo != nil { var err error mode, err = strconv.Atoi(fmt.Sprintf("%o", metadata.Mode())) @@ -203,25 +214,19 @@ func toFileType(ty stereoscopeFile.Type) string { } } -func toPackageModels(catalog *pkg.Collection, cfg EncoderConfig) []model.Package { +func toPackageModels(catalog *pkg.Collection, locationSorter func(a, b file.Location) int, cfg EncoderConfig) []model.Package { artifacts := make([]model.Package, 0) if catalog == nil { return artifacts } for _, p := range catalog.Sorted() { - artifacts = append(artifacts, toPackageModel(p, cfg)) + artifacts = append(artifacts, toPackageModel(p, locationSorter, cfg)) } return artifacts } -func toLicenseModel(pkgLicenses []pkg.License) (modelLicenses []model.License) { +func toLicenseModel(pkgLicenses []pkg.License, locationSorter func(a, b file.Location) int) (modelLicenses []model.License) { for _, l := range pkgLicenses { - // guarantee collection - locations := make([]file.Location, 0) - if v := l.Locations.ToSlice(); v != nil { - locations = v - } - // format model must have allocated collections urls := l.URLs if urls == nil { @@ -234,14 +239,14 @@ func toLicenseModel(pkgLicenses []pkg.License) (modelLicenses []model.License) { Contents: l.Contents, Type: l.Type, URLs: urls, - Locations: locations, + Locations: toLocationsModel(l.Locations, locationSorter), }) } return } // toPackageModel crates a new Package from the given pkg.Package. -func toPackageModel(p pkg.Package, cfg EncoderConfig) model.Package { +func toPackageModel(p pkg.Package, locationSorter func(a, b file.Location) int, cfg EncoderConfig) model.Package { var cpes = make([]model.CPE, len(p.CPEs)) for i, c := range p.CPEs { convertedCPE := model.CPE{ @@ -255,7 +260,7 @@ func toPackageModel(p pkg.Package, cfg EncoderConfig) model.Package { // initializing the array; this is a good choke point for this check var licenses = make([]model.License, 0) if !p.Licenses.Empty() { - licenses = toLicenseModel(p.Licenses.ToSlice()) + licenses = toLicenseModel(p.Licenses.ToSlice(), locationSorter) } return model.Package{ @@ -265,7 +270,7 @@ func toPackageModel(p pkg.Package, cfg EncoderConfig) model.Package { Version: p.Version, Type: p.Type, FoundBy: p.FoundBy, - Locations: p.Locations.ToSlice(), + Locations: toLocationsModel(p.Locations, locationSorter), Licenses: licenses, Language: p.Language, CPEs: cpes, @@ -278,6 +283,14 @@ func toPackageModel(p pkg.Package, cfg EncoderConfig) model.Package { } } +func toLocationsModel(locations file.LocationSet, locationSorter func(a, b file.Location) int) []file.Location { + if locations.Empty() { + return []file.Location{} + } + + return locations.ToSlice(locationSorter) +} + func toRelationshipModel(relationships []artifact.Relationship) []model.Relationship { result := make([]model.Relationship, len(relationships)) for i, r := range relationships { diff --git a/syft/format/syftjson/to_format_model_test.go b/syft/format/syftjson/to_format_model_test.go index a04c054ee..5450439ca 100644 --- a/syft/format/syftjson/to_format_model_test.go +++ b/syft/format/syftjson/to_format_model_test.go @@ -345,9 +345,707 @@ func Test_toPackageModel_metadataType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if d := cmp.Diff(tt.want, toPackageModel(tt.p, tt.cfg), cmpopts.EquateEmpty()); d != "" { + if d := cmp.Diff(tt.want, toPackageModel(tt.p, file.LocationSorter(nil), tt.cfg), cmpopts.EquateEmpty()); d != "" { t.Errorf("unexpected package (-want +got):\n%s", d) } }) } } + +func Test_toPackageModel_layerOrdering(t *testing.T) { + tests := []struct { + name string + p pkg.Package + layerOrder []string + cfg EncoderConfig + want model.Package + }{ + { + name: "with layer ordering", + p: pkg.Package{ + Name: "pkg-1", + Licenses: pkg.NewLicenseSet(pkg.License{ + Value: "MIT", + Locations: file.NewLocationSet( + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/lic-a", + FileSystemID: "fsid-3", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/lic-a", + FileSystemID: "fsid-1", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/lic-b", + FileSystemID: "fsid-0", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/lic-a", + FileSystemID: "fsid-2", + }), + ), + }), + Locations: file.NewLocationSet( + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-3", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-0", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-2", + }), + ), + }, + layerOrder: []string{ + "fsid-0", + "fsid-1", + "fsid-2", + "fsid-3", + }, + want: model.Package{ + PackageBasicData: model.PackageBasicData{ + Name: "pkg-1", + Licenses: []model.License{ + { + Value: "MIT", + Locations: []file.Location{ + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/lic-b", + FileSystemID: "fsid-0", // important! + }, + AccessPath: "/lic-b", + }, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/lic-a", + FileSystemID: "fsid-1", // important! + }, + AccessPath: "/lic-a", + }, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/lic-a", + FileSystemID: "fsid-2", // important! + }, + AccessPath: "/lic-a", + }, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/lic-a", + FileSystemID: "fsid-3", // important! + }, + AccessPath: "/lic-a", + }, + }, + }, + }, + }, + Locations: []file.Location{ + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-0", // important! + }, + AccessPath: "/b", + }, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", // important! + }, + AccessPath: "/a", + }, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-2", // important! + }, + AccessPath: "/a", + }, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-3", // important! + }, + AccessPath: "/a", + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if d := cmp.Diff(tt.want, toPackageModel(tt.p, file.LocationSorter(tt.layerOrder), tt.cfg), cmpopts.EquateEmpty(), cmpopts.IgnoreUnexported(file.LocationData{})); d != "" { + t.Errorf("unexpected package (-want +got):\n%s", d) + } + }) + } +} + +func Test_toLocationModel(t *testing.T) { + tests := []struct { + name string + locations file.LocationSet + layers []string + want []file.Location + }{ + { + name: "empty location set", + locations: file.NewLocationSet(), + layers: []string{"fsid-1"}, + want: []file.Location{}, + }, + { + name: "nil layer order map", + locations: file.NewLocationSet( + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-2", + }), + ), + layers: nil, // please don't panic! + want: []file.Location{ + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + AccessPath: "/a", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-2", + }, + AccessPath: "/b", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + }, + }, + { + name: "go case", + locations: file.NewLocationSet( + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-3", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-2", + }), + ), + layers: []string{ + "fsid-1", + "fsid-2", + "fsid-3", + }, + want: []file.Location{ + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + AccessPath: "/b", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-2", + }, + AccessPath: "/c", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-3", + }, + AccessPath: "/a", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + }, + }, + { + name: "same layer different paths", // prove we can sort by path irrespective of layer + locations: file.NewLocationSet( + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-1", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }), + ), + layers: []string{ + "fsid-1", + }, + want: []file.Location{ + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + AccessPath: "/a", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + AccessPath: "/b", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-1", + }, + AccessPath: "/c", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + }, + }, + { + name: "mixed layers and paths", + locations: file.NewLocationSet( + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/z", + FileSystemID: "fsid-3", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-2", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }), + file.NewLocationFromCoordinates(file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-2", + }), + ), + layers: []string{ + "fsid-1", + "fsid-2", + "fsid-3", + }, + want: []file.Location{ + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + AccessPath: "/b", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-2", + }, + AccessPath: "/a", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-2", + }, + AccessPath: "/c", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + { + LocationData: file.LocationData{ + Coordinates: file.Coordinates{ + RealPath: "/z", + FileSystemID: "fsid-3", + }, + AccessPath: "/z", + }, + LocationMetadata: file.LocationMetadata{Annotations: map[string]string{}}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := toLocationsModel(tt.locations, file.LocationSorter(tt.layers)) + if d := cmp.Diff(tt.want, got, cmpopts.IgnoreUnexported(file.LocationData{})); d != "" { + t.Errorf("toLocationsModel() mismatch (-want +got):\n%s", d) + } + }) + } +} + +func Test_sortFiles(t *testing.T) { + tests := []struct { + name string + files []model.File + layers []string + want []model.File + }{ + { + name: "empty files slice", + files: []model.File{}, + layers: []string{"fsid-1"}, + want: []model.File{}, + }, + { + name: "nil layer order map", + files: []model.File{ + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-2", + }, + }, + }, + layers: nil, + want: []model.File{ + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-2", + }, + }, + }, + }, + { + name: "layer ordering", + files: []model.File{ + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-3", + }, + }, + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-3", + Location: file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-2", + }, + }, + }, + layers: []string{ + "fsid-1", + "fsid-2", + "fsid-3", + }, + want: []model.File{ + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-3", + Location: file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-2", + }, + }, + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-3", + }, + }, + }, + }, + { + name: "same layer different paths", + files: []model.File{ + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-3", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + }, + }, + layers: []string{ + "fsid-1", + }, + want: []model.File{ + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-3", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/c", + FileSystemID: "fsid-1", + }, + }, + }, + }, + { + name: "stability test - preserve original order for equivalent items", + files: []model.File{ + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-3", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + }, + layers: []string{ + "fsid-1", + }, + want: []model.File{ + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + { + ID: "file-3", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-1", + }, + }, + }, + }, + { + name: "complex file metadata doesn't affect sorting", + files: []model.File{ + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-2", + }, + Metadata: &model.FileMetadataEntry{ + Mode: 0644, + Type: "file", + UserID: 1000, + GroupID: 1000, + MIMEType: "text/plain", + Size: 100, + }, + Contents: "content1", + Digests: []file.Digest{ + { + Algorithm: "sha256", + Value: "abc123", + }, + }, + }, + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + Metadata: &model.FileMetadataEntry{ + Mode: 0755, + Type: "directory", + UserID: 0, + GroupID: 0, + MIMEType: "application/directory", + Size: 4096, + }, + }, + }, + layers: []string{ + "fsid-1", + "fsid-2", + }, + want: []model.File{ + { + ID: "file-2", + Location: file.Coordinates{ + RealPath: "/b", + FileSystemID: "fsid-1", + }, + Metadata: &model.FileMetadataEntry{ + Mode: 0755, + Type: "directory", + UserID: 0, + GroupID: 0, + MIMEType: "application/directory", + Size: 4096, + }, + }, + { + ID: "file-1", + Location: file.Coordinates{ + RealPath: "/a", + FileSystemID: "fsid-2", + }, + Metadata: &model.FileMetadataEntry{ + Mode: 0644, + Type: "file", + UserID: 1000, + GroupID: 1000, + MIMEType: "text/plain", + Size: 100, + }, + Contents: "content1", + Digests: []file.Digest{ + { + Algorithm: "sha256", + Value: "abc123", + }, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + files := make([]model.File, len(tt.files)) + copy(files, tt.files) + + sortFiles(files, file.CoordinatesSorter(tt.layers)) + + if d := cmp.Diff(tt.want, files); d != "" { + t.Errorf("sortFiles() mismatch (-want +got):\n%s", d) + } + }) + } +} diff --git a/syft/internal/fileresolver/container_image_squash_test.go b/syft/internal/fileresolver/container_image_squash_test.go index 7525f8947..4c1df13a6 100644 --- a/syft/internal/fileresolver/container_image_squash_test.go +++ b/syft/internal/fileresolver/container_image_squash_test.go @@ -3,7 +3,9 @@ package fileresolver import ( "context" "io" + "slices" "sort" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -516,8 +518,8 @@ func compareLocations(t *testing.T, expected, actual []file.Location) { ignoreMetadata := cmpopts.IgnoreFields(file.LocationMetadata{}, "Annotations") ignoreFS := cmpopts.IgnoreFields(file.Coordinates{}, "FileSystemID") - sort.Sort(file.Locations(expected)) - sort.Sort(file.Locations(actual)) + slices.SortFunc(expected, locationSorter) + slices.SortFunc(actual, locationSorter) if d := cmp.Diff(expected, actual, ignoreUnexported, @@ -531,6 +533,16 @@ func compareLocations(t *testing.T, expected, actual []file.Location) { } +// locationSorter always sorts only by path information since test fixtures here only have filesystem IDs +// for one side of the comparison (expected) and not the other (actual). +func locationSorter(a, b file.Location) int { + if a.AccessPath != b.AccessPath { + return strings.Compare(a.AccessPath, b.AccessPath) + } + + return strings.Compare(a.RealPath, b.RealPath) +} + func TestSquashResolver_AllLocations(t *testing.T) { img := imagetest.GetFixtureImage(t, "docker-archive", "image-files-deleted") diff --git a/syft/pkg/license_set.go b/syft/pkg/license_set.go index a86d02829..a577cb3ff 100644 --- a/syft/pkg/license_set.go +++ b/syft/pkg/license_set.go @@ -64,15 +64,38 @@ func (s *LicenseSet) Add(licenses ...License) { } } -func (s LicenseSet) ToSlice() []License { +func (s LicenseSet) ToSlice(sorters ...func(a, b License) int) []License { + licenses := s.ToUnorderedSlice() + + var sorted bool + for _, sorter := range sorters { + if sorter == nil { + continue + } + sort.Slice(licenses, func(i, j int) bool { + return sorter(licenses[i], licenses[j]) < 0 + }) + sorted = true + break + } + + if !sorted { + sort.Sort(Licenses(licenses)) + } + + return licenses +} + +func (s LicenseSet) ToUnorderedSlice() []License { if s.set == nil { return nil } - var licenses []License + licenses := make([]License, len(s.set)) + idx := 0 for _, v := range s.set { - licenses = append(licenses, v) + licenses[idx] = v + idx++ } - sort.Sort(Licenses(licenses)) return licenses } diff --git a/syft/pkg/license_set_test.go b/syft/pkg/license_set_test.go index cecaa6b9c..00c9a1f27 100644 --- a/syft/pkg/license_set_test.go +++ b/syft/pkg/license_set_test.go @@ -42,7 +42,7 @@ func TestLicenseSet_Add(t *testing.T) { licenses: []License{ NewLicense(""), }, - want: nil, + want: []License{}, }, { name: "keep multiple licenses sorted",