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.
This commit is contained in:
2026-05-02 22:41:11 +01:00
parent dfe08b35d1
commit a8e48a9eb6
3 changed files with 56 additions and 8 deletions
+1
View File
@@ -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 {
+20 -8
View File
@@ -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.
@@ -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