Circular dependsOn silently disables event-driven reconciliation for all Kustomizations sharing the source

[gecube]

Describe the bug

When two (or more) Kustomizations referencing the same source form a dependsOn cycle, the watch mappers (requestsForRevisionChangeOf / config-change mapper in internal/controller/kustomization_indexers.go) fail in sortAndEnqueue:

{"level":"error","msg":"failed to sort dependencies for revision change","error":"circular dependency detected: flux-system/seq-a -> flux-system/seq-b -> flux-system/seq-a"}

and return nil — dropping the revision-change requests for all Kustomizations that reference that source, including ones that are not part of the cycle. From that moment, event-driven reconciliation on new revisions is effectively disabled for the whole source: every Kustomization only catches up on its spec.interval timer (commonly 10–30m in production) or via manual flux reconcile.

The degradation is completely silent from a user's perspective: all objects remain Ready=True (at the stale revision), there is no event and no condition — the only trace is the controller log line above.

Making it worse, such a cycle can enter a cluster without ever being noticed: there is no admission validation and no controller-side cycle detection, and if the cycle is introduced by patching healthy objects one at a time, each patch passes checkDependencies against the still-Ready peer — both members stay Ready=True with a closed cycle ("dormant" cycle). It only manifests on the next source revision.

Steps to reproduce

1. Create two Kustomizations seq-a, seq-b (same GitRepository, different paths, no dependsOn), plus a couple of unrelated Kustomizations on the same source. Wait for everything to become Ready.
2. kubectl patch kustomization seq-a --type=merge -p '{"spec":{"dependsOn":[{"name":"seq-b"}]}}' — reconciles fine, stays Ready.
3. kubectl patch kustomization seq-b --type=merge -p '{"spec":{"dependsOn":[{"name":"seq-a"}]}}' — also passes (seq-a is Ready), stays Ready. The cycle is now closed and undetected.
4. Push a new revision to the source.
5. Observe: the log line above; no Kustomization referencing this source reconciles — not just the cycle members, also the unrelated ones. All stay Ready=True at the old revision until spec.interval fires. (The cycle members themselves then get stuck with dependency ... revision is not up to date, which is expected; the unrelated ones apply fine once their timer or a manual reconcile triggers them.)

Verified on kustomize-controller v1.8.5 (flux CLI v2.8.8, Kubernetes v1.34); the relevant code is unchanged on main.

Expected behavior

A dependency cycle should at most affect its own members. Since the topological ordering in sortAndEnqueue is an optimization (reconcile order is not required for correctness — out-of-order reconciles are handled by the DependencyNotReady requeue mechanism), a reasonable fix seems to be: on dependency.Sort error, fall back to enqueueing the requests unsorted instead of returning nil. Optionally, the cycle members could also surface the condition as an event / Ready-condition message so the cycle is visible outside controller logs.

Additional context

Found while triaging fluxcd/flux2#5291 (flux tree ordering by dependsOn), where cycle-tolerance of dependency.Sort consumers came up.

[gecube]

Two more findings in the same code path (sortAndEnqueue → fluxcd/pkg/runtime/dependency.Sort), noting them here since one fix could cover all of it.

(a) External vertices leak into the enqueue requests. Sort builds edges from each input object's dependsOn, including targets that are not part of the input set (Kustomizations referencing a different source, or in another namespace). The DFS visits such an external vertex, marks it, and — since it has no incoming objects entry — appends it to the sorted output like any other node. sortAndEnqueue then converts the entire sorted slice into reconcile.Requests:

sorted, err := dependency.Sort(dd)
...
reqs := make([]reconcile.Request, len(sorted))

So a revision change on source S1 triggers event-driven reconciles of Kustomizations that don't reference S1 at all — they merely appear in someone's dependsOn. In a multi-tenant cluster that means tenant A pushing to their repo triggers reconciles of tenant B's Kustomization (and note that --no-cross-namespace-refs does not gate dependsOn — the ACL check only covers sourceRef — so this is reachable under the multi-tenancy lockdown profile too). No correctness impact, but spurious cross-tenant reconcile load. Filtering Sort's output back to the input set before building requests would fix this mechanically.

(b) Cross-source cycles are invisible to this detection — and to everything else. Since the outgoing edges of an external vertex are unknown to Sort (it only reads GetDependsOn of the input objects), a cycle spanning two sources — a (source S1) → x (source S2) → a — is never detected here, so it doesn't even hit the return-nil path. It's also never detected anywhere else. At runtime such a cycle is extra sneaky: cross-source dependencies skip the revision check in checkDependencies (only Ready is required), so the cycle stays fully functional while both members are Ready, and only collapses into a mutual DependencyNotReady deadlock when both happen to go not-Ready at the same time — e.g. during incident recovery, which is exactly when you least want it.

(b) is mostly an argument for the fix direction: Sort cannot serve as a reliable cycle gate (it only sees same-source subgraphs), so dropping all requests on its error buys no safety — a graceful fallback to unsorted enqueue loses nothing that the current behavior actually guarantees.

[matheuscscp]

And cycles will also not be detectable when we implement this: fluxcd/flux2#5988

So I don't really see why making noise only for cycles between Kustomizations is really helpful...