cleanup: refactored logic to avoid duplicate scans

Signed-off-by: Christian Kotzbauer <[email protected]>

internal/daemon/daemon.go

@@ -51,33 +51,24 @@ func (c *CronService) runBackgroundService() {
 		t.Initialize()
 	}
 
-	client := kubernetes.NewClient()
-	namespaces := client.ListNamespaces(viper.GetString(internal.ConfigKeyNamespaceLabelSelector))
+	k8s := kubernetes.NewClient()
+	namespaces := k8s.ListNamespaces(viper.GetString(internal.ConfigKeyNamespaceLabelSelector))
 	logrus.Debugf("Discovered %v namespaces", len(namespaces))
+	containerImages, allImages := k8s.LoadImageInfos(namespaces, viper.GetString(internal.ConfigKeyPodLabelSelector))
 
 	sy := syft.New(viper.GetString(internal.ConfigKeyGitWorkingTree), viper.GetString(internal.ConfigKeyGitPath), format)
-	allImages := []string{}
-
-	for _, ns := range namespaces {
-		pods := client.ListPods(ns.Name, viper.GetString(internal.ConfigKeyPodLabelSelector))
-		logrus.Debugf("Discovered %v pods in namespace %v", len(pods), ns.Name)
-
-		for _, pod := range pods {
-			filteredDigests, allPodImages := client.GetContainerDigests(pod)
-			allImages = append(allImages, allPodImages...)
-
-			for _, d := range filteredDigests {
-				// TODO: Avoid duplicate scans of the same image in different pods.
-				_, err := sy.ExecuteSyft(d)
-				// Error is already handled from syft module.
-				if err == nil {
-					client.UpdatePodAnnotation(pod)
-				}
+
+	for _, image := range containerImages {
+		_, err := sy.ExecuteSyft(image)
+		// Error is already handled from syft module.
+		if err == nil {
+			for _, pod := range image.Pods {
+				k8s.UpdatePodAnnotation(pod)
 			}
 		}
 
 		for _, t := range c.targets {
-			t.ProcessSboms(ns.Name)
+			t.ProcessSboms(image.ImageID)
 		}
 	}
 

internal/kubernetes/kubernetes.go

@@ -15,9 +15,10 @@ import (
 	"github.com/ckotzbauer/sbom-operator/internal"
 )
 
-type ImageDigest struct {
-	Digest string
-	Auth   []byte
+type ContainerImage struct {
+	ImageID string
+	Auth    []byte
+	Pods    []corev1.Pod
 }
 
 type KubeClient struct {
@@ -68,7 +69,7 @@ func (client *KubeClient) ListNamespaces(labelSelector string) []corev1.Namespac
 	return list.Items
 }
 
-func (client *KubeClient) ListPods(namespace, labelSelector string) []corev1.Pod {
+func (client *KubeClient) listPods(namespace, labelSelector string) []corev1.Pod {
 	list, err := client.Client.CoreV1().Pods(namespace).List(context.Background(), prepareLabelSelector(labelSelector))
 
 	if err != nil {
@@ -79,6 +80,50 @@ func (client *KubeClient) ListPods(namespace, labelSelector string) []corev1.Pod
 	return list.Items
 }
 
+func (client *KubeClient) LoadImageInfos(namespaces []corev1.Namespace, podLabelSelector string) (map[string]ContainerImage, []string) {
+	images := map[string]ContainerImage{}
+	allImages := []string{}
+
+	for _, ns := range namespaces {
+		pods := client.listPods(ns.Name, podLabelSelector)
+
+		for _, pod := range pods {
+			annotations := pod.Annotations
+			statuses := []corev1.ContainerStatus{}
+			statuses = append(statuses, pod.Status.ContainerStatuses...)
+			statuses = append(statuses, pod.Status.InitContainerStatuses...)
+			statuses = append(statuses, pod.Status.EphemeralContainerStatuses...)
+
+			pullSecrets, err := client.loadSecrets(pod.Namespace, pod.Spec.ImagePullSecrets)
+
+			if err != nil {
+				logrus.WithError(err).Errorf("PullSecrets could not be retrieved for pod %s/%s", ns.Name, pod.Name)
+				continue
+			}
+
+			for _, c := range statuses {
+				if c.ImageID != "" {
+					if !client.hasAnnotation(annotations, c) {
+						img, ok := images[c.ImageID]
+						if !ok {
+							img = ContainerImage{ImageID: c.ImageID, Auth: pullSecrets, Pods: []corev1.Pod{}}
+						}
+
+						img.Pods = append(img.Pods, pod)
+						images[c.ImageID] = img
+					} else {
+						logrus.Debugf("Skip image %s", c.ImageID)
+					}
+
+					allImages = append(allImages, c.ImageID)
+				}
+			}
+		}
+	}
+
+	return images, allImages
+}
+
 func (client *KubeClient) UpdatePodAnnotation(pod corev1.Pod) {
 	newPod, err := client.Client.CoreV1().Pods(pod.Namespace).Get(context.Background(), pod.Name, meta.GetOptions{})
 
@@ -112,38 +157,6 @@ func (client *KubeClient) UpdatePodAnnotation(pod corev1.Pod) {
 	}
 }
 
-func (client *KubeClient) GetContainerDigests(pod corev1.Pod) ([]ImageDigest, []string) {
-	digests := []ImageDigest{}
-	allImages := []string{}
-
-	annotations := pod.Annotations
-	pullSecrets, err := client.loadSecrets(pod.Namespace, pod.Spec.ImagePullSecrets)
-
-	if err != nil {
-		logrus.WithError(err).Error("PullSecrets could not be retrieved!")
-		return []ImageDigest{}, []string{}
-	}
-
-	statuses := []corev1.ContainerStatus{}
-	statuses = append(statuses, pod.Status.ContainerStatuses...)
-	statuses = append(statuses, pod.Status.InitContainerStatuses...)
-	statuses = append(statuses, pod.Status.EphemeralContainerStatuses...)
-
-	for _, c := range statuses {
-		if c.ImageID != "" {
-			if !client.hasAnnotation(annotations, c) {
-				digests = append(digests, ImageDigest{Digest: c.ImageID, Auth: pullSecrets})
-			} else {
-				logrus.Debugf("Skip image %s", c.ImageID)
-			}
-
-			allImages = append(allImages, c.ImageID)
-		}
-	}
-
-	return removeDuplicateValues(digests), allImages
-}
-
 func (client *KubeClient) hasAnnotation(annotations map[string]string, status corev1.ContainerStatus) bool {
 	if annotations == nil || client.ignoreAnnotations {
 		return false
@@ -156,19 +169,6 @@ func (client *KubeClient) hasAnnotation(annotations map[string]string, status co
 	return false
 }
 
-func removeDuplicateValues(slice []ImageDigest) []ImageDigest {
-	keys := make(map[string]bool)
-	list := []ImageDigest{}
-
-	for _, entry := range slice {
-		if _, value := keys[entry.Digest]; !value {
-			keys[entry.Digest] = true
-			list = append(list, entry)
-		}
-	}
-	return list
-}
-
 func (client *KubeClient) loadSecrets(namespace string, secrets []corev1.LocalObjectReference) ([]byte, error) {
 	// TODO: Support all secrets which are referenced as imagePullSecrets instead of only the first one.
 	for _, s := range secrets {

internal/registry/registry.go

@@ -17,7 +17,7 @@ import (
 	parser "github.com/novln/docker-parser"
 )
 
-func SaveImage(imagePath, workDir string, image kubernetes.ImageDigest) error {
+func SaveImage(imagePath, workDir string, image kubernetes.ContainerImage) error {
 	imageMap := map[string]v1.Image{}
 
 	o := crane.GetOptions()
@@ -28,7 +28,7 @@ func SaveImage(imagePath, workDir string, image kubernetes.ImageDigest) error {
 			return err
 		}
 
-		fullRef, err := parser.Parse(image.Digest)
+		fullRef, err := parser.Parse(image.ImageID)
 		if err != nil {
 			return err
 		}
@@ -64,10 +64,10 @@ func SaveImage(imagePath, workDir string, image kubernetes.ImageDigest) error {
 		}
 	}
 
-	ref, err := name.ParseReference(image.Digest, o.Name...)
+	ref, err := name.ParseReference(image.ImageID, o.Name...)
 
 	if err != nil {
-		return fmt.Errorf("parsing reference %q: %w", image, err)
+		return fmt.Errorf("parsing reference %q: %w", image.ImageID, err)
 	}
 
 	rmt, err := remote.Get(ref, o.Remote...)
@@ -80,7 +80,7 @@ func SaveImage(imagePath, workDir string, image kubernetes.ImageDigest) error {
 		return err
 	}
 
-	imageMap[image.Digest] = img
+	imageMap[image.ImageID] = img
 
 	if err := crane.MultiSave(imageMap, imagePath); err != nil {
 		return fmt.Errorf("saving tarball %s: %w", imagePath, err)

internal/registry/registry_suite_test.go

@@ -24,7 +24,7 @@ func testRegistry(name, image string) {
 	Expect(err).To(BeNil())
 
 	os.Mkdir("/tmp/"+name, 0777)
-	err = registry.SaveImage("/tmp/"+name+"/image.tar.gz", "/tmp/"+name, kubernetes.ImageDigest{Digest: image, Auth: []byte(decoded)})
+	err = registry.SaveImage("/tmp/"+name+"/image.tar.gz", "/tmp/"+name, kubernetes.ContainerImage{ImageID: image, Auth: []byte(decoded)})
 
 	if err == nil {
 		stat, _ := os.Stat("/tmp/" + name + "/image.tar.gz")

internal/syft/syft.go

@@ -34,12 +34,12 @@ func New(gitWorkingTree, gitPath, sbomFormat string) Syft {
 	}
 }
 
-func (s *Syft) ExecuteSyft(img kubernetes.ImageDigest) (string, error) {
+func (s *Syft) ExecuteSyft(img kubernetes.ContainerImage) (string, error) {
 	fileName := GetFileName(s.SbomFormat)
-	filePath := strings.ReplaceAll(img.Digest, "@", "/")
+	filePath := strings.ReplaceAll(img.ImageID, "@", "/")
 	filePath = strings.ReplaceAll(path.Join(s.GitWorkingTree, s.GitPath, filePath, fileName), ":", "_")
 
-	logrus.Debugf("Processing image %s", img.Digest)
+	logrus.Infof("Processing image %s", img.ImageID)
 
 	workDir := "/tmp/" + util.RandStringBytes(10)
 	imagePath := workDir + "/image.tar.gz"

internal/target/git_target.go

@@ -76,8 +76,8 @@ func (g *GitTarget) Initialize() {
 		viper.GetString(internal.ConfigKeyGitBranch))
 }
 
-func (g *GitTarget) ProcessSboms(namespace string) {
-	g.gitAccount.CommitAll(g.workingTree, fmt.Sprintf("Created new SBOMs for pods in namespace %s", namespace))
+func (g *GitTarget) ProcessSboms(imageID string) {
+	g.gitAccount.CommitAll(g.workingTree, fmt.Sprintf("Created new SBOM for image %s", imageID))
 }
 
 func (g *GitTarget) Cleanup(allImages []string) {

internal/target/target.go

@@ -3,6 +3,6 @@ package target
 type Target interface {
 	Initialize()
 	ValidateConfig() error
-	ProcessSboms(namespace string)
+	ProcessSboms(imageID string)
 	Cleanup(allImages []string)
 }