From 99c0eccd53b708dbdffdad7b9dff06ef54daa817 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sat, 2 May 2026 22:36:50 +0100 Subject: [PATCH] fix: default verify-source-freshness=true; honor opt-out for glob H1: --verify-source-freshness used to default to false, so any source update whose annotation was still in the informer cache (5-20s lag) would resolve the wrong target list. cleanupOrphanedMirrors then ran against the stale list and missed orphans (manifested in e2e as 'Orphaned mirror in kubemirror-e2e-app-1 not deleted within timeout' after target-namespaces was changed). Defaulting to true fixes the race; the trade-off is one extra API read per stale-cache reconcile. M2: ResolveTargetNamespaces glob branch checked filter.IsAllowed but not the opt-out map, so a namespace labeled allow-mirrors=false would still receive a mirror through patterns like 'app-*'. The 'all' branch already had the guard; the glob branch now does too. Direct namespace listings still bypass opt-out by design (explicit author intent). --- cmd/kubemirror/main.go | 8 +++++--- pkg/filter/namespace.go | 11 ++++++++--- pkg/filter/namespace_test.go | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/cmd/kubemirror/main.go b/cmd/kubemirror/main.go index 7f0fd73..5e54e26 100644 --- a/cmd/kubemirror/main.go +++ b/cmd/kubemirror/main.go @@ -102,10 +102,12 @@ func main() { "Burst limit for API server requests.") flag.DurationVar(&resyncPeriod, "resync-period", 10*time.Minute, "Period for resyncing all resources (catches updates missed due to informer cache delays).") - flag.BoolVar(&verifySourceFreshness, "verify-source-freshness", false, + flag.BoolVar(&verifySourceFreshness, "verify-source-freshness", true, "Verify source resource freshness by comparing cache with direct API read. "+ - "Prevents mirroring stale data when cache lags behind watch events. "+ - "Trade-off: Extra API call when cache is stale.") + "Prevents mirroring stale data and missed orphan cleanups when the informer "+ + "cache lags behind watch events. Trade-off: one extra API call per reconcile "+ + "when the cache is stale. Disable only if you are confident your cluster's "+ + "watch latency is negligible.") flag.BoolVar(&lazyWatcherInit, "lazy-watcher-init", false, "Enable lazy watcher initialization - only create informers for resource types that have resources marked for mirroring. "+ "Significantly reduces memory usage by avoiding watchers for unused resource types. "+ diff --git a/pkg/filter/namespace.go b/pkg/filter/namespace.go index 7fd941c..ed7db86 100644 --- a/pkg/filter/namespace.go +++ b/pkg/filter/namespace.go @@ -222,14 +222,19 @@ func ResolveTargetNamespaces( default: // Check if it's a pattern or direct namespace name if strings.Contains(pattern, "*") || strings.Contains(pattern, "?") { - // It's a glob pattern - match against all namespaces + // It's a glob pattern - match against all namespaces. Honor + // opt-out namespaces (allow-mirrors=false) the same way the + // "all" case does — otherwise an opt-out namespace can still + // receive a mirror via a glob like "app-*". for _, ns := range allNamespaces { - if matchesPattern(ns, pattern) && ns != sourceNamespace && filter.IsAllowed(ns) { + if matchesPattern(ns, pattern) && ns != sourceNamespace && filter.IsAllowed(ns) && !optOutMap[ns] { targetMap[ns] = true } } } else { - // Direct namespace name + // Direct namespace name. Explicit listing is treated as + // intentional opt-in by the source author and bypasses the + // opt-out guard (matches prior behavior). if pattern != sourceNamespace && filter.IsAllowed(pattern) { targetMap[pattern] = true } diff --git a/pkg/filter/namespace_test.go b/pkg/filter/namespace_test.go index 0013649..d972d63 100644 --- a/pkg/filter/namespace_test.go +++ b/pkg/filter/namespace_test.go @@ -376,6 +376,23 @@ func TestResolveTargetNamespaces_EdgeCases(t *testing.T) { // Only "specific-ns" would be allowed, but it's not in allNamespaces assert.Empty(t, got) }) + + t.Run("glob pattern honors opt-out namespaces", func(t *testing.T) { + // Regression: a namespace with allow-mirrors=false used to receive a + // mirror via a glob like "app-*" because the glob branch ignored the + // opt-out list (only the "all" branch checked it). + got := ResolveTargetNamespaces( + []string{"app-*"}, + []string{"app-1", "app-2", "app-optout"}, + []string{}, + []string{"app-optout"}, + "default", + NewNamespaceFilter([]string{}, []string{}), + ) + assert.Contains(t, got, "app-1") + assert.Contains(t, got, "app-2") + assert.NotContains(t, got, "app-optout", "opt-out namespace must not receive mirrors via glob match") + }) } // Benchmark tests for critical paths