Fix: Move finalizers away from GetAllMatchingInstances

controllers/controller_shared.go

@@ -126,45 +126,6 @@ func GetScopedMatchingInstances(log logr.Logger, ctx context.Context, k8sClient
 	return selectedList, nil
 }
 
-// Same as GetScopedMatchingInstances, except the scope is always global
-// Intended to be used in finalizer and onDelete functions due to allowCrossNamespaceImport being a mutable field
-// Not using this may leave behind resources in instances no longer in scope.
-func GetAllMatchingInstances(ctx context.Context, k8sClient client.Client, cr v1beta1.CommonResource) ([]v1beta1.Grafana, error) {
-	instanceSelector := cr.MatchLabels()
-
-	// Should never happen, sanity check
-	if instanceSelector == nil {
-		return []v1beta1.Grafana{}, nil
-	}
-
-	var list v1beta1.GrafanaList
-	err := k8sClient.List(ctx, &list, client.MatchingLabels(instanceSelector.MatchLabels))
-	if err != nil {
-		return []v1beta1.Grafana{}, err
-	}
-
-	if len(list.Items) == 0 {
-		return []v1beta1.Grafana{}, nil
-	}
-
-	selectedList := []v1beta1.Grafana{}
-	for _, instance := range list.Items {
-		// Matches all instances when MatchExpressions is undefined
-		selected := labelsSatisfyMatchExpressions(instance.Labels, instanceSelector.MatchExpressions)
-		if !selected {
-			continue
-		}
-		// admin url is required to interact with Grafana
-		// the instance or route might not yet be ready
-		if instance.Status.Stage != v1beta1.OperatorStageComplete || instance.Status.StageStatus != v1beta1.OperatorStageResultSuccess {
-			continue
-		}
-		selectedList = append(selectedList, instance)
-	}
-
-	return selectedList, nil
-}
-
 // getFolderUID fetches the folderUID from an existing GrafanaFolder CR declared in the specified namespace
 func getFolderUID(ctx context.Context, k8sClient client.Client, ref operatorapi.FolderReferencer) (string, error) {
 	if ref.FolderUID() != "" {

controllers/controller_shared_test.go

@@ -335,18 +335,4 @@ var _ = Describe("GetMatchingInstances functions", Ordered, func() {
 			Expect(instances).To(HaveLen(1))
 		})
 	})
-
-	Context("Ensure AllowCrossNamespaceImport is ignored by GetAllMatchingInstances", func() {
-		It("Finds all ready instances when instanceSelector is empty", func() {
-			instances, err := GetAllMatchingInstances(ctx, k8sClient, matchAllFolder)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(instances).To(HaveLen(3))
-		})
-		It("Finds matching and ready instances ignoring AllowCrossNamespaceImport", func() {
-			instances, err := GetAllMatchingInstances(ctx, k8sClient, denyFolder)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(instances).ToNot(BeEmpty())
-			Expect(instances).To(HaveLen(2))
-		})
-	})
 })

controllers/grafanafolder_controller.go

@@ -282,23 +282,23 @@ func (r *GrafanaFolderReconciler) SetupWithManager(mgr ctrl.Manager, ctx context
 }
 
 func (r *GrafanaFolderReconciler) finalize(ctx context.Context, folder *grafanav1beta1.GrafanaFolder) error {
-	list := grafanav1beta1.GrafanaList{}
-	var opts []client.ListOption
-	err := r.Client.List(ctx, &list, opts...)
+	instances, err := GetScopedMatchingInstances(r.Log, ctx, r.Client, folder)
 	if err != nil {
-		return err
+		return fmt.Errorf("fetching instances: %w", err)
 	}
 
-	for _, grafana := range list.Items {
+	reftrue := true
+	params := folders.NewDeleteFolderParams().WithForceDeleteRules(&reftrue)
+
+	for _, grafana := range instances {
 		grafana := grafana
 		if found, uid := grafana.Status.Folders.Find(folder.Namespace, folder.Name); found {
 			grafanaClient, err := client2.NewGeneratedGrafanaClient(ctx, r.Client, &grafana)
 			if err != nil {
 				return err
 			}
-			reftrue := true
-			params := folders.NewDeleteFolderParams().WithFolderUID(*uid).WithForceDeleteRules(&reftrue)
-			_, err = grafanaClient.Folders.DeleteFolder(params) //nolint
+
+			_, err = grafanaClient.Folders.DeleteFolder(params.WithFolderUID(*uid)) //nolint
 			if err != nil {
 				var notFound *folders.DeleteFolderNotFound
 				if !errors.As(err, &notFound) {
@@ -307,9 +307,8 @@ func (r *GrafanaFolderReconciler) finalize(ctx context.Context, folder *grafanav
 			}
 
 			grafana.Status.Folders = grafana.Status.Folders.Remove(folder.Namespace, folder.Name)
-			err = r.Client.Status().Update(ctx, &grafana)
-			if err != nil {
-				return err
+			if err = r.Client.Status().Update(ctx, &grafana); err != nil {
+				return fmt.Errorf("removing Folder from Grafana cr: %w", err)
 			}
 		}
 	}

controllers/grafananotificationtemplate_controller.go

@@ -160,7 +160,7 @@ func (r *GrafanaNotificationTemplateReconciler) reconcileWithInstance(ctx contex
 func (r *GrafanaNotificationTemplateReconciler) finalize(ctx context.Context, notificationTemplate *grafanav1beta1.GrafanaNotificationTemplate) error {
 	r.Log.Info("Finalizing GrafanaNotificationTemplate")
 
-	instances, err := GetAllMatchingInstances(ctx, r.Client, notificationTemplate)
+	instances, err := GetScopedMatchingInstances(r.Log, ctx, r.Client, notificationTemplate)
 	if err != nil {
 		return fmt.Errorf("fetching instances: %w", err)
 	}