FWI-5471 - Fix CI image owner matching and add multiple owners support (

#862)

* Add image deduplication logic

* bump changelog and version

* fix tests

* fix slice mutation

plugins/ci/CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 5.3.4
+* Fix image owners matching logic
+
 ## 5.3.3
 * Trim spaces from masterBranch before using it
 

plugins/ci/pkg/ci/ci.go

@@ -198,7 +198,36 @@ func (ci *CIScan) getAllResources() ([]trivymodels.Image, []models.Resource, err
 		errors = multierror.Append(errors, fmt.Errorf("error walking directory %s: %v", ci.configFolder, err))
 		return nil, nil, errors
 	}
-	return images, resources, errors.ErrorOrNil()
+
+	// multiple images with the same name may belong to different owners, so we need to deduplicate them
+	dedupedImages := dedupImages(images)
+
+	return dedupedImages, resources, errors.ErrorOrNil()
+}
+
+func dedupImages(images []trivymodels.Image) []trivymodels.Image {
+	imageOwnersMap := map[string][]trivymodels.Resource{}
+	for _, img := range images {
+		if _, ok := imageOwnersMap[img.Name]; !ok {
+			imageOwnersMap[img.Name] = []trivymodels.Resource{} // initialize
+		}
+		imageOwnersMap[img.Name] = append(imageOwnersMap[img.Name], img.Owners...)
+	}
+
+	imagesMap := lo.KeyBy(images, func(img trivymodels.Image) string { return img.Name })
+
+	dedupedImages := []trivymodels.Image{}
+	for k, i := range imagesMap {
+		dedupedImages = append(dedupedImages, trivymodels.Image{
+			ID:                 i.ID,
+			PullRef:            i.PullRef,
+			Name:               i.Name,
+			Owners:             imageOwnersMap[k],
+			RecommendationOnly: i.RecommendationOnly,
+		})
+	}
+
+	return dedupedImages
 }
 
 func (ci *CIScan) getDisplayFilenameAndHelmName(path string) (string, string, error) {

plugins/ci/pkg/ci/ci_test.go

@@ -88,7 +88,7 @@ func TestGetAllResources(t *testing.T) {
   line 5: mapping key "kind" already defined at line 4
 
 `)
-	assert.Len(t, images, 3, "even though there are errors, we should still get the images")
+	assert.Len(t, images, 2, "even though there are errors, we should still get the images")
 	assert.Len(t, resources, 7, "even though there are errors, we should still get the resources")
 }
 

plugins/ci/pkg/ci/trivy.go

@@ -25,12 +25,12 @@ import (
 
 func (ci *CIScan) GetTrivyReport(dockerImages []trivymodels.DockerImage, manifestImages []trivymodels.Image) (report *models.ReportInfo, errs error) {
 	allErrs := new(multierror.Error)
-	err := updatePullRef(ci.config.Images.FolderName, dockerImages, manifestImages)
+	dockerImages, manifestImages, err := updatePullRef(ci.config.Images.FolderName, dockerImages, manifestImages)
 	if err != nil {
 		return nil, err
 	}
 
-	filenameToImageName, err := downloadMissingImages(ci.config.Images.FolderName, dockerImages, manifestImages, ci.config.Options.RegistryCredentials)
+	filenameToImageName, dockerImages, manifestImages, err := downloadMissingImages(ci.config.Images.FolderName, downloadImageViaSkopeo, dockerImages, manifestImages, ci.config.Options.RegistryCredentials)
 	if err != nil {
 		allErrs = multierror.Append(allErrs, err)
 	}
@@ -89,32 +89,39 @@ func walkImages(folderPath string, cb imageCallback) error {
 }
 
 // updatePullRef looks through image tarballs and mark which ones are already there, by setting image.PullRef
-func updatePullRef(folderPath string, dockerImages []trivymodels.DockerImage, manifestImages []trivymodels.Image) error {
+func updatePullRef(folderPath string, dockerImages []trivymodels.DockerImage, manifestImages []trivymodels.Image) ([]trivymodels.DockerImage, []trivymodels.Image, error) {
 	logrus.Infof("Looking through images in %s", folderPath)
-	return walkImages(folderPath, func(filename string, sha string, repoTags []string) {
-		for _, image := range manifestImages {
+	err := walkImages(folderPath, func(filename string, sha string, repoTags []string) {
+		for i := range manifestImages {
+			image := &manifestImages[i]
 			if slices.Contains(repoTags, image.Name) {
 				// already downloaded
 				logrus.Infof("image (manifest) %s already downloaded", image.Name)
 				image.PullRef = filename
 			}
 		}
 
-		for _, image := range dockerImages {
+		for i := range dockerImages {
+			image := &dockerImages[i]
 			if slices.Contains(repoTags, image.Name) {
 				// already downloaded
 				logrus.Infof("image (docker) %s already downloaded", image.Name)
 				image.PullRef = filename
 			}
 		}
 	})
+	if err != nil {
+		return nil, nil, err
+	}
+	return dockerImages, manifestImages, nil
 }
 
-func downloadMissingImages(folderPath string, dockerImages []trivymodels.DockerImage, manifestImages []trivymodels.Image, registryCredentials models.RegistryCredentials) (map[string]string, error) {
+func downloadMissingImages(folderPath string, imageDownloaderFunc ImageDownloaderFunc, dockerImages []trivymodels.DockerImage, manifestImages []trivymodels.Image, registryCredentials models.RegistryCredentials) (map[string]string, []trivymodels.DockerImage, []trivymodels.Image, error) {
 	allErrs := new(multierror.Error)
 	refLookup := map[string]string{} // postgres:15.1-bullseye -> postgres_15_1_bullseye
 	// Download missing images
-	for _, image := range manifestImages {
+	for i := range manifestImages {
+		image := &manifestImages[i]
 		if image.PullRef != "" {
 			continue
 		}
@@ -123,7 +130,7 @@ func downloadMissingImages(folderPath string, dockerImages []trivymodels.DockerI
 			continue
 		}
 		rc := registryCredentials.FindCredentialForImage(image.Name)
-		output, err := downloadImageViaSkopeo(commands.ExecWithMessage, folderPath, image.Name, rc)
+		output, err := imageDownloaderFunc(commands.ExecWithMessage, folderPath, image.Name, rc)
 		if err != nil {
 			allErrs = multierror.Append(allErrs, fmt.Errorf("%v: %s", err, output))
 		} else {
@@ -132,7 +139,8 @@ func downloadMissingImages(folderPath string, dockerImages []trivymodels.DockerI
 		}
 	}
 
-	for _, image := range dockerImages {
+	for i := range dockerImages {
+		image := &dockerImages[i]
 		if image.PullRef != "" {
 			continue
 		}
@@ -141,7 +149,7 @@ func downloadMissingImages(folderPath string, dockerImages []trivymodels.DockerI
 			continue
 		}
 		rc := registryCredentials.FindCredentialForImage(image.Name)
-		output, err := downloadImageViaSkopeo(commands.ExecWithMessage, folderPath, image.Name, rc)
+		output, err := imageDownloaderFunc(commands.ExecWithMessage, folderPath, image.Name, rc)
 		if err != nil {
 			allErrs = multierror.Append(allErrs, fmt.Errorf("%v: %s", err, output))
 		} else {
@@ -150,7 +158,7 @@ func downloadMissingImages(folderPath string, dockerImages []trivymodels.DockerI
 		}
 	}
 	if len(allErrs.Errors) > 0 {
-		return ciutil.ReverseMap(refLookup), models.ScanErrorsReportResult{
+		return ciutil.ReverseMap(refLookup), dockerImages, manifestImages, models.ScanErrorsReportResult{
 			ErrorMessage: allErrs.Error(), // keep multiple errors combined into a single error / action item
 			ErrorContext: "downloading missing images to be scanned by trivy",
 			Kind:         "InternalOperation",
@@ -159,9 +167,12 @@ func downloadMissingImages(folderPath string, dockerImages []trivymodels.DockerI
 			Filename:     filepath.Clean(folderPath), // clean() removes potential trailing slash
 		}
 	}
-	return ciutil.ReverseMap(refLookup), nil // postgres_15_1_bullseye -> postgres:15.1-bullseye
+	return ciutil.ReverseMap(refLookup), dockerImages, manifestImages, nil // postgres_15_1_bullseye -> postgres:15.1-bullseye
 }
 
+// ImageDownloaderFunc - downloads an image and returns the output and error
+type ImageDownloaderFunc = func(cmdExecutor cmdExecutor, folderPath, imageName string, rc *models.RegistryCredential) (string, error)
+
 func downloadImageViaSkopeo(cmdExecutor cmdExecutor, folderPath, imageName string, rc *models.RegistryCredential) (string, error) {
 	logrus.Infof("Downloading missing image %s", imageName)
 	dockerURL := "docker://" + imageName

plugins/ci/pkg/ci/trivy_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/fairwindsops/insights-plugins/plugins/ci/pkg/models"
+	trivymodels "github.com/fairwindsops/insights-plugins/plugins/trivy/pkg/models"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -57,3 +58,41 @@ func TestDownloadImageViaSkopeo(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "[skopeo copy --src-creds my-username:my-password random args docker://postgres:15.1-bullseye docker-archive:./postgres_15_1_bullseye]", cmd)
 }
+
+func TestDownloadMissingImages(t *testing.T) {
+	mockDownloaderFn := func(cmdExecutor cmdExecutor, folderPath, imageName string, rc *models.RegistryCredential) (string, error) {
+		return "", nil // noop
+	}
+	rc := models.RegistryCredentials{}
+
+	dockerImages := []trivymodels.DockerImage{{Name: "postgres:15.1-bullseye", PullRef: "postgres_15_1_bullseye"}, {Name: "nginx:1.23.2-alpine", PullRef: "nginx_1_23_2_alpine"}}
+	manifestImages := []trivymodels.Image{
+		{
+			Name:   "postgres:15.1-bullseye",
+			Owners: []trivymodels.Resource{{Name: "postgres", Kind: "Deployment", Namespace: "default", Container: "postgres"}},
+		},
+		{
+			Name:   "nginx:1.23.2-alpine",
+			Owners: []trivymodels.Resource{{Name: "nginx", Kind: "Deployment", Namespace: "default", Container: "nginx"}},
+		},
+		{
+			Name:   "alpine:1.24.2",
+			Owners: []trivymodels.Resource{{Name: "nginx", Kind: "Deployment", Namespace: "default", Container: "nginx"}},
+		},
+	}
+
+	refToImageName, dockerImages, manifestImages, err := downloadMissingImages("_/images", mockDownloaderFn, dockerImages, manifestImages, rc)
+	assert.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		"postgres_15_1_bullseye": "postgres:15.1-bullseye",
+		"nginx_1_23_2_alpine":    "nginx:1.23.2-alpine",
+		"alpine_1_24_2":          "alpine:1.24.2",
+	}, refToImageName)
+
+	assert.Len(t, dockerImages, 2)
+	assert.Len(t, manifestImages, 3)
+
+	for _, mi := range manifestImages {
+		assert.NotEmpty(t, mi.PullRef)
+	}
+}

plugins/ci/version.txt

@@ -1 +1 @@
-5.3.3
+5.3.4