From a8e48a9eb66903adc1f15a67e46a8fa81b2cc41f Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sat, 2 May 2026 22:41:11 +0100 Subject: [PATCH] fix(controller): forward APIReader+CircuitBreaker through NamespaceReconciler H4: NamespaceReconciler.reconcileMirror builds an ad-hoc SourceReconciler to delegate mirror creation. The previous version left APIReader and CircuitBreaker as nil, which silently disabled freshness verification on the namespace-driven path (a label change to a target namespace would mirror cached, possibly stale source data) and bypassed circuit breaker accounting for those reconciles. Construction extracted into newSourceReconciler so the forwarding is covered by a unit test that pins both fields by identity. --- cmd/kubemirror/main.go | 1 + pkg/controller/namespace_reconciler.go | 28 ++++++++++++----- pkg/controller/namespace_reconciler_test.go | 35 +++++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/cmd/kubemirror/main.go b/cmd/kubemirror/main.go index 5e54e26..5e28a1f 100644 --- a/cmd/kubemirror/main.go +++ b/cmd/kubemirror/main.go @@ -429,6 +429,7 @@ func main() { NamespaceLister: namespaceLister, ResourceTypes: cfg.MirroredResourceTypes, APIReader: mgr.GetAPIReader(), // Direct API reader for fresh namespace lookups + CircuitBreaker: cb, // Forwarded into reconcileMirror so namespace-driven mirrors share failure throttling } if err = namespaceReconciler.SetupWithManager(mgr); err != nil { diff --git a/pkg/controller/namespace_reconciler.go b/pkg/controller/namespace_reconciler.go index ce4acee..48f2c76 100644 --- a/pkg/controller/namespace_reconciler.go +++ b/pkg/controller/namespace_reconciler.go @@ -10,12 +10,14 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" + "github.com/lukaszraczylo/kubemirror/pkg/circuitbreaker" "github.com/lukaszraczylo/kubemirror/pkg/config" "github.com/lukaszraczylo/kubemirror/pkg/constants" "github.com/lukaszraczylo/kubemirror/pkg/filter" @@ -30,6 +32,7 @@ type NamespaceReconciler struct { Scheme *runtime.Scheme Config *config.Config Filter *filter.NamespaceFilter + CircuitBreaker *circuitbreaker.CircuitBreaker ResourceTypes []config.ResourceType } @@ -281,21 +284,30 @@ func (r *NamespaceReconciler) resolveTargetNamespaces(ctx context.Context, sourc return targetNamespaces, nil } -// reconcileMirror creates or updates a mirror in the target namespace. -// This calls the mirror creation logic from the SourceReconciler. +// reconcileMirror creates or updates a mirror in the target namespace by +// delegating to SourceReconciler.reconcileMirror so all freshness, ownership, +// and circuit-breaker behavior stays in one place. func (r *NamespaceReconciler) reconcileMirror(ctx context.Context, source *unstructured.Unstructured, targetNamespace string) error { - // Create a temporary SourceReconciler to use its mirror creation logic - // This avoids code duplication - sourceReconciler := &SourceReconciler{ + return r.newSourceReconciler(source.GroupVersionKind()). + reconcileMirror(ctx, source, source, targetNamespace) +} + +// newSourceReconciler builds an ad-hoc SourceReconciler for delegating mirror +// reconciliation. APIReader and CircuitBreaker are forwarded so namespace-driven +// mirror creates/updates use the same freshness checks and failure throttling +// as direct source reconciles. Without this, namespace label changes would +// silently bypass --verify-source-freshness and the per-resource circuit breaker. +func (r *NamespaceReconciler) newSourceReconciler(gvk schema.GroupVersionKind) *SourceReconciler { + return &SourceReconciler{ Client: r.Client, Scheme: r.Scheme, Config: r.Config, Filter: r.Filter, NamespaceLister: r.NamespaceLister, - GVK: source.GroupVersionKind(), + GVK: gvk, + APIReader: r.APIReader, + CircuitBreaker: r.CircuitBreaker, } - - return sourceReconciler.reconcileMirror(ctx, source, source, targetNamespace) } // SetupWithManager sets up the controller with the Manager. diff --git a/pkg/controller/namespace_reconciler_test.go b/pkg/controller/namespace_reconciler_test.go index 667ba06..d63f27b 100644 --- a/pkg/controller/namespace_reconciler_test.go +++ b/pkg/controller/namespace_reconciler_test.go @@ -17,6 +17,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" + "github.com/lukaszraczylo/kubemirror/pkg/circuitbreaker" "github.com/lukaszraczylo/kubemirror/pkg/config" "github.com/lukaszraczylo/kubemirror/pkg/constants" "github.com/lukaszraczylo/kubemirror/pkg/filter" @@ -237,6 +238,40 @@ func TestNamespaceReconciler_CleanupWhenNamespaceNoLongerTarget(t *testing.T) { }) } } +func TestNamespaceReconciler_newSourceReconciler_forwardsAPIReaderAndCircuitBreaker(t *testing.T) { + // Regression test (H4): the SourceReconciler that NamespaceReconciler builds + // for delegated mirror reconciliation must carry the APIReader and the + // CircuitBreaker, otherwise namespace-driven mirror updates silently bypass + // --verify-source-freshness and the per-resource failure throttling. + apiReader := &stubAPIReader{} + cb := circuitbreaker.NewWithDefaults() + + r := &NamespaceReconciler{ + APIReader: apiReader, + CircuitBreaker: cb, + Config: &config.Config{}, + } + + gvk := schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Secret"} + sr := r.newSourceReconciler(gvk) + + require.NotNil(t, sr) + assert.Same(t, apiReader, sr.APIReader, "APIReader must be forwarded") + assert.Same(t, cb, sr.CircuitBreaker, "CircuitBreaker must be forwarded") + assert.Equal(t, gvk, sr.GVK) +} + +// stubAPIReader is a minimal client.Reader for identity-comparison tests; it +// is never invoked, so the methods only need to satisfy the interface. +type stubAPIReader struct{} + +func (s *stubAPIReader) Get(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { + return nil +} + +func (s *stubAPIReader) List(_ context.Context, _ client.ObjectList, _ ...client.ListOption) error { + return nil +} // Helper functions