feat(bom): render divergent recipe version pins as BOM variants

[yuanchen8911]

Summary

The container BOM (docs/user/container-images.md and the CycloneDX output of tools/bom) now lists version variants: every Helm chart version explicitly pinned by a base/overlay/mixin that differs from the component's registry default, with the images that version actually ships.

Motivation / Context

A recipe source that pins a component to a non-default chart version deploys images the BOM never mentioned — the doc only rendered each component at its registry defaultVersion. Issue #1611 (revised) scopes the fix to declaration-level discovery: derive variants directly from the explicit componentRefs[].version pins versus the registry defaults, with no shared exemption file and no effective-recipe resolution. Today that surfaces one variant: kube-prometheus-stack@83.7.0, pinned by aks.

Fixes: #1611
Related: #1615, #1616

Non-goals (documented in tools/bom/variants.go, per the revised #1611):

• Effective-recipe resolution (inheritance chains, mixin composition, criteria matching) — that is the recipe resolver's job.
• Source/chart coordinate divergence analysis — variants render at registry coordinates by definition (catalog parity).
• ComponentRef type analysis beyond "is this declared pin a Helm pin" — explicitly non-Helm-typed refs are skipped.

Type of Change

• New feature (non-breaking change that adds functionality)
• Documentation update
• Build/CI/tooling

Component(s) Affected

• Docs/examples (docs/, examples/)
• Other: pkg/bom, tools/bom, .github/workflows/merge-gate.yaml

Implementation Notes

Discovery (tools/bom/variants.go):

• loadRecipeSources reads every overlay/mixin YAML under the same -repo-root recursively (nested overlay subdirectories are documented and supported), parsing into the canonical pkg/recipe types with the metadata store's exact semantics: mixins decode strictly (KnownFields(true), hard RecipeMixin kind check — a typo'd field fails closed just as it does at recipe load); overlays decode leniently exactly as the store does (empty kind is legacy RecipeMetadata, any other kind is skipped and never mined for pins — a field the resolver would ignore is equally invisible here, so the projection matches what deploys).
• Fail-closed on ambiguity: malformed YAML, a missing metadata.name, duplicate source names, and whitespace-padded version pins are errors, never silent skips — a silently dropped source could hide a divergent pin.
• Reads are confined to the recipes root via os.Root (an escaping symlink is rejected, not followed) and size-bounded; sources are opened O_NONBLOCK and validated regular via fstat on the opened handle, so a non-regular target (e.g. a symlinked FIFO, whose open would otherwise block until a writer appears) fails closed with no stat-to-open race window; the walk checks ctx per entry.
• deriveVariants compares each explicit Helm pin against the registry defaultVersion and aggregates divergences by (component, version) with the sorted declaring source names. An explicit pin over an empty registry default still renders as a variant (external -repo-root registries may omit defaultVersion). The version-pin guard's versionPinExemptions plays no role: it is validation policy (whether a divergence is allowed), not deployment fact.
• Variants render through the same surveyComponent path as default entries, at the variant version — so variant image sets are rendered, never copied, and mixed Helm+manifest components contribute their manifest images too.

Rendering (pkg/bom):

• Additive entry points BuildBOMWithVariants / WriteMarkdownWithVariants; the legacy BuildBOM / WriteMarkdown surfaces are byte-identical to main (pinned by TestLegacyEntryPointsUnchanged), so external importers see no behavioral change. pkg/evidence/attestation is untouched.
• Variant CycloneDX entries get version-qualified bom-refs (<meta.Name>/<comp>@<ver>) with aicr:variant:of and sorted aicr:variant:sources properties; variant images join the dependency graph. The checked builder fails closed on any duplicate bom-ref instead of emitting an ambiguous graph.
• Markdown gains a ## Version variants table (distinct headers, so the Components table and its existing freshness gate are unaffected) and per-variant image sections; the section is omitted entirely when no variants exist. A caller-supplied rendering-fidelity note (Metadata.RenderFidelity) documents the catalog-parity limitation: charts are rendered with the shared recipes/components/<name>/values.yaml, per-recipe overlay overrides are not applied.

Freshness gate:

• TestCommittedBOMVariantsMatchRecipePins gates the committed variants table bidirectionally: a new divergent pin without a variant row fails, and a stale row without a backing pin fails. The table parser is strict (heading-anchored, delimiter-validated, malformed rows are errors) so a mangled doc cannot pass vacuously.
• The bom-freshness merge-gate job runs both freshness tests and positively asserts a PASS line for each, so a rename or skip cannot silently drop either gate.

Testing

make qualify           # full gate, passing at the pushed SHA
go test -race ./pkg/bom/... ./tools/bom/...
make bom-docs          # regenerated docs/user/container-images.md (committed)

• Coverage vs origin/main: pkg/bom 95.5% → 95.8% (+0.3%), tools/bom 79.5% → 81.8% (+2.3%).
• Legacy compatibility is pinned by test: WriteMarkdown ≡ WriteMarkdownWithVariants(nil) byte-for-byte; legacy BuildBOM keeps its historical duplicate tolerance. All pre-existing pkg/bom and tools/bom tests pass unchanged.

Risk Assessment

• Low — Isolated change, well-tested, easy to revert

Rollout notes: Additive only. The legacy pkg/bom Go API and its output are byte-identical; tools/bom output changes are confined to the new variants table, the fidelity note, and variant entries in the CycloneDX document. Reverting restores the previous BOM shape with no migration.

Checklist

• Tests pass locally (make test with -race)
• Linter passes (make lint)
• I did not skip/disable tests to make CI green
• I added/updated tests for new functionality
• I updated docs if user-facing behavior changed
• Changes follow existing patterns in the codebase
• Commits are cryptographically signed (git commit -S) — GPG signing info

[github-actions]

🌿 Preview your docs: https://nvidia-preview-feat-bom-variant-rows.docs.buildwithfern.com/aicr

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds recipe-derived BOM variants for divergent Helm pins, rendering variant images through CycloneDX and Markdown outputs. It adds checked BOM reference validation, render-fidelity metadata, strict recipe-source loading, bidirectional freshness tests, CI execution checks, variant-focused test coverage, and regenerated container image documentation.

Estimated code review effort: 4 (Complex) | ~75 minutes

Suggested labels: area/recipes, area/tests, theme/recipes, theme/validation

Suggested reviewers: mchmarny, lockwobr

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: rendering divergent recipe version pins as BOM variants.
Description check	✅ Passed	The description is directly related to the variant-rendering BOM changes and supporting docs, tests, and CI updates.
Linked Issues check	✅ Passed	The PR matches #1611 by deriving recipe-based variants, preserving default refs, adding variant tables, and gating freshness bidirectionally.
Out of Scope Changes check	✅ Passed	The added docs, tests, and workflow updates appear necessary to support the BOM variant feature and are not unrelated scope.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/bom/bom.go`:
- Around line 114-121: Add a regression assertion for the serialized CycloneDX
output produced by BuildBOM, not just the markdown and duplicate-count checks in
TestLegacyEntryPointsUnchanged. Use BuildBOM and the encoded BOM serialization
path to compare the full byte-stable output, verifying component/dependency
ordering and image dedup remain unchanged for the legacy entry point.

In `@tools/bom/freshness_test.go`:
- Around line 465-486: In parseBOMVariantsTable, the generic blank-row skip
currently allows all-empty table rows like | | | | | to be ignored before
validation, so remove that early continue and let the existing empty-cell checks
reject malformed rows. Update the row handling around expectSeparator/cell
validation to ensure blank version rows surface an error, and add a
TestParseBOMVariantsTable case covering the all-blank row to lock in the
behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 2dadc471-b53d-442a-ae1e-512100a4ff33

📥 Commits

Reviewing files that changed from the base of the PR and between e1bd87a and df55122.

📒 Files selected for processing (14)

• .github/workflows/merge-gate.yaml
• docs/contributor/recipe.md
• docs/user/container-images.md
• pkg/bom/bom.go
• pkg/bom/markdown.go
• pkg/bom/variants.go
• pkg/bom/variants_test.go
• tools/bom/freshness_parse_test.go
• tools/bom/freshness_test.go
• tools/bom/main.go
• tools/bom/main_test.go
• tools/bom/variants.go
• tools/bom/variants_test.go
• tools/bom/variants_unix_test.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tools/bom/variants.go`:
- Around line 264-315: Make deriveVariants cancellation-aware by adding the
existing context parameter and checking ctx.Done() while iterating over sources
and refs, returning ctx.Err() when cancelled. Update its caller(s), including
the relevant command flow, to pass the context through without changing variant
aggregation behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 4be67163-ff6f-456f-8d51-4f9608df63fe

📥 Commits

Reviewing files that changed from the base of the PR and between df55122 and bfac8fb.

📒 Files selected for processing (14)

• .github/workflows/merge-gate.yaml
• docs/contributor/recipe.md
• docs/user/container-images.md
• pkg/bom/bom.go
• pkg/bom/markdown.go
• pkg/bom/variants.go
• pkg/bom/variants_test.go
• tools/bom/freshness_parse_test.go
• tools/bom/freshness_test.go
• tools/bom/main.go
• tools/bom/main_test.go
• tools/bom/variants.go
• tools/bom/variants_test.go
• tools/bom/variants_unix_test.go

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

tools/bom/variants.go (1)

264-341: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

deriveVariants still isn't cancellation-aware.

This traverses arbitrary -repo-root declarations (potentially large/nested overlay trees) but has no context.Context parameter and never checks for cancellation, unlike loadRecipeSources/walkYAML which already do. This was flagged in a previous review round and the fix was not applied to this function.

🔧 Proposed fix

-func deriveVariants(reg *registry, sources []recipeSource) ([]bom.VariantResult, error) {
+func deriveVariants(ctx context.Context, reg *registry, sources []recipeSource) ([]bom.VariantResult, error) {
 	byName := make(map[string]component, len(reg.Components))
 	for _, c := range reg.Components {
 		byName[c.Name] = c
 	}

 	agg := map[variantKey]map[string]struct{}{}
 	for _, src := range sources {
 		for _, ref := range src.Refs {
+			if ctxErr := ctx.Err(); ctxErr != nil {
+				return nil, errors.Wrap(errors.ErrCodeTimeout, "deriving BOM variants canceled", ctxErr)
+			}
 			pin := ref.Version

Also update the caller in tools/bom/main.go (deriveVariants(reg, sources) → deriveVariants(ctx, reg, sources)) and the test call sites in tools/bom/variants_test.go.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/bom/variants.go` around lines 264 - 341, Make deriveVariants
cancellation-aware by adding a context.Context parameter, checking ctx.Err()
while traversing sources and refs, and returning the cancellation error
promptly. Update its caller in main and every test invocation to pass the
existing context, preserving the current aggregation behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/bom/bom.go`:
- Around line 312-369: Extract the shared Helm property construction from
componentProperties and variantComponent into a helper that assembles
aicr:helm:repository, aicr:helm:chart, aicr:helm:version, and
aicr:helm:namespace using the existing component fields and omission rules.
Update both functions to call the helper, while keeping componentProperties’
Kustomize-specific property names and behavior intact.

In `@tools/bom/freshness_test.go`:
- Around line 402-490: Ensure parseBOMVariantsTable validates expectSeparator
before handling any non-table line and after the scan ends, returning the
existing invalid-delimiter error when a matched header is not immediately
followed by a valid delimiter row; do not silently reset
inTable/expectSeparator. Update the loop logic and add an end-of-function guard
so a header followed by blank/prose/heading or end-of-input is rejected.

---

Duplicate comments:
In `@tools/bom/variants.go`:
- Around line 264-341: Make deriveVariants cancellation-aware by adding a
context.Context parameter, checking ctx.Err() while traversing sources and refs,
and returning the cancellation error promptly. Update its caller in main and
every test invocation to pass the existing context, preserving the current
aggregation behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: c124cb74-b06f-4849-9f90-1566e0194eb3

📥 Commits

Reviewing files that changed from the base of the PR and between bfac8fb and 08621e3.

📒 Files selected for processing (14)

• .github/workflows/merge-gate.yaml
• docs/contributor/recipe.md
• docs/user/container-images.md
• pkg/bom/bom.go
• pkg/bom/markdown.go
• pkg/bom/variants.go
• pkg/bom/variants_test.go
• tools/bom/freshness_parse_test.go
• tools/bom/freshness_test.go
• tools/bom/main.go
• tools/bom/main_test.go
• tools/bom/variants.go
• tools/bom/variants_test.go
• tools/bom/variants_unix_test.go

[mchmarny]

Solid design and execution. The declaration-level scope (pins vs registry defaults, no effective-recipe resolution) is the right cut for #1611, and the implementation holds its fail-closed promises: strict mixin decode matching the store's KnownFields semantics, duplicate-source and padded-pin rejection, os.Root-confined size-bounded reads with the O_NONBLOCK/fstat trick against blocking FIFOs, and a checked bom-ref identity set so variants can't produce an ambiguous CycloneDX graph. I verified the .yaml-only filter matches the store's, surveyVariant's version swap is safe (Helm is a value struct, no registry aliasing), and the bidirectional freshness gate plus the merge-gate PASS-line assertion for both tests closes the rename-to-no-op hole. The variants-table parser is appropriately paranoid and well pinned by TestParseBOMVariantsTable.

Two inline comments, both low severity (a scan-scope assumption vs the store's full-tree walk that deserves an explicit doc note, and a minor fail-closed inconsistency for direct in-tree FIFOs) — neither blocks merge. Core CI (Test, Lint, bom-freshness, CodeQL) is green; E2E was still running at review time.