mirror of
https://github.com/lukaszraczylo/kubemirror.git
synced 2026-06-26 04:43:12 +00:00
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).
This commit is contained in:
@@ -102,10 +102,12 @@ func main() {
|
|||||||
"Burst limit for API server requests.")
|
"Burst limit for API server requests.")
|
||||||
flag.DurationVar(&resyncPeriod, "resync-period", 10*time.Minute,
|
flag.DurationVar(&resyncPeriod, "resync-period", 10*time.Minute,
|
||||||
"Period for resyncing all resources (catches updates missed due to informer cache delays).")
|
"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. "+
|
"Verify source resource freshness by comparing cache with direct API read. "+
|
||||||
"Prevents mirroring stale data when cache lags behind watch events. "+
|
"Prevents mirroring stale data and missed orphan cleanups when the informer "+
|
||||||
"Trade-off: Extra API call when cache is stale.")
|
"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,
|
flag.BoolVar(&lazyWatcherInit, "lazy-watcher-init", false,
|
||||||
"Enable lazy watcher initialization - only create informers for resource types that have resources marked for mirroring. "+
|
"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. "+
|
"Significantly reduces memory usage by avoiding watchers for unused resource types. "+
|
||||||
|
|||||||
@@ -222,14 +222,19 @@ func ResolveTargetNamespaces(
|
|||||||
default:
|
default:
|
||||||
// Check if it's a pattern or direct namespace name
|
// Check if it's a pattern or direct namespace name
|
||||||
if strings.Contains(pattern, "*") || strings.Contains(pattern, "?") {
|
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 {
|
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
|
targetMap[ns] = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} 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) {
|
if pattern != sourceNamespace && filter.IsAllowed(pattern) {
|
||||||
targetMap[pattern] = true
|
targetMap[pattern] = true
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -376,6 +376,23 @@ func TestResolveTargetNamespaces_EdgeCases(t *testing.T) {
|
|||||||
// Only "specific-ns" would be allowed, but it's not in allNamespaces
|
// Only "specific-ns" would be allowed, but it's not in allNamespaces
|
||||||
assert.Empty(t, got)
|
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
|
// Benchmark tests for critical paths
|
||||||
|
|||||||
Reference in New Issue
Block a user