Skip to content

Recipe topo sort does not honor enabled:false (overlay component disable broken) #1464

Description

@yuanchen8911

Summary

TopologicalSort() in pkg/recipe/metadata.go does not honor a component's enabled: false setting, so a component disabled in a recipe/overlay (via overrides: {enabled: false}) still appears in the recipe's deploymentOrder. As a result, disabling a base component in an overlay does not work at the recipe layer — only at bundle time via --set <component>:enabled=false.

This blocks the clean way to make OKE drop the AICR-bundled cert-manager (OKE infra provides its own), which is part of the P0 vanilla-OKE recipe work.

Details

There are two distinct gaps:

  1. Topo sort ignores IsEnabled(). TopologicalSort() builds its node set by iterating over every ComponentRef with no IsEnabled() check:

    for _, c := range s.ComponentRefs {
        inDegree[c.Name] = len(c.DependencyRefs)
    }

    So a cert-manager marked enabled: false in recipes/overlays/oke.yaml still lands in deploymentOrder and in the resolved recipe output, breaking the OKE UAT assert (assert-recipe.yaml) which expects exactly 10 components with no cert-manager.

  2. Dependency edges to a disabled node. Even after adding an IsEnabled() skip to the node loop, the sort still breaks: gpu-operator and nvsentinel list cert-manager in their dependencyRefs, so their inDegree still counts it. With cert-manager removed from the queue, that edge never decrements, the dependent never reaches inDegree 0, and the cycle guard (len(result) \!= len(s.ComponentRefs)) falsely fires "circular dependencies exist".

Note the asymmetry: the bundler already filters disabled components (pkg/bundler/bundler.go, ~L224-262), but the recipe layer does not — so disabled components remain visible in aicr recipe output and in deploymentOrder.

Proposed fix

Make the recipe layer honor enabled:

  • Skip disabled components when building inDegree and the work queue.
  • When computing inDegree, only count dependency edges whose target is an enabled component — i.e., treat an edge pointing at a disabled (externally-provided) component as already satisfied.
  • Compare len(result) against the count of enabled components, not all components, in the cycle check.

Semantic: a disabled component that others depend on is treated as "satisfied externally" (the OKE/OCI cert-manager case). Estimated ~10 lines, fully unit-testable.

This keeps base.yaml as the single source of truth (no need to remove cert-manager from base and re-add it to every other overlay), makes overrides: {enabled: false} work as expected in overlays, and generalizes to any "CSP already provides X" case. The bundle-time --set <component>:enabled=false escape hatch continues to work and takes precedence over the recipe-level default.

Acceptance criteria

  • An overlay with overrides: {enabled: false} on a base component produces a recipe whose deploymentOrder and componentRefs exclude that component.
  • Disabling a component that others depend on does not trigger a false "circular dependencies" error.
  • --set <component>:enabled=true still re-enables a recipe-disabled component at bundle time.
  • Unit tests cover: disabled leaf component, disabled component with dependents, and the --set override precedence.

Metadata

Metadata

Assignees

Type

Fields

No fields configured for Bug.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions