Skip to content

chore(recipes): make resolved recipes the single source of truth for chart versions #966

Description

@yuanchen8911

Summary

Make the resolved recipe the single source of truth for chart versions:

  1. Stop reading helm.defaultVersion during recipe resolution (and from tools/bom).
  2. Require every Helm componentRef in a resolved recipe to carry an explicit version.
  3. Deprecate helm.defaultVersion in recipes/registry.yaml; keep helm.defaultRepository and helm.defaultChart (component identity).

The drift class is real: today the resolver silently copies helm.defaultVersion into ComponentRef.Version when an overlay omits one (pkg/recipe/metadata.go:172), and tools/bom reads the registry directly (tools/bom/main.go:190, tools/bom/helm.go:51), so the final deployed version can come from either place and they can disagree.

Motivation

#894 surfaced the concrete failure: gpu-operator registry advertised v26.3.1, base.yaml shadowed it with v25.10.1, so leaves shipped v25.10.1 while docs/user/container-images.md (rendered from registry) advertised v26.3.1.

Leaf-level version variance is broad, not unique to gpu-operator. Examples on main:

Component Registry / base Leaf override
kube-prometheus-stack 84.4.0 (registry.yaml:337) 83.7.0 on AKS (aks.yaml:76)
network-operator 26.1.1 (registry) 26.1.1 (aks.yaml — same, but mid-overlay pin)
aws-ebs-csi-driver registry default 2.59.0 pinned in eks.yaml
dynamo-platform, grove registry default per-overlay pins on AKS / GB200 inference leaves

A component-only BOM today cannot represent these correctly. Conceptually the registry is also wrong as a version owner: per .claude/CLAUDE.md, "metadata is separate from consumption — recipes define what, bundlers determine how." Version is consumption.

Prerequisites (block the cleanup; each is a separable PR)

  1. Migrate tools/bom to render per resolved leaf, using both the resolved chart version AND resolved effective values.

    • Today: walks recipes/registry.yaml and renders each chart at helm.defaultVersion.
    • Required: enumerate the resolvable leaves, resolve each through pkg/recipe, and for every componentRef, run helm template with the same effective values that bundling uses — that's the resolved chart version plus the overlay's valuesFile, any overrides, and leaf-specific component enablement (a leaf that disables driver won't pull the driver image at all). Rendering at version alone misses image-set changes driven by values.
    • Aggregate per (component, version, effective-values-hash) so the BOM table reflects every distinct rendered image set actually deployed by some leaf — including AKS's kube-prometheus-stack 83.7.0 and any per-cloud sub-image overrides.
    • Leaf enumeration must use an explicit source of truth. Two viable definitions, pick one in the implementing PR:
      • (a) Graph leaves: overlays in recipes/overlays/*.yaml that are not named as base: by any other overlay. Easiest to derive deterministically.
      • (b) Fully-resolvable-criteria overlays: overlays whose spec.criteria declares every axis (service, accelerator, intent, os, optionally platform) that the criteria resolver requires for a unique match.
      • Avoid implicit "everything with criteria" — many parent overlays (eks.yaml, aks-training.yaml) carry partial criteria and would be incorrectly enumerated.
    • Without all of this, removing the registry default silently empties the BOM AND continues to hide existing per-leaf drift.
    • References to update: tools/bom/main.go:190, tools/bom/helm.go:51.
  2. Add a recipe-resolution version-required invariant with a migration-aware error message.

    • pkg/recipe/metadata.go:172-174 silently leaves ref.Version as "" when the registry default is absent.
    • Downstream is inconsistent: localformat deployer hard-fails (pkg/bundler/deployer/localformat/vendor.go:303), but helmfile / flux / argocd emit empty version: and let helm resolve to "latest" at install time — a silent stale-default failure mode.
    • Required: any resolved Helm componentRef with empty VersionErrCodeInvalidRequest during recipe resolution. The error message MUST be migration-aware so external-registry users who relied on helm.defaultVersion understand what changed:
      Helm component "<name>" is missing version on its componentRef.
      helm.defaultVersion in the registry is no longer consulted during
      recipe resolution (see ADR-008 / issue #966). Set version: on the
      componentRef in the overlay where it is introduced.
      
    • Worth doing on its own merits even if the rest of chore(recipes): make resolved recipes the single source of truth for chart versions #966 slips.
  3. Update the pinning gate and ADR-006.

    • make bom-pinning-check (Makefile:297) currently enforces "every Helm component in registry.yaml has helm.defaultVersion." After the migration, this needs to become "every Helm componentRef in every resolved recipe has a version." Same leaf-enumeration source as Prereq 1.
    • docs/design/006-image-pinning-policy.md:79 explicitly says registry.yaml MUST declare defaultVersion. The ADR needs to be amended (or a successor ADR added) saying resolved component refs MUST be pinned, and registry.yaml no longer owns versions.

External-registry compatibility (staged)

Current docs show custom external ComponentRegistry resources using defaultVersion. Removing the field outright is a data-model break. Stage the change:

  • Stage 1 (this issue): Stop reading helm.defaultVersion in pkg/recipe resolution and in tools/bom. The field is still tolerated in registry YAML for backwards compatibility but is no longer consulted. The version-required validator (Prereq 2) will fail loudly with a migration-aware message rather than silently deploying latest. Release-note migration guidance: external registries should move their version pins into the matching overlay.
  • Stage 2 (separate issue): Once a release has shipped with Stage 1, mark helm.defaultVersion as deprecated in the schema and emit a load-time warning when it's set. Eventually remove.

Migration plan (after prerequisites land)

  1. Locate, then promote. For every component currently relying on the registry default, identify the overlay(s) where its componentRef is introduced and pin the version there. Don't move pins around unless componentRef placement is wrong on its own merits.
  2. Drop defaultVersion from recipes/registry.yaml for every Helm component. Keep defaultRepository, defaultChart, defaultNamespace.
  3. Update docs/user/component-catalog.md prose. Current lede says "The source of truth is recipes/registry.yaml. Each entry … defines the component's Helm chart (or Kustomize source), default version, namespace …" Reword: registry holds identity; overlays hold version.
  4. Update docs/contributor/data.md wherever it documents the resolver's version-fallback behavior.

Acceptance tests

These are the real acceptance gates for this proposal — passing aicr validate against the current tree does not validate this work (it tests the current data model, not the new invariant):

  • For every enumerated leaf (same source-of-truth as Prereq 1), aicr query --selector components.<name>.version resolves to the same value pre- and post-migration. Snapshot diff in the PR description proves it.
  • BOM (docs/user/container-images.md) contains every (component, version, effective-values) row that any leaf actually deploys, including the AKS kube-prometheus-stack 83.7.0 ↔ base 84.4.0 split. The BOM diff in the PR description proves it.
  • A test recipe with a Helm componentRef missing version: produces ErrCodeInvalidRequest from aicr recipe. The error message contains the migration guidance from Prereq 2.
  • make qualify passes — including the updated bom-pinning-check.

Definition of done

  • Prereq 1: tools/bom rewritten to traverse resolved leaves with resolved effective values; per-(component, version) aggregation; explicit leaf-enumeration rule.
  • Prereq 2: Recipe-resolution version-required invariant merged with migration-aware error message.
  • Prereq 3: make bom-pinning-check rewritten + ADR-006 amended (or ADR-008 supersedes it).
  • defaultVersion removed from every Helm component in recipes/registry.yaml; versions live at their overlay/mixin/leaf of introduction.
  • Component-catalog prose + docs/contributor/data.md updated.
  • Stage-1 external-registry migration documented in release notes.
  • All acceptance tests pass.

Out of scope

  • Stage-2 deprecation / removal of the defaultVersion field itself (separate issue, gated on Stage 1 shipping in a release).
  • Kustomize components. defaultTag lives in the same field position; we can address Kustomize-version semantics separately if the parallel makes sense.
  • Any version bumps. This is a pure structural refactor — every resolved leaf must produce the same chart version before and after.

Related


Credit: scope (v2), prereq-3, BOM-per-leaf, and the staged-deprecation framing came from a Codex review of the v1 draft. Resolved-effective-values BOM, precise leaf enumeration, and migration-aware error message added from a v2 Codex review.

Metadata

Metadata

Assignees

Type

Fields

No fields configured for Task.

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions