Merge pull request #41 from fabriziosestito/fix/handlers-existing-res…

…ource-logic

fix: improve the logic of handlers to cover scenarios where resources already exist

internal/handlers/create_catalog.go

@@ -18,6 +18,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	storagev1alpha1 "github.com/rancher/sbombastic/api/storage/v1alpha1"
@@ -75,7 +76,7 @@ func (h *CreateCatalogHandler) Handle(message messaging.Message) error {
 		return fmt.Errorf("cannot discover repositories: %w", err)
 	}
 
-	var imageNames []string
+	discoveredImageNames := sets.Set[string]{}
 	for _, repository := range repositories {
 		repoImages, err := h.discoverImages(ctx, registryClient, repository)
 		if err != nil {
@@ -86,38 +87,42 @@ func (h *CreateCatalogHandler) Handle(message messaging.Message) error {
 			)
 			continue
 		}
-		imageNames = append(imageNames, repoImages...)
+		discoveredImageNames.Insert(repoImages...)
 	}
 
-	for _, imageName := range imageNames {
-		ref, err := name.ParseReference(imageName)
+	existingImageList := &storagev1alpha1.ImageList{}
+	if err := h.k8sClient.List(ctx, existingImageList, client.InNamespace(registry.Namespace), client.MatchingLabels{"registry": registry.Name}); err != nil {
+		return fmt.Errorf("cannot list existing images: %w", err)
+	}
+	existingImageNames := sets.Set[string]{}
+	for _, existingImage := range existingImageList.Items {
+		existingImageNames.Insert(existingImage.Name)
+	}
+
+	for newImageName := range discoveredImageNames {
+		ref, err := name.ParseReference(newImageName)
 		if err != nil {
-			h.logger.Error(
-				"cannot parse image name",
-				zap.String("image", imageName),
-				zap.Error(err),
-			)
-			continue
+			return fmt.Errorf("cannot parse reference %s: %w", newImageName, err)
 		}
 
 		images, err := h.refToImages(registryClient, ref, registry.Name, registry.Namespace)
 		if err != nil {
-			h.logger.Error(
-				"cannot convert reference to Image",
-				zap.String("image", ref.Name()),
-				zap.Error(err),
-			)
-			continue
+			return fmt.Errorf("cannot get images for %s: %w", ref, err)
 		}
 		for _, image := range images {
-			// TODO: ignore creation of images that already exist
+			if existingImageNames.Has(image.Name) {
+				continue
+			}
+
 			if err := h.k8sClient.Create(ctx, &image); err != nil {
 				return fmt.Errorf("cannot create image %s: %w", image.Name, err)
 			}
 		}
 	}
 
-	// TODO: remove images that are not in the registry anymore
+	if err := h.deleteObsoleteImages(ctx, existingImageNames, discoveredImageNames, registry.Namespace); err != nil {
+		return fmt.Errorf("cannot delete obsolete images: %w", err)
+	}
 
 	return nil
 }
@@ -259,6 +264,28 @@ func (h *CreateCatalogHandler) transportFromRegistry(registry *v1alpha1.Registry
 	return transport
 }
 
+// deleteObsoleteImages deletes images that are not present in the discovered registry anymore.
+func (h *CreateCatalogHandler) deleteObsoleteImages(ctx context.Context, existingImageNames sets.Set[string], discoveredImageNames sets.Set[string], namespace string) error {
+	for existingImageName := range existingImageNames {
+		if discoveredImageNames.Has(existingImageName) {
+			continue
+		}
+
+		existingImage := storagev1alpha1.Image{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      existingImageName,
+				Namespace: namespace,
+			},
+		}
+
+		if err := h.k8sClient.Delete(ctx, &existingImage); err != nil {
+			return fmt.Errorf("cannot delete image %s: %w", existingImageName, err)
+		}
+	}
+
+	return nil
+}
+
 func imageDetailsToImage(ref name.Reference, details registryclient.ImageDetails, registryName, registryNamespace string) (storagev1alpha1.Image, error) {
 	imageLayers := []storagev1alpha1.ImageLayer{}
 

internal/handlers/create_catalog_test.go

@@ -22,6 +22,9 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/types"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	storagev1alpha1 "github.com/rancher/sbombastic/api/storage/v1alpha1"
@@ -205,6 +208,51 @@ func TestCreateCatalogHandler_DiscoverRepositories(t *testing.T) {
 	}
 }
 
+func TestCataloghandler_DeleteObsoleteImages(t *testing.T) {
+	scheme := runtime.NewScheme()
+	require.NoError(t, storagev1alpha1.AddToScheme(scheme))
+	existingImages := []runtime.Object{
+		&storagev1alpha1.Image{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "image-1",
+				Namespace: "default",
+			},
+		},
+		&storagev1alpha1.Image{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "image-2",
+				Namespace: "default",
+			},
+		},
+	}
+	k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(existingImages...).Build()
+
+	handler := &CreateCatalogHandler{
+		k8sClient: k8sClient,
+		logger:    zap.NewNop(),
+	}
+
+	ctx := context.Background()
+
+	existingImageNames := sets.New[string](
+		"image-1",
+		"image-2",
+	)
+	newImageNames := sets.New[string](
+		"image-1", // Image 2 is obsolete
+	)
+
+	err := handler.deleteObsoleteImages(ctx, existingImageNames, newImageNames, "default")
+	require.NoError(t, err)
+
+	var remainingImages storagev1alpha1.ImageList
+	err = k8sClient.List(ctx, &remainingImages, client.InNamespace("default"))
+	require.NoError(t, err)
+
+	require.Len(t, remainingImages.Items, 1)
+	assert.Equal(t, "image-1", remainingImages.Items[0].Name)
+}
+
 func TestImageDetailsToImage(t *testing.T) {
 	digest, err := cranev1.NewHash("sha256:f41b7d70c5779beba4a570ca861f788d480156321de2876ce479e072fb0246f1")
 	require.NoError(t, err)

internal/handlers/generate_sbom.go

@@ -9,6 +9,7 @@ import (
 	"go.uber.org/zap"
 
 	trivyCommands "github.com/aquasecurity/trivy/pkg/commands"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -105,7 +106,9 @@ func (h *GenerateSBOMHandler) Handle(message messaging.Message) error {
 		},
 	}
 	if err := h.k8sClient.Create(ctx, sbom); err != nil {
-		return fmt.Errorf("failed to create SBOM: %w", err)
+		if !apierrors.IsAlreadyExists(err) {
+			return fmt.Errorf("failed to create SBOM: %w", err)
+		}
 	}
 
 	return nil

internal/handlers/scan_sbom.go

@@ -12,6 +12,7 @@ import (
 
 	trivyCommands "github.com/aquasecurity/trivy/pkg/commands"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	storagev1alpha1 "github.com/rancher/sbombastic/api/storage/v1alpha1"
 	"github.com/rancher/sbombastic/internal/messaging"
@@ -117,14 +118,16 @@ func (h *ScanSBOMHandler) Handle(message messaging.Message) error {
 			Name:      sbom.Name,
 			Namespace: sbom.Namespace,
 		},
-		Spec: storagev1alpha1.VulnerabilityReportSpec{
+	}
+	_, err = controllerutil.CreateOrUpdate(ctx, h.k8sClient, vulnerabilityReport, func() error {
+		vulnerabilityReport.Spec = storagev1alpha1.VulnerabilityReportSpec{
 			ImageMetadata: sbom.GetImageMetadata(),
 			SARIF:         runtime.RawExtension{Raw: reportBytes},
-		},
-	}
-
-	if err := h.k8sClient.Create(ctx, vulnerabilityReport); err != nil {
-		return fmt.Errorf("failed to create vulnerability report: %w", err)
+		}
+		return nil
+	})
+	if err != nil {
+		return fmt.Errorf("failed to create or update vulnerability report: %w", err)
 	}
 
 	return nil