From 4c25f20a4cc6da8a6393747277412f90bd227c2b Mon Sep 17 00:00:00 2001
From: Melvin Hillsman <mhillsma@redhat.com>
Date: Thu, 28 May 2026 01:13:49 -0500
Subject: [PATCH] fix(resources): collect all deletion errors instead of
 bailing on first

DeleteManagedSecrets and DeleteManagedServices returned immediately on
the first non-NotFound deletion error, abandoning remaining resources.
Now they continue attempting all deletions and return an aggregated
error via errors.Join alongside the count of successful deletions.

Resolves #102

Signed-off-by: Melvin Hillsman <mhillsma@redhat.com>
---
 internal/resources/resources.go      | 13 ++--
 internal/resources/resources_test.go | 94 ++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 4 deletions(-)

diff --git a/internal/resources/resources.go b/internal/resources/resources.go
index ed0a3fa..1d5e459 100644
--- a/internal/resources/resources.go
+++ b/internal/resources/resources.go
@@ -5,6 +5,7 @@ package resources
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
@@ -83,16 +84,18 @@ func DeleteManagedSecrets(
 	}
 
 	deleted := 0
+	var errs []error
 	for i := range secretList.Items {
 		if err := c.Delete(ctx, &secretList.Items[i]); err != nil {
 			if apierrors.IsNotFound(err) {
 				continue
 			}
-			return deleted, fmt.Errorf("deleting secret %s: %w", secretList.Items[i].Name, err)
+			errs = append(errs, fmt.Errorf("deleting secret %s: %w", secretList.Items[i].Name, err))
+			continue
 		}
 		deleted++
 	}
-	return deleted, nil
+	return deleted, errors.Join(errs...)
 }
 
 // DeleteManagedServices lists and deletes services matching the given labels in
@@ -113,14 +116,16 @@ func DeleteManagedServices(
 	}
 
 	deleted := 0
+	var errs []error
 	for i := range svcList.Items {
 		if err := c.Delete(ctx, &svcList.Items[i]); err != nil {
 			if apierrors.IsNotFound(err) {
 				continue
 			}
-			return deleted, fmt.Errorf("deleting service %s: %w", svcList.Items[i].Name, err)
+			errs = append(errs, fmt.Errorf("deleting service %s: %w", svcList.Items[i].Name, err))
+			continue
 		}
 		deleted++
 	}
-	return deleted, nil
+	return deleted, errors.Join(errs...)
 }
diff --git a/internal/resources/resources_test.go b/internal/resources/resources_test.go
index 3f0ced4..627ea45 100644
--- a/internal/resources/resources_test.go
+++ b/internal/resources/resources_test.go
@@ -326,6 +326,53 @@ var _ = Describe("DeleteManagedSecrets", func() {
 		})
 		Expect(err).To(HaveOccurred())
 	})
+
+	It("should continue deleting after a single delete error and return aggregated error", func() {
+		sec1 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "sec-a",
+				Namespace: "default",
+				Labels:    map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
+			},
+		}
+		sec2 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "sec-b",
+				Namespace: "default",
+				Labels:    map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
+			},
+		}
+		sec3 := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "sec-c",
+				Namespace: "default",
+				Labels:    map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
+			},
+		}
+		c := fake.NewClientBuilder().
+			WithScheme(scheme).
+			WithObjects(sec1, sec2, sec3).
+			WithInterceptorFuncs(interceptor.Funcs{
+				Delete: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.DeleteOption) error {
+					if obj.GetName() == "sec-b" {
+						return apierrors.NewForbidden(
+							schema.GroupResource{Group: "", Resource: "secrets"},
+							"sec-b",
+							nil,
+						)
+					}
+					return cl.Delete(ctx, obj, opts...)
+				},
+			}).
+			Build()
+
+		count, err := resources.DeleteManagedSecrets(ctx, c, "default", map[string]string{
+			"app.kubernetes.io/managed-by": "virtwork",
+		})
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("sec-b"))
+		Expect(count).To(Equal(2))
+	})
 })
 
 // nolint:dupl
@@ -412,4 +459,51 @@ var _ = Describe("DeleteManagedServices", func() {
 		})
 		Expect(err).To(HaveOccurred())
 	})
+
+	It("should continue deleting after a single delete error and return aggregated error", func() {
+		svc1 := &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "svc-a",
+				Namespace: "default",
+				Labels:    map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
+			},
+		}
+		svc2 := &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "svc-b",
+				Namespace: "default",
+				Labels:    map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
+			},
+		}
+		svc3 := &corev1.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "svc-c",
+				Namespace: "default",
+				Labels:    map[string]string{"app.kubernetes.io/managed-by": "virtwork"},
+			},
+		}
+		c := fake.NewClientBuilder().
+			WithScheme(scheme).
+			WithObjects(svc1, svc2, svc3).
+			WithInterceptorFuncs(interceptor.Funcs{
+				Delete: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.DeleteOption) error {
+					if obj.GetName() == "svc-b" {
+						return apierrors.NewForbidden(
+							schema.GroupResource{Group: "", Resource: "services"},
+							"svc-b",
+							nil,
+						)
+					}
+					return cl.Delete(ctx, obj, opts...)
+				},
+			}).
+			Build()
+
+		count, err := resources.DeleteManagedServices(ctx, c, "default", map[string]string{
+			"app.kubernetes.io/managed-by": "virtwork",
+		})
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("svc-b"))
+		Expect(count).To(Equal(2))
+	})
 })