You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Unify the two topological-sort implementations in pkg/recipe/metadata.go into a single code path. Today there are two parallel Kahn's-algorithm implementations:
TopologicalSort() — flat order, used at recipe resolution to compute DeploymentOrder.
ComponentRefsTopologicalLevels() / TopologicalLevels() — level-grouped order, used at bundle generation by the helmfile deployer.
Motivation
The two paths are duplicated logic that must be kept in sync. This was called out as known tech debt left in under release pressure when the level-grouped variant was added. The duplication directly caused extra work in #1465: honoring overrides: {enabled: false} had to be implemented twice (skip-disabled + drop-edges-to-declared-but-disabled) — once in each function — and the bundler needed a separate dangling-dependency prune so the helmfile path didn't re-trip a false circular-dependency error. A single implementation would have required the fix in one place.
Proposed approach
Have one implementation be the single source of truth and express the other in terms of it (e.g. derive the flat order by flattening the levels, or factor the shared graph build + enabled-filtering + cycle detection into one helper that both thin wrappers call). Preserve current behavior:
Deterministic ordering (alphabetical tie-break within a level / queue).
Enabled-filtering semantics from fix(recipe): fix deployment ordering to honor enabled:false #1465: skip disabled components; treat an edge to a declared-but-disabled component as satisfied externally; keep edges to undeclared components so missing-dependency errors still surface.
The level grouping the helmfile deployer relies on (mutually independent components share a level).
Acceptance criteria
One shared graph/sort implementation; TopologicalSort and TopologicalLevels/ComponentRefsTopologicalLevels are thin wrappers over it (no duplicated Kahn loop or enabled-filtering logic).
Existing tests pass unchanged, including TestRecipeMetadataSpecTopologicalLevelsMatchesSort (flat order stays consistent with flattened levels).
No behavior change for recipes without disabled components.
Context
Follow-up to #1465 (which fixed enabled: false across both paths) and the discussion there. Refactor only — no functional change.
Summary
Unify the two topological-sort implementations in
pkg/recipe/metadata.gointo a single code path. Today there are two parallel Kahn's-algorithm implementations:TopologicalSort()— flat order, used at recipe resolution to computeDeploymentOrder.ComponentRefsTopologicalLevels()/TopologicalLevels()— level-grouped order, used at bundle generation by the helmfile deployer.Motivation
The two paths are duplicated logic that must be kept in sync. This was called out as known tech debt left in under release pressure when the level-grouped variant was added. The duplication directly caused extra work in #1465: honoring
overrides: {enabled: false}had to be implemented twice (skip-disabled + drop-edges-to-declared-but-disabled) — once in each function — and the bundler needed a separate dangling-dependency prune so the helmfile path didn't re-trip a false circular-dependency error. A single implementation would have required the fix in one place.Proposed approach
Have one implementation be the single source of truth and express the other in terms of it (e.g. derive the flat order by flattening the levels, or factor the shared graph build + enabled-filtering + cycle detection into one helper that both thin wrappers call). Preserve current behavior:
Acceptance criteria
TopologicalSortandTopologicalLevels/ComponentRefsTopologicalLevelsare thin wrappers over it (no duplicated Kahn loop or enabled-filtering logic).TestRecipeMetadataSpecTopologicalLevelsMatchesSort(flat order stays consistent with flattened levels).Context
Follow-up to #1465 (which fixed
enabled: falseacross both paths) and the discussion there. Refactor only — no functional change.