diff --git a/pkg/controller/source_reconciler.go b/pkg/controller/source_reconciler.go index 7f91497..08873e1 100644 --- a/pkg/controller/source_reconciler.go +++ b/pkg/controller/source_reconciler.go @@ -120,6 +120,15 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } } + // Clean up orphaned mirrors (namespaces that no longer match the target criteria) + orphanedCount, err := r.cleanupOrphanedMirrors(ctx, sourceObj, targetNamespaces) + if err != nil { + logger.Error(err, "failed to cleanup orphaned mirrors") + // Don't fail reconciliation for cleanup errors, just log them + } else if orphanedCount > 0 { + logger.Info("cleaned up orphaned mirrors", "count", orphanedCount) + } + // Update status annotation with last sync info if err := r.updateLastSyncStatus(ctx, source, sourceObj, reconciledCount, errorCount); err != nil { logger.Error(err, "failed to update sync status") @@ -293,6 +302,81 @@ func (r *SourceReconciler) deleteAllMirrors(ctx context.Context, sourceObj metav return nil } +// cleanupOrphanedMirrors removes mirrors that exist but are no longer in the target list. +// This handles cases where target-namespaces annotation changes (e.g., "all" → "all-labeled" or "app-*" → "prod-*"). +func (r *SourceReconciler) cleanupOrphanedMirrors(ctx context.Context, sourceObj metav1.Object, targetNamespaces []string) (int, error) { + logger := log.FromContext(ctx) + + // List all namespaces + allNamespaces, err := r.NamespaceLister.ListNamespaces(ctx) + if err != nil { + return 0, fmt.Errorf("failed to list namespaces: %w", err) + } + + // Get GVK from source object + sourceUnstructured, ok := sourceObj.(*unstructured.Unstructured) + if !ok { + return 0, fmt.Errorf("source object is not unstructured") + } + + // Create a set of target namespaces for quick lookup + targetSet := make(map[string]bool) + for _, ns := range targetNamespaces { + targetSet[ns] = true + } + + var deletedCount int + for _, ns := range allNamespaces { + // Skip source namespace + if ns == sourceObj.GetNamespace() { + continue + } + + // Skip if this namespace IS in the current target list + if targetSet[ns] { + continue + } + + // Check if a mirror exists in this namespace + mirror := &unstructured.Unstructured{} + mirror.SetGroupVersionKind(sourceUnstructured.GroupVersionKind()) + mirror.SetNamespace(ns) + mirror.SetName(sourceObj.GetName()) + + err := r.Get(ctx, client.ObjectKey{Namespace: ns, Name: sourceObj.GetName()}, mirror) + if errors.IsNotFound(err) { + // No mirror exists, nothing to clean up + continue + } + if err != nil { + logger.Error(err, "failed to check for mirror", "namespace", ns) + continue + } + + // Verify this is actually our mirror (not someone else's resource with the same name) + if !IsManagedByUs(mirror) { + continue + } + + // Verify this mirror points to our source + srcNs, srcName, _, found := GetSourceReference(mirror) + if !found || srcNs != sourceObj.GetNamespace() || srcName != sourceObj.GetName() { + continue + } + + // This is an orphaned mirror - delete it + if err := r.Delete(ctx, mirror); err != nil { + logger.Error(err, "failed to delete orphaned mirror", "namespace", ns) + continue + } + + deletedCount++ + logger.V(1).Info("deleted orphaned mirror", "namespace", ns) + } + + return deletedCount, nil +} + // resolveTargetNamespaces determines which namespaces should receive mirrors. func (r *SourceReconciler) resolveTargetNamespaces(ctx context.Context, sourceObj metav1.Object) ([]string, error) { annotations := sourceObj.GetAnnotations() diff --git a/pkg/controller/source_reconciler_test.go b/pkg/controller/source_reconciler_test.go index aaa8ed4..548adb5 100644 --- a/pkg/controller/source_reconciler_test.go +++ b/pkg/controller/source_reconciler_test.go @@ -466,3 +466,88 @@ func BenchmarkResolveTargetNamespaces(b *testing.B) { _, _ = r.resolveTargetNamespaces(ctx, sourceObj) } } + +func TestSourceReconciler_cleanupOrphanedMirrors(t *testing.T) { + // Setup: Source in default namespace with mirrors in app-1, app-2, app-3 + // Then target-namespaces changes to only app-1, app-2 + // Expect: app-3 mirror is deleted (orphaned) + + sourceObj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "test-secret", + "namespace": "default", + "uid": "source-uid-123", + }, + }, + } + + // Mock existing mirror in app-3 (will be orphaned) + orphanedMirror := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "test-secret", + "namespace": "app-3", + "labels": map[string]interface{}{ + constants.LabelManagedBy: constants.ControllerName, + constants.LabelMirror: "true", + }, + "annotations": map[string]interface{}{ + constants.AnnotationSourceNamespace: "default", + constants.AnnotationSourceName: "test-secret", + constants.AnnotationSourceUID: "source-uid-123", + }, + }, + }, + } + + mockClient := new(MockClient) + mockLister := new(MockNamespaceLister) + + // Setup: all namespaces in cluster + allNamespaces := []string{"default", "app-1", "app-2", "app-3", "prod-1"} + mockLister.On("ListNamespaces", mock.Anything).Return(allNamespaces, nil) + + // Current target list (after annotation change): only app-1 and app-2 + targetNamespaces := []string{"app-1", "app-2"} + + // The function will iterate through all namespaces and: + // - Skip "default" (source namespace) + // - Skip "app-1" and "app-2" (in target list) + // - Check "app-3" (not in target list) → will find orphaned mirror + // - Check "prod-1" (not in target list) → no mirror exists + + notFoundErr := errors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "test-secret") + + // app-3: orphaned mirror exists + mockClient.On("Get", mock.Anything, types.NamespacedName{Namespace: "app-3", Name: "test-secret"}, mock.Anything). + Return(nil, orphanedMirror) + + // prod-1: no mirror exists + mockClient.On("Get", mock.Anything, types.NamespacedName{Namespace: "prod-1", Name: "test-secret"}, mock.Anything). + Return(notFoundErr, nil) + + // Expect delete call for app-3 mirror + mockClient.On("Delete", mock.Anything, mock.MatchedBy(func(obj client.Object) bool { + u, ok := obj.(*unstructured.Unstructured) + return ok && u.GetNamespace() == "app-3" && u.GetName() == "test-secret" + }), mock.Anything).Return(nil) + + r := &SourceReconciler{ + Client: mockClient, + NamespaceLister: mockLister, + } + + ctx := context.Background() + deletedCount, err := r.cleanupOrphanedMirrors(ctx, sourceObj, targetNamespaces) + + require.NoError(t, err) + assert.Equal(t, 1, deletedCount, "should have deleted 1 orphaned mirror") + + mockClient.AssertExpectations(t) + mockLister.AssertExpectations(t) +}