Deduplicate packages across multiple container image layers (#930)

test/cli/packages_cmd_test.go

@@ -8,6 +8,7 @@ import (
 )
 
 func TestPackagesCmdFlags(t *testing.T) {
+	hiddenPackagesImage := "docker-archive:" + getFixtureImage(t, "image-hidden-packages")
 	coverageImage := "docker-archive:" + getFixtureImage(t, "image-pkg-coverage")
 	//badBinariesImage := "docker-archive:" + getFixtureImage(t, "image-bad-binaries")
 	tmp := t.TempDir() + "/"
@@ -100,21 +101,34 @@ func TestPackagesCmdFlags(t *testing.T) {
 			},
 		},
 		{
-			name: "all-layers-scope-flag",
-			args: []string{"packages", "-o", "json", "-s", "all-layers", coverageImage},
+			name: "squashed-scope-flag-hidden-packages",
+			args: []string{"packages", "-o", "json", "-s", "squashed", hiddenPackagesImage},
 			assertions: []traitAssertion{
-				assertPackageCount(22),
+				assertPackageCount(162),
+				assertNotInOutput("vsftpd"), // hidden package
+				assertSuccessfulReturnCode,
+			},
+		},
+		{
+			name: "all-layers-scope-flag",
+			args: []string{"packages", "-o", "json", "-s", "all-layers", hiddenPackagesImage},
+			assertions: []traitAssertion{
+				assertPackageCount(163), // packages are now deduplicated for this case
+				assertInOutput("all-layers"),
+				assertInOutput("vsftpd"), // hidden package
 				assertSuccessfulReturnCode,
 			},
 		},
 		{
 			name: "all-layers-scope-flag-by-env",
-			args: []string{"packages", "-o", "json", coverageImage},
+			args: []string{"packages", "-o", "json", hiddenPackagesImage},
 			env: map[string]string{
 				"SYFT_PACKAGE_CATALOGER_SCOPE": "all-layers",
 			},
 			assertions: []traitAssertion{
-				assertPackageCount(22),
+				assertPackageCount(163), // packages are now deduplicated for this case
+				assertInOutput("all-layers"),
+				assertInOutput("vsftpd"), // hidden package
 				assertSuccessfulReturnCode,
 			},
 		},

test/cli/test-fixtures/image-hidden-packages/Dockerfile

@@ -0,0 +1,4 @@
+FROM centos:7.9.2009
+# all-layers scope should pickup on vsftpd
+RUN yum install -y vsftpd
+RUN yum remove -y vsftpd

test/integration/catalog_packages_test.go

@@ -54,7 +54,7 @@ func BenchmarkImagePackageCatalogers(b *testing.B) {
 }
 
 func TestPkgCoverageImage(t *testing.T) {
-	sbom, _ := catalogFixtureImage(t, "image-pkg-coverage")
+	sbom, _ := catalogFixtureImage(t, "image-pkg-coverage", source.SquashedScope)
 
 	observedLanguages := internal.NewStringSet()
 	definedLanguages := internal.NewStringSet()

test/integration/distro_test.go

@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"github.com/anchore/syft/syft/source"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -9,7 +10,7 @@ import (
 )
 
 func TestDistroImage(t *testing.T) {
-	sbom, _ := catalogFixtureImage(t, "image-distro-id")
+	sbom, _ := catalogFixtureImage(t, "image-distro-id", source.SquashedScope)
 
 	expected := &linux.Release{
 		PrettyName: "BusyBox v1.31.1",

test/integration/encode_decode_cycle_test.go

@@ -2,12 +2,14 @@ package integration
 
 import (
 	"bytes"
-	"regexp"
-	"testing"
-
+	"fmt"
 	"github.com/anchore/syft/internal/formats/cyclonedxjson"
 	"github.com/anchore/syft/internal/formats/cyclonedxxml"
 	"github.com/anchore/syft/internal/formats/syftjson"
+	"github.com/anchore/syft/syft/source"
+	"regexp"
+	"testing"
+
 	"github.com/anchore/syft/syft/sbom"
 	"github.com/stretchr/testify/require"
 
@@ -51,12 +53,12 @@ func TestEncodeDecodeEncodeCycleComparison(t *testing.T) {
 			},
 		},
 	}
-	for _, test := range tests {
-		t.Run(string(test.formatOption), func(t *testing.T) {
 
-			// use second image for relationships
-			for _, image := range []string{"image-pkg-coverage", "image-owning-package"} {
-				originalSBOM, _ := catalogFixtureImage(t, image)
+	for _, test := range tests {
+		// use second image for relationships
+		for _, image := range []string{"image-pkg-coverage", "image-owning-package"} {
+			t.Run(fmt.Sprintf("%s/%s", test.formatOption, image), func(t *testing.T) {
+				originalSBOM, _ := catalogFixtureImage(t, image, source.SquashedScope)
 
 				format := syft.FormatByID(test.formatOption)
 				require.NotNil(t, format)
@@ -87,7 +89,7 @@ func TestEncodeDecodeEncodeCycleComparison(t *testing.T) {
 						t.Errorf("diff: %s", dmp.DiffPrettyText(diffs))
 					}
 				}
-			}
-		})
+			})
+		}
 	}
 }

test/integration/node_packages_test.go

@@ -14,7 +14,7 @@ func TestNpmPackageLockDirectory(t *testing.T) {
 	foundPackages := internal.NewStringSet()
 
 	for actualPkg := range sbom.Artifacts.PackageCatalog.Enumerate(pkg.NpmPkg) {
-		for _, actualLocation := range actualPkg.Locations {
+		for _, actualLocation := range actualPkg.Locations.ToSlice() {
 			if strings.Contains(actualLocation.RealPath, "node_modules") {
 				t.Errorf("found packages from package-lock.json in node_modules: %s", actualLocation)
 			}
@@ -35,7 +35,7 @@ func TestYarnPackageLockDirectory(t *testing.T) {
 	foundPackages := internal.NewStringSet()
 
 	for actualPkg := range sbom.Artifacts.PackageCatalog.Enumerate(pkg.NpmPkg) {
-		for _, actualLocation := range actualPkg.Locations {
+		for _, actualLocation := range actualPkg.Locations.ToSlice() {
 			if strings.Contains(actualLocation.RealPath, "node_modules") {
 				t.Errorf("found packages from yarn.lock in node_modules: %s", actualLocation)
 			}

test/integration/package_deduplication_test.go

@@ -0,0 +1,84 @@
+package integration
+
+import (
+	"fmt"
+	"github.com/anchore/syft/syft/source"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+func TestPackageDeduplication(t *testing.T) {
+	tests := []struct {
+		scope         source.Scope
+		packageCount  int
+		instanceCount map[string]int
+		locationCount map[string]int
+	}{
+		{
+			scope:        source.AllLayersScope,
+			packageCount: 172, // without deduplication this would be 618
+			instanceCount: map[string]int{
+				"basesystem": 1,
+				"wget":       1,
+				"curl":       2, // upgraded in the image
+				"vsftpd":     1,
+				"httpd":      1,
+			},
+			locationCount: map[string]int{
+				"basesystem-10.0-7.el7.centos": 4,
+				"curl-7.29.0-59.el7":           1, // from base image
+				"curl-7.29.0-59.el7_9.1":       3, // upgrade
+				"wget-1.14-18.el7_6.1":         3,
+				"vsftpd-3.0.2-29.el7_9":        2,
+				"httpd-2.4.6-97.el7.centos.5":  1,
+			},
+		},
+		{
+			scope:        source.SquashedScope,
+			packageCount: 170,
+			instanceCount: map[string]int{
+				"basesystem": 1,
+				"wget":       1,
+				"curl":       1, // upgraded, but the most recent
+				"vsftpd":     1,
+				"httpd":      1,
+			},
+			locationCount: map[string]int{
+				"basesystem-10.0-7.el7.centos": 1,
+				"curl-7.29.0-59.el7_9.1":       1, // upgrade
+				"wget-1.14-18.el7_6.1":         1,
+				"vsftpd-3.0.2-29.el7_9":        1,
+				"httpd-2.4.6-97.el7.centos.5":  1,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(string(tt.scope), func(t *testing.T) {
+			sbom, _ := catalogFixtureImage(t, "image-vertical-package-dups", tt.scope)
+
+			assert.Equal(t, tt.packageCount, sbom.Artifacts.PackageCatalog.PackageCount())
+			for name, expectedInstanceCount := range tt.instanceCount {
+				pkgs := sbom.Artifacts.PackageCatalog.PackagesByName(name)
+
+				// with multiple packages with the same name, something is wrong (or this is the wrong fixture)
+				require.Len(t, pkgs, expectedInstanceCount)
+				for _, p := range pkgs {
+					nameVersion := fmt.Sprintf("%s-%s", name, p.Version)
+					expectedLocationCount, ok := tt.locationCount[nameVersion]
+					if !ok {
+						t.Fatalf("missing name-version: %s", nameVersion)
+					}
+
+					// we should see merged locations (assumption, there was 1 location for each package)
+					assert.Len(t, p.Locations.ToSlice(), expectedLocationCount)
+
+					// all paths should match
+					assert.Len(t, p.Locations.CoordinateSet().Paths(), 1)
+				}
+			}
+
+		})
+	}
+}

test/integration/package_ownership_relationship_test.go

@@ -3,6 +3,7 @@ package integration
 import (
 	"bytes"
 	"encoding/json"
+	"github.com/anchore/syft/syft/source"
 	"testing"
 
 	"github.com/anchore/syft/internal/formats/syftjson"
@@ -22,7 +23,7 @@ func TestPackageOwnershipRelationships(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.fixture, func(t *testing.T) {
-			sbom, _ := catalogFixtureImage(t, test.fixture)
+			sbom, _ := catalogFixtureImage(t, test.fixture, source.SquashedScope)
 
 			output := bytes.NewBufferString("")
 			err := syftjson.Format().Encode(output, sbom)

test/integration/regression_apk_scanner_buffer_size_test.go

@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"github.com/anchore/syft/syft/source"
 	"testing"
 
 	"github.com/anchore/syft/syft/pkg"
@@ -9,7 +10,7 @@ import (
 func TestRegression212ApkBufferSize(t *testing.T) {
 	// This is a regression test for issue #212 (https://github.com/anchore/syft/issues/212) in which the apk db could
 	// not be processed due to a scanner buffer that was too small
-	sbom, _ := catalogFixtureImage(t, "image-large-apk-data")
+	sbom, _ := catalogFixtureImage(t, "image-large-apk-data", source.SquashedScope)
 
 	expectedPkgs := 58
 	actualPkgs := 0

test/integration/regression_go_bin_scanner_arch_test.go

@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"github.com/anchore/syft/syft/source"
 	"strings"
 	"testing"
 
@@ -15,12 +16,12 @@ func TestRegressionGoArchDiscovery(t *testing.T) {
 	)
 	// This is a regression test to make sure the way we detect go binary packages
 	// stays consistent and reproducible as the tool chain evolves
-	sbom, _ := catalogFixtureImage(t, "image-go-bin-arch-coverage")
+	sbom, _ := catalogFixtureImage(t, "image-go-bin-arch-coverage", source.SquashedScope)
 
 	var actualELF, actualWIN, actualMACOS int
 
 	for p := range sbom.Artifacts.PackageCatalog.Enumerate(pkg.GoModulePkg) {
-		for _, l := range p.Locations {
+		for _, l := range p.Locations.ToSlice() {
 			switch {
 			case strings.Contains(l.RealPath, "elf"):
 				actualELF++

test/integration/regression_java_no_main_package_test.go

@@ -1,9 +1,10 @@
 package integration
 
 import (
+	"github.com/anchore/syft/syft/source"
 	"testing"
 )
 
 func TestRegressionJavaNoMainPackage(t *testing.T) { // Regression: https://github.com/anchore/syft/issues/252
-	catalogFixtureImage(t, "image-java-no-main-package")
+	catalogFixtureImage(t, "image-java-no-main-package", source.SquashedScope)
 }

test/integration/test-fixtures/image-vertical-package-dups/Dockerfile

@@ -0,0 +1,6 @@
+FROM centos:7.9.2009
+# modifying the RPM DB multiple times will result in duplicate packages when using all-layers (if there was no de-dup logic)
+# curl is tricky, it already exists in the image and is being upgraded
+RUN yum install -y wget curl
+RUN yum install -y vsftpd
+RUN yum install -y httpd

test/integration/utils_test.go

@@ -13,7 +13,7 @@ import (
 	"github.com/anchore/syft/syft/source"
 )
 
-func catalogFixtureImage(t *testing.T, fixtureImageName string) (sbom.SBOM, *source.Source) {
+func catalogFixtureImage(t *testing.T, fixtureImageName string, scope source.Scope) (sbom.SBOM, *source.Source) {
 	imagetest.GetFixtureImage(t, "docker-archive", fixtureImageName)
 	tarPath := imagetest.GetFixtureImageTarPath(t, fixtureImageName)
 	userInput := "docker-archive:" + tarPath
@@ -25,7 +25,7 @@ func catalogFixtureImage(t *testing.T, fixtureImageName string) (sbom.SBOM, *sou
 
 	// TODO: this would be better with functional options (after/during API refactor)
 	c := cataloger.DefaultConfig()
-	c.Search.Scope = source.SquashedScope
+	c.Search.Scope = scope
 	pkgCatalog, relationships, actualDistro, err := syft.CatalogPackages(theSource, c)
 	if err != nil {
 		t.Fatalf("failed to catalog image: %+v", err)