feat(drift-detection): add periodic drift detection to all controllers (#94) (#111)

Add a DriftCheckInterval constant (5 min) and checkDrift helper methods to
all 14 CRD controllers so that resources deleted externally in AxonOps are
automatically detected and recreated on the next reconcile cycle.

- Add DriftCheckInterval = 5 * time.Minute to common package
- Extract checkDrift method per controller (returns ctrl.Result, bool, error)
- Drift check fires when Ready=True, ObservedGeneration matches, and synced ID
  is non-empty; sets DriftDetected condition and clears synced ID on mismatch
- Failed drift-check API calls set DriftCheckFailed condition and requeue
- AlertEndpoint and AlertRoute resolve their own API client inside checkDrift
  (since apiClient is resolved after the idempotency guard in those controllers)
- AdaptiveRepair, DashboardTemplate, CommitlogArchive detect drift by settings
  comparison or empty-list check instead of ID match
- Extract applyRoute helper in AlertRoute controller to keep Reconcile
  cyclomatic complexity under 30
- Update LogCollector idempotency test to use stateful mock server

Author: digiserg

internal/controller/alerts/axonopsadaptiverepair_controller.go

@@ -116,13 +116,14 @@ func (r *AxonOpsAdaptiveRepairReconciler) Reconcile(ctx context.Context, req ctr
 		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 
-	// Check if already synced and spec unchanged
+	// Check if already synced with drift detection
 	readyCond := meta.FindStatusCondition(repair.Status.Conditions, condTypeReady)
 	if readyCond != nil && readyCond.Status == metav1.ConditionTrue &&
 		repair.Status.ObservedGeneration == repair.Generation &&
 		repair.Status.LastSyncTime != nil {
-		log.Info("Adaptive repair already synced and spec unchanged, skipping API call")
-		return ctrl.Result{}, nil
+		if result, done, err := r.checkDrift(ctx, repair, apiClient); done {
+			return result, err
+		}
 	}
 
 	// Get current settings from API
@@ -163,6 +164,36 @@ func (r *AxonOpsAdaptiveRepairReconciler) Reconcile(ctx context.Context, req ctr
 }
 
 // handleDeletion handles cleanup when the CR is being deleted.
+// checkDrift verifies whether adaptive repair settings have drifted from the desired spec.
+// Returns (result, true, err) if the reconcile loop should return; (zero, false, nil) to continue.
+func (r *AxonOpsAdaptiveRepairReconciler) checkDrift(ctx context.Context, repair *alertsv1alpha1.AxonOpsAdaptiveRepair, apiClient *axonops.Client) (ctrl.Result, bool, error) {
+	log := logf.FromContext(ctx)
+	currentSettings, err := apiClient.GetAdaptiveRepair(ctx, repair.Spec.ClusterType, repair.Spec.ClusterName)
+	if err != nil {
+		log.Error(err, "Failed to perform drift check")
+		r.setFailedCondition(ctx, repair, "DriftCheckFailed", common.SafeConditionMsg("Failed to check for drift", err))
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	desired := r.buildAdaptiveRepairSettings(repair)
+	if r.settingsEqual(desired, *currentSettings) {
+		log.Info("Adaptive repair already synced and spec unchanged, skipping API call")
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	log.Info("Drift detected: adaptive repair settings have changed externally")
+	meta.SetStatusCondition(&repair.Status.Conditions, metav1.Condition{
+		Type:               "DriftDetected",
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: repair.Generation,
+		Reason:             "SettingsChanged",
+		Message:            "Adaptive repair settings were changed externally; reapplying",
+	})
+	repair.Status.LastSyncTime = nil
+	if statusErr := r.Status().Update(ctx, repair); statusErr != nil {
+		return ctrl.Result{}, true, statusErr
+	}
+	return ctrl.Result{}, false, nil
+}
+
 // Adaptive repair is a cluster-level singleton -- it cannot be "deleted" via the API.
 // The finalizer simply removes itself without making any API calls.
 func (r *AxonOpsAdaptiveRepairReconciler) handleDeletion(ctx context.Context, repair *alertsv1alpha1.AxonOpsAdaptiveRepair) error {

internal/controller/alerts/axonopsalertendpoint_controller.go

@@ -110,14 +110,14 @@ func (r *AxonOpsAlertEndpointReconciler) Reconcile(ctx context.Context, req ctrl
 		return ctrl.Result{}, nil
 	}
 
-	// Idempotency check: skip if already synced and generation unchanged
+	// Idempotency check with drift detection
 	readyCond := meta.FindStatusCondition(endpoint.Status.Conditions, condTypeReady)
 	if readyCond != nil && readyCond.Status == metav1.ConditionTrue &&
 		endpoint.Status.ObservedGeneration == endpoint.Generation &&
 		endpoint.Status.SyncedIntegrationID != "" {
-		log.Info("Endpoint already synced, skipping reconciliation",
-			"integrationID", endpoint.Status.SyncedIntegrationID, "generation", endpoint.Generation)
-		return ctrl.Result{}, nil
+		if result, done, err := r.checkDrift(ctx, endpoint); done {
+			return result, err
+		}
 	}
 
 	log.Info("Reconciling AxonOpsAlertEndpoint",
@@ -227,6 +227,53 @@ func (r *AxonOpsAlertEndpointReconciler) Reconcile(ctx context.Context, req ctrl
 	return ctrl.Result{}, nil
 }
 
+// checkDrift verifies whether the synced integration still exists in AxonOps.
+// Resolves its own API client since the main Reconcile path doesn't do so until later.
+// Returns (result, true, err) if the reconcile loop should return; (zero, false, nil) to continue.
+func (r *AxonOpsAlertEndpointReconciler) checkDrift(ctx context.Context, endpoint *alertsv1alpha1.AxonOpsAlertEndpoint) (ctrl.Result, bool, error) {
+	log := logf.FromContext(ctx)
+	apiClient, err := ResolveAPIClient(ctx, r.Client, endpoint.Namespace, endpoint.Spec.ConnectionRef)
+	if err != nil {
+		log.Error(err, "Failed to resolve API client for drift check")
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	integrations, err := apiClient.GetIntegrations(ctx, endpoint.Spec.ClusterType, endpoint.Spec.ClusterName)
+	if err != nil {
+		log.Error(err, "Failed to perform drift check")
+		meta.SetStatusCondition(&endpoint.Status.Conditions, metav1.Condition{
+			Type:               "DriftCheckFailed",
+			Status:             metav1.ConditionTrue,
+			ObservedGeneration: endpoint.Generation,
+			Reason:             "DriftCheckFailed",
+			Message:            common.SafeConditionMsg("Failed to check for drift", err),
+		})
+		if statusErr := r.Status().Update(ctx, endpoint); statusErr != nil {
+			log.Error(statusErr, "Failed to update status")
+		}
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	for _, def := range integrations.Definitions {
+		if def.ID == endpoint.Status.SyncedIntegrationID {
+			log.Info("Endpoint already synced, skipping reconciliation",
+				"integrationID", endpoint.Status.SyncedIntegrationID, "generation", endpoint.Generation)
+			return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+		}
+	}
+	log.Info("Drift detected: integration no longer exists in AxonOps", "integrationID", endpoint.Status.SyncedIntegrationID)
+	meta.SetStatusCondition(&endpoint.Status.Conditions, metav1.Condition{
+		Type:               "DriftDetected",
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: endpoint.Generation,
+		Reason:             "ResourceDeleted",
+		Message:            "Integration was deleted externally; recreating",
+	})
+	endpoint.Status.SyncedIntegrationID = ""
+	if statusErr := r.Status().Update(ctx, endpoint); statusErr != nil {
+		return ctrl.Result{}, true, statusErr
+	}
+	return ctrl.Result{}, false, nil
+}
+
 // handleDeletion handles cleanup when the CR is being deleted
 func (r *AxonOpsAlertEndpointReconciler) handleDeletion(ctx context.Context, endpoint *alertsv1alpha1.AxonOpsAlertEndpoint) (ctrl.Result, error) {
 	log := logf.FromContext(ctx)

internal/controller/alerts/axonopsalertroute_controller.go

@@ -112,14 +112,14 @@ func (r *AxonOpsAlertRouteReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{}, nil
 	}
 
-	// Check idempotency: skip if already synced and generation unchanged
+	// Check idempotency with drift detection
 	readyCond := meta.FindStatusCondition(route.Status.Conditions, condTypeReady)
 	if readyCond != nil && readyCond.Status == metav1.ConditionTrue &&
 		route.Status.ObservedGeneration == route.Generation &&
 		route.Status.IntegrationID != "" {
-		log.Info("Route already synced, skipping reconciliation",
-			"integrationID", route.Status.IntegrationID, "generation", route.Generation)
-		return ctrl.Result{}, nil
+		if result, done, err := r.checkDrift(ctx, route); done {
+			return result, err
+		}
 	}
 
 	log.Info("Reconciling AxonOpsAlertRoute",
@@ -199,10 +199,42 @@ func (r *AxonOpsAlertRouteReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{}, err
 	}
 
-	// Check if route already exists
+	// Set override and add route if needed
 	routeExists := checkRouteExists(integrations, apiRouteType, route.Spec.Severity, integrationID)
+	if result, stop := r.applyRoute(ctx, apiClient, route, apiRouteType, integrationID, routeExists); stop {
+		return result, nil
+	}
+
+	// Update status
+	now := metav1.Now()
+	if err := r.Get(ctx, req.NamespacedName, route); err != nil {
+		return ctrl.Result{}, client.IgnoreNotFound(err)
+	}
+	route.Status.IntegrationID = integrationID
+	route.Status.LastSyncTime = &now
+	route.Status.ObservedGeneration = route.Generation
+	meta.SetStatusCondition(&route.Status.Conditions, metav1.Condition{
+		Type:               condTypeReady,
+		Status:             metav1.ConditionTrue,
+		Reason:             "RouteSynced",
+		Message:            "Alert route successfully synced with AxonOps",
+		ObservedGeneration: route.Generation,
+	})
+
+	if err := r.Status().Update(ctx, route); err != nil {
+		log.Error(err, "Failed to update status")
+		return ctrl.Result{}, err
+	}
+
+	log.Info("Successfully reconciled AxonOpsAlertRoute")
+	return ctrl.Result{}, nil
+}
+
+// applyRoute sets the integration override (if needed) and adds the route (if missing).
+// Returns (result, true) if the reconcile loop should stop.
+func (r *AxonOpsAlertRouteReconciler) applyRoute(ctx context.Context, apiClient *axonops.Client, route *alertsv1alpha1.AxonOpsAlertRoute, apiRouteType, integrationID string, routeExists bool) (ctrl.Result, bool) {
+	log := logf.FromContext(ctx)
 
-	// Set override if non-global and enabled (defaults to true when nil)
 	enableOverride := route.Spec.EnableOverride == nil || *route.Spec.EnableOverride
 	if route.Spec.Type != "global" && enableOverride {
 		log.Info("Setting integration override", "routeType", apiRouteType, "severity", route.Spec.Severity)
@@ -216,58 +248,78 @@ func (r *AxonOpsAlertRouteReconciler) Reconcile(ctx context.Context, req ctrl.Re
 				Message:            common.SafeConditionMsg("Failed to set override", err),
 				ObservedGeneration: route.Generation,
 			})
-			if err := r.Status().Update(ctx, route); err != nil {
-				log.Error(err, "Failed to update status")
+			if statusErr := r.Status().Update(ctx, route); statusErr != nil {
+				log.Error(statusErr, "Failed to update status")
 			}
-			return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
+			return ctrl.Result{RequeueAfter: 30 * time.Second}, true
 		}
 	}
 
-	// Add route if it doesn't exist
-	if !routeExists {
-		log.Info("Adding integration route", "integrationID", integrationID, "routeType", apiRouteType, "severity", route.Spec.Severity)
-		if err := apiClient.AddIntegrationRoute(ctx, route.Spec.ClusterType, route.Spec.ClusterName,
-			apiRouteType, route.Spec.Severity, integrationID); err != nil {
-			log.Error(err, "Failed to add integration route")
-			meta.SetStatusCondition(&route.Status.Conditions, metav1.Condition{
-				Type:               condTypeReady,
-				Status:             metav1.ConditionFalse,
-				Reason:             "RouteError",
-				Message:            common.SafeConditionMsg("Failed to add route", err),
-				ObservedGeneration: route.Generation,
-			})
-			if err := r.Status().Update(ctx, route); err != nil {
-				log.Error(err, "Failed to update status")
-			}
-			return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
-		}
-	} else {
+	if routeExists {
 		log.Info("Route already exists, skipping creation")
+		return ctrl.Result{}, false
 	}
 
-	// Update status
-	now := metav1.Now()
-	if err := r.Get(ctx, req.NamespacedName, route); err != nil {
-		return ctrl.Result{}, client.IgnoreNotFound(err)
+	log.Info("Adding integration route", "integrationID", integrationID, "routeType", apiRouteType, "severity", route.Spec.Severity)
+	if err := apiClient.AddIntegrationRoute(ctx, route.Spec.ClusterType, route.Spec.ClusterName,
+		apiRouteType, route.Spec.Severity, integrationID); err != nil {
+		log.Error(err, "Failed to add integration route")
+		meta.SetStatusCondition(&route.Status.Conditions, metav1.Condition{
+			Type:               condTypeReady,
+			Status:             metav1.ConditionFalse,
+			Reason:             "RouteError",
+			Message:            common.SafeConditionMsg("Failed to add route", err),
+			ObservedGeneration: route.Generation,
+		})
+		if statusErr := r.Status().Update(ctx, route); statusErr != nil {
+			log.Error(statusErr, "Failed to update status")
+		}
+		return ctrl.Result{RequeueAfter: 30 * time.Second}, true
 	}
-	route.Status.IntegrationID = integrationID
-	route.Status.LastSyncTime = &now
-	route.Status.ObservedGeneration = route.Generation
+	return ctrl.Result{}, false
+}
+
+// checkDrift verifies whether the alert route still exists in AxonOps.
+// Returns (result, true, err) if the reconcile loop should return; (zero, false, nil) to continue.
+func (r *AxonOpsAlertRouteReconciler) checkDrift(ctx context.Context, route *alertsv1alpha1.AxonOpsAlertRoute) (ctrl.Result, bool, error) {
+	log := logf.FromContext(ctx)
+	apiClient, err := ResolveAPIClient(ctx, r.Client, route.Namespace, route.Spec.ConnectionRef)
+	if err != nil {
+		log.Error(err, "Failed to resolve API client for drift check")
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	integrations, err := apiClient.GetIntegrations(ctx, route.Spec.ClusterType, route.Spec.ClusterName)
+	if err != nil {
+		log.Error(err, "Failed to get integrations during drift check")
+		meta.SetStatusCondition(&route.Status.Conditions, metav1.Condition{
+			Type:               "DriftCheckFailed",
+			Status:             metav1.ConditionTrue,
+			ObservedGeneration: route.Generation,
+			Reason:             "APIError",
+			Message:            common.SafeConditionMsg("Failed to check for drift", err),
+		})
+		if statusErr := r.Status().Update(ctx, route); statusErr != nil {
+			log.Error(statusErr, "Failed to update status after drift check failure")
+		}
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	apiRouteType, _ := getAPIRouteType(route.Spec.Type)
+	if checkRouteExists(integrations, apiRouteType, route.Spec.Severity, route.Status.IntegrationID) {
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	log.Info("Drift detected: alert route no longer exists in AxonOps, will recreate")
 	meta.SetStatusCondition(&route.Status.Conditions, metav1.Condition{
-		Type:               condTypeReady,
+		Type:               "DriftDetected",
 		Status:             metav1.ConditionTrue,
-		Reason:             "RouteSynced",
-		Message:            "Alert route successfully synced with AxonOps",
 		ObservedGeneration: route.Generation,
+		Reason:             "ResourceDeleted",
+		Message:            "Alert route was deleted externally; reapplying",
 	})
-
-	if err := r.Status().Update(ctx, route); err != nil {
-		log.Error(err, "Failed to update status")
-		return ctrl.Result{}, err
+	route.Status.IntegrationID = ""
+	if statusErr := r.Status().Update(ctx, route); statusErr != nil {
+		return ctrl.Result{}, true, statusErr
 	}
-
-	log.Info("Successfully reconciled AxonOpsAlertRoute")
-	return ctrl.Result{}, nil
+	return ctrl.Result{}, false, nil
 }
 
 // handleDeletion handles cleanup when the CR is being deleted

internal/controller/alerts/axonopscommitlogarchive_controller.go

@@ -115,8 +115,9 @@ func (r *AxonOpsCommitlogArchiveReconciler) Reconcile(ctx context.Context, req c
 	if readyCond != nil && readyCond.Status == metav1.ConditionTrue &&
 		archive.Status.ObservedGeneration == archive.Generation &&
 		archive.Status.LastSyncTime != nil {
-		log.Info("Commitlog archive already synced and spec unchanged, skipping")
-		return ctrl.Result{}, nil
+		if result, done, err := r.checkDrift(ctx, archive, apiClient); done {
+			return result, err
+		}
 	}
 
 	if err := r.validateConfig(archive); err != nil {
@@ -195,6 +196,35 @@ func (r *AxonOpsCommitlogArchiveReconciler) Reconcile(ctx context.Context, req c
 	return ctrl.Result{}, nil
 }
 
+// checkDrift verifies whether commitlog archive settings still exist in AxonOps.
+// Returns (result, true, err) if the reconcile loop should return; (zero, false, nil) to continue.
+func (r *AxonOpsCommitlogArchiveReconciler) checkDrift(ctx context.Context, archive *alertsv1alpha1.AxonOpsCommitlogArchive, apiClient *axonops.Client) (ctrl.Result, bool, error) {
+	log := logf.FromContext(ctx)
+	settings, err := apiClient.GetCommitlogArchiveSettings(ctx, archive.Spec.ClusterType, archive.Spec.ClusterName)
+	if err != nil {
+		log.Error(err, "Failed to perform drift check")
+		r.setFailedCondition(ctx, archive, "DriftCheckFailed", common.SafeConditionMsg("Failed to check for drift", err))
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	if len(settings) > 0 {
+		log.Info("Commitlog archive already synced and spec unchanged, skipping")
+		return ctrl.Result{RequeueAfter: common.DriftCheckInterval}, true, nil
+	}
+	log.Info("Drift detected: commitlog archive settings no longer exist in AxonOps")
+	meta.SetStatusCondition(&archive.Status.Conditions, metav1.Condition{
+		Type:               "DriftDetected",
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: archive.Generation,
+		Reason:             "ResourceDeleted",
+		Message:            "Commitlog archive settings were removed externally; recreating",
+	})
+	archive.Status.LastSyncTime = nil
+	if statusErr := r.Status().Update(ctx, archive); statusErr != nil {
+		return ctrl.Result{}, true, statusErr
+	}
+	return ctrl.Result{}, false, nil
+}
+
 func (r *AxonOpsCommitlogArchiveReconciler) validateConfig(archive *alertsv1alpha1.AxonOpsCommitlogArchive) error {
 	switch archive.Spec.RemoteType {
 	case "s3":