fix: loopDetected condition never applied

chore: Move InvalidSpec loopDetection above Grafana instance fetching

controllers/notificationpolicy_controller.go

@@ -116,6 +116,31 @@ func (r *GrafanaNotificationPolicyReconciler) Reconcile(ctx context.Context, req
 	}
 	removeInvalidSpec(&notificationPolicy.Status.Conditions)
 
+	// Assemble routes and check for loops
+	var mergedRoutes []*v1beta1.GrafanaNotificationPolicyRoute
+	if notificationPolicy.Spec.Route.RouteSelector != nil || notificationPolicy.Spec.Route.HasRouteSelector() {
+		mergedRoutes, err = assembleNotificationPolicyRoutes(ctx, r.Client, notificationPolicy)
+
+		if errors.Is(err, ErrLoopDetected) {
+			meta.SetStatusCondition(&notificationPolicy.Status.Conditions, metav1.Condition{
+				Type:               conditionNotificationPolicyLoopDetected,
+				Status:             metav1.ConditionTrue,
+				ObservedGeneration: notificationPolicy.Generation,
+				Reason:             "LoopDetected",
+				Message:            fmt.Sprintf("Loop detected in notification policy routes: %s", err.Error()),
+			})
+			meta.RemoveStatusCondition(&notificationPolicy.Status.Conditions, conditionNotificationPolicySynchronized)
+			return ctrl.Result{}, fmt.Errorf("failed to assemble notification policy routes: %w", err)
+		}
+
+		if err != nil {
+			r.Recorder.Event(notificationPolicy, corev1.EventTypeWarning, "AssemblyFailed", fmt.Sprintf("Failed to assemble GrafanaNotificationPolicy using routeSelectors: %v", err))
+			return ctrl.Result{}, fmt.Errorf("failed to assemble GrafanaNotificationPolicy using routeSelectors: %w", err)
+		}
+	}
+
+	meta.RemoveStatusCondition(&notificationPolicy.Status.Conditions, conditionNotificationPolicyLoopDetected)
+
 	instances, err := GetScopedMatchingInstances(ctx, r.Client, notificationPolicy)
 	if err != nil {
 		setNoMatchingInstancesCondition(&notificationPolicy.Status.Conditions, notificationPolicy.Generation, err)
@@ -132,16 +157,6 @@ func (r *GrafanaNotificationPolicyReconciler) Reconcile(ctx context.Context, req
 	removeNoMatchingInstance(&notificationPolicy.Status.Conditions)
 	log.Info("found matching Grafana instances for notificationPolicy", "count", len(instances))
 
-	var mergedRoutes []*v1beta1.GrafanaNotificationPolicyRoute
-
-	if notificationPolicy.Spec.Route.RouteSelector != nil || notificationPolicy.Spec.Route.HasRouteSelector() {
-		mergedRoutes, err = assembleNotificationPolicyRoutes(ctx, r.Client, notificationPolicy)
-		if err := r.handleAssembleError(err, notificationPolicy); err != nil {
-			return ctrl.Result{}, err
-		}
-	}
-	meta.RemoveStatusCondition(&notificationPolicy.Status.Conditions, conditionNotificationPolicyLoopDetected)
-
 	applyErrors := make(map[string]string)
 	for _, grafana := range instances {
 		// can be removed in go 1.22+
@@ -212,12 +227,12 @@ func assembleNotificationPolicyRoutes(ctx context.Context, k8sClient client.Clie
 				matchedRoute := &routes[i]
 				key := matchedRoute.NamespacedResource()
 
-				if _, exists := visitedGlobal[key]; !exists {
+				if !visitedGlobal[key] {
 					mergedRoutes = append(mergedRoutes, matchedRoute)
 					visitedGlobal[key] = true
 				}
 
-				if _, exists := visitedChilds[key]; exists {
+				if visitedChilds[key] {
 					return fmt.Errorf("%w: %s exists", ErrLoopDetected, key)
 				}
 				visitedChilds[key] = true
@@ -450,23 +465,3 @@ func statusDiscoveredRoutes(routes []*v1beta1.GrafanaNotificationPolicyRoute) []
 func setInvalidSpecMutuallyExclusive(conditions *[]metav1.Condition, generation int64) {
 	setInvalidSpec(conditions, generation, "FieldsMutuallyExclusive", "RouteSelector and Routes are mutually exclusive")
 }
-
-// handleAssembleError handles errors during notification policy assembly
-func (r *GrafanaNotificationPolicyReconciler) handleAssembleError(err error, notificationPolicy *grafanav1beta1.GrafanaNotificationPolicy) error {
-	if err == nil {
-		return nil
-	}
-
-	if errors.Is(err, ErrLoopDetected) {
-		meta.SetStatusCondition(&notificationPolicy.Status.Conditions, metav1.Condition{
-			Type:               conditionNotificationPolicyLoopDetected,
-			Status:             metav1.ConditionTrue,
-			ObservedGeneration: notificationPolicy.Generation,
-			Reason:             "LoopDetected",
-			Message:            fmt.Sprintf("Loop detected in notification policy routes: %s", err.Error()),
-		})
-		return nil
-	}
-	r.Recorder.Event(notificationPolicy, corev1.EventTypeWarning, "AssemblyFailed", fmt.Sprintf("Failed to assemble GrafanaNotificationPolicy using routeSelectors: %v", err))
-	return fmt.Errorf("failed to assemble GrafanaNotificationPolicy using routeSelectors: %w", err)
-}