fix(recipes): guard overlay version pins against registry defaults#1572
Merged
Conversation
mchmarny
added
the
theme/recipes
Contributor
Member
Author
|
@CodeRabbit full review |
Contributor
Recipe evidence check
Protected recipesRecipes with committed evidence (
Other affected recipes without evidence yet: 63These recipes are affected by this PR but carry no committed evidence pointer, so there is
How to refresh evidenceRun on a cluster matching the recipe's aicr snapshot -o snapshot.yaml
aicr validate \
-r recipes/overlays/<slug>.yaml \
-s snapshot.yaml \
--emit-attestation ./out \
--push ghcr.io/<your-fork>/aicr-evidence
# Copy to the per-source path printed in the emit 'copyTo' hint:
# recipes/evidence/<slug>/<source>/<bundle-digest>.yamlThis gate is warning-only and never blocks merge. See ADR-007 for the trust model. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds CI tests to compare recipe version pins and the committed BOM against registry defaults, with exemption handling for intentional divergence. It also updates several recipe YAML files to new component versions and revises contributor docs to describe the registry/defaultVersion and version-pinning rules. Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Coverage Report ✅
Coverage BadgeNo Go source files changed in this PR. |
mchmarny
force-pushed
the
fix/1424-bom-version-pin-guard
branch
from
5c7e98f to
e1eab34
Compare
yuanchen8911
previously requested changes
Jul 1, 2026
yuanchen8911
left a comment
yuanchen8911
left a comment
Contributor
There was a problem hiding this comment.
The version-pin guard works for the current embedded Helm catalog, but does not fully enforce #1424's end-to-end acceptance criteria. Details inline: finding 1 is a merge blocker if this PR carries Fixes #1424; finding 2 should be addressed or explicitly scoped out; finding 3 is non-blocking follow-up.
mchmarny
force-pushed
the
fix/1424-bom-version-pin-guard
branch
2 times, most recently
from
cd10e03 to
5c1b675
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@docs/contributor/recipe.md`:
- Around line 479-489: Promote the bold “Version freshness *is* gated;
image-drift freshness is not.” label in contributor/recipe.md to a proper
third-level heading because it introduces a substantive standalone topic with
multiple explanatory sentences. Update the surrounding section so this text uses
the same heading structure as the nearby “Version pinning is single-source”
section, and keep the explanatory paragraphs beneath it unchanged but nested
under the new heading.
In `@tools/bom/freshness_test.go`:
- Around line 100-158: The table parser in parseBOMVersionTable can miss a valid
BOM table if a non-header pipe row appears before the real Component/Pinned
Version header after a reset. Update the header detection logic so the function
keeps searching for the header row until both columns are found, rather than
locking into the first pipe-prefixed row; keep the existing inTable reset
behavior and preserve the column lookup by header text.
🪄 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: 055662bd-d99a-4299-93a5-ddd4fbefec32
📒 Files selected for processing (7)
docs/contributor/recipe.mdexamples/recipes/aks-training.yamlexamples/recipes/eks-gb200-ubuntu-training-with-validation.yamlexamples/recipes/eks-training.yamlpkg/recipe/version_pin_guard_test.gorecipes/overlays/base.yamltools/bom/freshness_test.go
The container-images BOM renders each chart at its registry defaultVersion, but at resolution that default is only a fallback: a componentRef version/tag pin in base.yaml, an overlay, or a mixin overrides it. So the BOM reflects what recipes install only when no pin diverges. A pin that drifts from the default (most dangerously on a component whose sole consumer diverges) makes the BOM advertise a version no recipe installs -- the #1418 aws-efa class of bug. Enforce both halves of #1424's acceptance: - TestOverlayVersionPinsMatchRegistry (pkg/recipe): every base/overlay/mixin componentRefs version/tag must equal the component's registry default unless declared in versionPinExemptions with a justification. Each exemption binds one divergence via expectedPin/expectedDefault, so a later move of either the pin or the default re-triggers review. It also resolves every overlay and fails if an exemption's registry default is installed by no recipe (the sole-consumer policy). Fails on undeclared drift, stale/unsafe exemptions, and vacuous discovery. - TestCommittedBOMVersionsMatchRegistry (tools/bom): the committed docs/user/container-images.md version column must match the registry pins. This catches a coordinated bump (registry + pins moved together) that forgot make bom-docs -- which the recipe guard alone would pass. Version-only diff, no Helm rendering, runs under make test. Both run under make test -> make qualify. Fix the one live divergence caught: base.yaml pinned nvsentinel v1.3.0 while the registry default (and BOM) advertised v1.9.0 -- every recipe shipped v1.3.0. Align base to v1.9.0 so reality matches the BOM. Record the intentional aks kube-prometheus-stack 83.7.0 divergence (#700) as an exemption. Realign stale pins in examples/recipes/{eks-training,aks-training, eks-gb200-ubuntu-training-with-validation}.yaml to resolved output (gpu-operator v26.3.2, nvsentinel v1.9.0, AKS kube-prometheus-stack 83.7.0), preserving their curated form. Document the single-source version model and the two gates in docs/contributor/recipe.md. Fixes: #1424 Signed-off-by: Mark Chmarny <[email protected]>
mchmarny
force-pushed
the
fix/1424-bom-version-pin-guard
branch
from
5c1b675 to
f881fd4
Compare
yuanchen8911
dismissed
their stale review
Resolved on the updated PR. The three original findings are addressed: BOM freshness is now gated by TestCommittedBOMVersionsMatchRegistry (tools/bom), the example recipes were refreshed to their resolved versions, and the exemption sole-consumer rule is enforced by assertExemptionDefaultsInstalled. #1424's acceptance criteria are met. Remaining freshness-gate edge cases (bidirectional BOM parsing, docs-only PRs skipping the test, Kustomize type-safety) are non-blocking and will be tracked as a separate hardening follow-up. One concrete item posted separately below.
yuanchen8911
reviewed
Jul 1, 2026
| type: Helm | ||
| source: oci://ghcr.io/nvidia | ||
| version: v1.3.0 | ||
| version: v1.9.0 |
Contributor
There was a problem hiding this comment.
One more stale value in this refreshed example, separate from the chart-version pins: further up (around lines 119-122) it still sets overrides.driver.version: 580.82.07, while the current base gpu-operator pin is 580.126.20 (recipes/components/gpu-operator/values.yaml:152) — the driver version the BOM advertises. Bundling this example therefore installs a driver version nothing else ships. This is a nested image override rather than a registry/chart pin, so the chart-version refresh in this PR didn't touch it.
Recommended: resolve the GB200 criteria against the current recipes and, if it produces 580.126.20, drop this override (or update it to match). Non-blocking, but cheap to fix while this file is already being refreshed.
Member
Author
There was a problem hiding this comment.
Good catch — dropped the stale driver.version: 580.82.07 override (04cd0e4). Confirmed no gb200 overlay in the resolution chain (gb200-eks-ubuntu-training → gb200-eks-training → eks-training) sets driver.version; it inherits the base gpu-operator values pin 580.126.20 (recipes/components/gpu-operator/values.yaml:152), which is what the BOM advertises. Rather than update the literal, I removed the override entirely to match resolved output — sibling examples (eks-training.yaml, aks-training.yaml) carry no driver.version override either; the version comes from the referenced valuesFile. Recipe + BOM tests still green, yamllint clean.
yuanchen8911
requested changes
Jul 1, 2026
yuanchen8911
left a comment
yuanchen8911
left a comment
Contributor
There was a problem hiding this comment.
Version-management model and guard tests look correct — both new tests compile, lint clean, and pass, the version bumps are internally consistent with the registry defaults, and the AKS kube-prometheus-stack divergence is covered by a well-formed exemption. A few items to address before merge:
examples/recipes/*.yaml are guarded by neither new test. TestOverlayVersionPinsMatchRegistry iterates only base/overlays/mixins (store.Base, store.Overlays, store.Mixins), and TestCommittedBOMVersionsMatchRegistry checks only the registry against the committed doc. The persisted examples are updated by hand in this PR, so they are consistent now, but a future registry or overlay bump can leave the examples advertising a stale chart version while both guards stay green — the same drift class #1424 targets, on the examples surface. Please either extend the guard (or a regeneration/consistency check) to cover examples/recipes/, or file a tracked follow-up.
assertExemptionDefaultsInstalled excludes base.yaml as a standalone consumer. The sole-consumer check resolves only overlays with non-nil Spec.Criteria; base.yaml pins kube-prometheus-stack to the 84.4.0 default but has no criteria, so it is not in results. The AKS exemption stays satisfied only because a criteria-bearing non-AKS overlay inherits base's 84.4.0 default. This holds today (and the test fails loudly if no overlay carries criteria), but the guarantee depends on overlay structure. Worth a comment documenting the coupling.
PR description omits two example bumps. The Implementation Notes cover only the nvsentinel bump and the AKS exemption, but the diff also bumps the example gpu-operator (v25.10.1 -> v26.3.2) and AKS kube-prometheus-stack (84.4.0 -> 83.7.0). The changes are correct; please note them in the description so reviewers reading the summary know examples were touched.
The gb200 example pinned driver.version 580.82.07 via a nested gpu-operator override, but no gb200 overlay in the resolution chain sets driver.version — it inherits the base gpu-operator values pin (580.126.20). Bundling the example therefore installed a driver version nothing else ships. Sibling examples (eks-training, aks-training) carry no driver.version override; the value comes from the referenced valuesFile. Drop the stale override to match resolved output. Signed-off-by: Mark Chmarny <[email protected]>
The Common Pitfalls bullet claimed a chart pin or values change goes stale silently and never surfaces in qualify. That now contradicts the new 'Version freshness is gated; image drift is not' section: a stale version column fails CI via TestCommittedBOMVersionsMatchRegistry. Scope the pitfall to image drift (values change without a pin bump), which remains ungated, and link to the authoritative section. Signed-off-by: Yuan Chen <[email protected]>
yuanchen8911
approved these changes
Jul 1, 2026
yuanchen8911
left a comment
yuanchen8911
left a comment
Contributor
There was a problem hiding this comment.
Approving. The version-management model and both guard tests are correct — they compile, lint clean, pass, and version pins align with registry defaults; CI coverage is above the floor. The stale gb200 driver.version override is resolved (the example now inherits the chart default), and the review findings are addressed.
Two housekeeping items before merge:
-
Please rebase onto origin/main and squash before merging — the branch currently carries several merge-from-main commits plus a follow-up doc-consistency commit. A single squashed, rebased commit keeps history clean per repo policy (not the "Update branch"/merge path).
-
The example recipes (examples/recipes/*) bypass overlay resolution and are not covered by either new guard; they're kept consistent by hand in this PR. Let's track guarding them (or a regeneration check) in a follow-up issue so the drift class can't silently return there.
Non-blocking: the example chart-version bumps are now noted in the PR description, and the Common Pitfalls doc bullet was reconciled with the new version-freshness gating section.
13 tasks
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. F1 — make the committed-BOM check an exact, bidirectional projection. TestCommittedBOMVersionsMatchRegistry previously checked only registry -> doc and scanned every pipe table in the file, so a stale row for a removed component passed and a duplicate row could shadow the generated value. Now it parses only the generated AICR-BOM marker section, rejects duplicate component rows, and compares the registry and doc component sets bidirectionally before comparing versions. F2 — run the freshness assertion on docs-only PRs, and never let it no-op. The merge gate classifies Markdown/docs-only changes as non-code and skips the full tests job, so a PR editing only docs/user/container-images.md never ran the check. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed (a `go test -run` that matches nothing exits 0), so a future rename/removal fails the gate instead of silently passing. F4 — make version handling deployment-type-safe end to end. The BOM generator emits the pinned version by effective component type (Helm defaultVersion or Kustomize defaultTag); pkg/bom names the CycloneDX property accordingly (aicr:kustomize:tag vs aicr:helm:version) so a Kustomize Git tag is not mislabeled as a Helm version. The exemption sole-consumer proof selects the active field by the registry type and requires the resolved ref's type to match, so a ref whose type was overridden in an overlay cannot satisfy the proof via an inactive inherited field. Adds unit tests for the marker extraction, the table parser, and the type-named version property, and updates docs/contributor/recipe.md to describe the bidirectional check and the docs-only gate coverage.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. F1 — make the committed-BOM check an exact, bidirectional projection. TestCommittedBOMVersionsMatchRegistry previously checked only registry -> doc and scanned every pipe table in the file, so a stale row for a removed component passed and a duplicate row could shadow the generated value. Now it parses only the generated AICR-BOM marker section, rejects duplicate component rows, and compares the registry and doc component sets bidirectionally before comparing versions. F2 — run the freshness assertion on docs-only PRs, and never let it no-op. The merge gate classifies Markdown/docs-only changes as non-code and skips the full tests job, so a PR editing only docs/user/container-images.md never ran the check. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed (a `go test -run` that matches nothing exits 0), so a future rename/removal fails the gate instead of silently passing. F4 — make version handling deployment-type-safe end to end. The BOM generator emits the pinned version by effective component type (Helm defaultVersion or Kustomize defaultTag); pkg/bom names the CycloneDX property accordingly (aicr:kustomize:tag vs aicr:helm:version) so a Kustomize Git tag is not mislabeled as a Helm version. The exemption sole-consumer proof selects the active field by the registry type and requires the resolved ref's type to match, so a ref whose type was overridden in an overlay cannot satisfy the proof via an inactive inherited field. Adds unit tests for the marker extraction, the table parser, and the type-named version property, and updates docs/contributor/recipe.md to describe the bidirectional check and the docs-only gate coverage.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. F1 — make the committed-BOM check an exact, bidirectional projection. TestCommittedBOMVersionsMatchRegistry previously checked only registry -> doc and scanned every pipe table in the file, so a stale row for a removed component passed and a duplicate row could shadow the generated value. Now it parses only the generated AICR-BOM marker section, rejects duplicate component rows, and compares the registry and doc component sets bidirectionally before comparing versions. F2 — run the freshness assertion on docs-only PRs, and never let it no-op. The merge gate classifies Markdown/docs-only changes as non-code and skips the full tests job, so a PR editing only docs/user/container-images.md never ran the check. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed (a `go test -run` that matches nothing exits 0), so a future rename/removal fails the gate instead of silently passing. F4 — make version handling deployment-type-safe end to end. The BOM generator emits the pinned version by effective component type (Helm defaultVersion or Kustomize defaultTag); pkg/bom names the CycloneDX property accordingly (aicr:kustomize:tag vs aicr:helm:version) so a Kustomize Git tag is not mislabeled as a Helm version. The exemption sole-consumer proof selects the active field by the registry type and requires the resolved ref's type to match, so a ref whose type was overridden in an overlay cannot satisfy the proof via an inactive inherited field. Adds unit tests for the marker extraction, the table parser, and the type-named version property, and updates docs/contributor/recipe.md to describe the bidirectional check and the docs-only gate coverage.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. Freshness check (tools/bom/freshness_test.go) — make the committed BOM an exact registry projection: parse only the generated AICR-BOM marker section (requiring exactly one begin/end pair, so a second stale section cannot be appended), reject duplicate component rows, compare the registry and doc component sets bidirectionally, and check EVERY row — pinned components by effective type, unpinned components against the "—" sentinel so a fabricated version on an unpinned row cannot slip through. Merge gate (.github/workflows/merge-gate.yaml) — run the freshness check on docs-only PRs, which skip the full tests job. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed, so a future rename/removal fails the gate instead of silently passing (`go test -run` that matches nothing exits 0). Type-safe BOM metadata (tools/bom/main.go, pkg/bom/bom.go, pkg/evidence/attestation/bom.go) — the standalone generator and the recipe-bound attestation builder both emit the pinned version and source by effective component type (Helm defaultVersion/repository or Kustomize defaultTag/source), and pkg/bom names the CycloneDX properties by type (aicr:kustomize:tag / aicr:kustomize:source vs aicr:helm:*, matched case-insensitively) so a Kustomize Git tag/source is not mislabeled as Helm metadata. Strict pinning stays Helm-only per ADR-006. Exemption sole-consumer proof (pkg/recipe/version_pin_guard_test.go) — select the active field by the registry type and fail closed when a resolved ref's declared type OR its deploy shape (a Tag/Path that the deployers classify as Kustomize) is inconsistent with the registry type, so a ref cannot satisfy the proof via an inactive inherited field. Adds unit tests for marker extraction (including duplicate markers), the table parser, and the type-named version/source properties, and updates docs/contributor/recipe.md.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. Freshness check (tools/bom/freshness_test.go) — make the committed BOM an exact registry projection: parse only the generated AICR-BOM marker section (requiring exactly one begin/end pair, so a second stale section cannot be appended), reject duplicate component rows, compare the registry and doc component sets bidirectionally, and check EVERY row — pinned components by effective type, unpinned components against the "—" sentinel so a fabricated version on an unpinned row cannot slip through. Merge gate (.github/workflows/merge-gate.yaml) — run the freshness check on docs-only PRs, which skip the full tests job. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed, so a future rename/removal fails the gate instead of silently passing (`go test -run` that matches nothing exits 0). Type-safe BOM metadata (tools/bom/main.go, pkg/bom/bom.go, pkg/evidence/attestation/bom.go) — the standalone generator and the recipe-bound attestation builder both emit the pinned version and source by effective component type (Helm defaultVersion/repository or Kustomize defaultTag/source), and pkg/bom names the CycloneDX properties by type (aicr:kustomize:tag / aicr:kustomize:source vs aicr:helm:*, matched case-insensitively) so a Kustomize Git tag/source is not mislabeled as Helm metadata. Strict pinning stays Helm-only per ADR-006. Exemption sole-consumer proof (pkg/recipe/version_pin_guard_test.go) — select the active field by the registry type and fail closed when a resolved ref's declared type OR its deploy shape (a Tag/Path that the deployers classify as Kustomize) is inconsistent with the registry type, so a ref cannot satisfy the proof via an inactive inherited field. Adds unit tests for marker extraction (including duplicate markers), the table parser, and the type-named version/source properties, and updates docs/contributor/recipe.md.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. Freshness check (tools/bom/freshness_test.go) — make the committed BOM an exact registry projection: parse only the generated AICR-BOM marker section (requiring exactly one begin/end pair, so a second stale section cannot be appended), reject duplicate component rows, compare the registry and doc component sets bidirectionally, and check EVERY row — pinned components by effective type, unpinned components against the "—" sentinel so a fabricated version on an unpinned row cannot slip through. Merge gate (.github/workflows/merge-gate.yaml) — run the freshness check on docs-only PRs, which skip the full tests job. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed, so a future rename/removal fails the gate instead of silently passing (`go test -run` that matches nothing exits 0). Type-safe BOM metadata (tools/bom/main.go, pkg/bom/bom.go, pkg/evidence/attestation/bom.go) — the standalone generator and the recipe-bound attestation builder both emit the pinned version and source by effective component type (Helm defaultVersion/repository or Kustomize defaultTag/source), and pkg/bom names the CycloneDX properties by type (aicr:kustomize:tag / :source / :namespace vs aicr:helm:*, matched case-insensitively) so Kustomize metadata is not mislabeled as Helm. Strict pinning stays Helm-only per ADR-006. Exemption sole-consumer proof (pkg/recipe/version_pin_guard_test.go) — select the active field by the registry type and fail closed when a resolved ref's declared type OR its deploy shape (a Tag/Path that the deployers classify as Kustomize, or a Kustomize ref lacking the Path localformat requires) is inconsistent with the registry type, so a ref cannot satisfy the proof via an inactive inherited field. Adds unit tests for marker extraction (including duplicate markers), the table parser, and the type-named version/source/namespace properties. Reconciles the BOM-freshness guidance across docs/contributor/recipe.md, docs/contributor/tests.md, ADR-006, and .claude/CLAUDE.md / AGENTS.md: name/version and component-set freshness is now gated; only rendered-image drift and full `make bom-check` remain opt-in.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. Freshness check (tools/bom/freshness_test.go) — make the committed BOM an exact registry projection: parse only the generated AICR-BOM marker section (requiring exactly one begin/end pair, so a second stale section cannot be appended), reject duplicate component rows, compare the registry and doc component sets bidirectionally, and check EVERY row — pinned components by effective type, unpinned components against the "—" sentinel so a fabricated version on an unpinned row cannot slip through. Merge gate (.github/workflows/merge-gate.yaml) — run the freshness check on docs-only PRs, which skip the full tests job. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed, so a future rename/removal fails the gate instead of silently passing (`go test -run` that matches nothing exits 0). Type-safe BOM metadata (tools/bom/main.go, pkg/bom/bom.go, pkg/evidence/attestation/bom.go) — the standalone generator and the recipe-bound attestation builder both emit the pinned version and source by effective component type (Helm defaultVersion/repository or Kustomize defaultTag/source), and pkg/bom names the CycloneDX properties by type (aicr:kustomize:tag / :source / :namespace vs aicr:helm:*, matched case-insensitively) so Kustomize metadata is not mislabeled as Helm. Strict pinning stays Helm-only per ADR-006. Exemption sole-consumer proof (pkg/recipe/version_pin_guard_test.go) — select the active field by the registry type and fail closed when a resolved ref's declared type OR its deploy shape (a Tag/Path that the deployers classify as Kustomize, or a Kustomize ref lacking the Path localformat requires) is inconsistent with the registry type, so a ref cannot satisfy the proof via an inactive inherited field. Adds unit tests for marker extraction (including duplicate markers), the table parser, and the type-named version/source/namespace properties. Reconciles the BOM-freshness guidance across docs/contributor/recipe.md, docs/contributor/tests.md, ADR-006, and .claude/CLAUDE.md / AGENTS.md: name/version and component-set freshness is now gated; only rendered-image drift and full `make bom-check` remain opt-in.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. Freshness check (tools/bom/freshness_test.go) — make the committed BOM an exact registry projection: parse only the generated AICR-BOM marker section (requiring exactly one begin/end pair, so a second stale section cannot be appended), reject duplicate component rows, compare the registry and doc component sets bidirectionally, and check EVERY row — pinned components by effective type, unpinned components against the "—" sentinel so a fabricated version on an unpinned row cannot slip through. Merge gate (.github/workflows/merge-gate.yaml) — run the freshness check on docs-only PRs, which skip the full tests job. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed, so a future rename/removal fails the gate instead of silently passing (`go test -run` that matches nothing exits 0). Type-safe BOM metadata (tools/bom/main.go, pkg/bom/bom.go, pkg/evidence/attestation/bom.go) — the standalone generator and the recipe-bound attestation builder both emit the pinned version and source by effective component type (Helm defaultVersion/repository or Kustomize defaultTag/source), and pkg/bom names the CycloneDX properties by type (aicr:kustomize:tag / :source / :namespace vs aicr:helm:*, matched case-insensitively) so Kustomize metadata is not mislabeled as Helm. Strict pinning stays Helm-only per ADR-006. Exemption sole-consumer proof (pkg/recipe/version_pin_guard_test.go) — select the active field by the registry type and fail closed when a resolved ref's declared type OR its deploy shape (a Tag/Path that the deployers classify as Kustomize, or a Kustomize ref lacking the Path localformat requires) is inconsistent with the registry type, so a ref cannot satisfy the proof via an inactive inherited field. Adds unit tests for marker extraction (including duplicate markers), the table parser, and the type-named version/source/namespace properties. Reconciles the BOM-freshness guidance across docs/contributor/recipe.md, docs/contributor/tests.md, ADR-006, and .claude/CLAUDE.md / AGENTS.md: name/version and component-set freshness is now gated; only rendered-image drift and full `make bom-check` remain opt-in.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. Freshness check (tools/bom/freshness_test.go) — make the committed BOM an exact registry projection: parse only the generated AICR-BOM marker section (requiring exactly one begin/end pair, so a second stale section cannot be appended), reject duplicate component rows, compare the registry and doc component sets bidirectionally, and check EVERY row — pinned components by effective type, unpinned components against the "—" sentinel so a fabricated version on an unpinned row cannot slip through. Merge gate (.github/workflows/merge-gate.yaml) — run the freshness check on docs-only PRs, which skip the full tests job. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed, so a future rename/removal fails the gate instead of silently passing (`go test -run` that matches nothing exits 0). Type-safe BOM metadata (tools/bom/main.go, pkg/bom/bom.go, pkg/evidence/attestation/bom.go) — the standalone generator and the recipe-bound attestation builder both emit the pinned version and source by effective component type (Helm defaultVersion/repository or Kustomize defaultTag/source), and pkg/bom names the CycloneDX properties by type (aicr:kustomize:tag / :source / :namespace vs aicr:helm:*, matched case-insensitively) so Kustomize metadata is not mislabeled as Helm. Strict pinning stays Helm-only per ADR-006. Exemption sole-consumer proof (pkg/recipe/version_pin_guard_test.go) — select the active field by the registry type and fail closed when a resolved ref's declared type OR its deploy shape (a Tag/Path that the deployers classify as Kustomize, or a Kustomize ref lacking the Path localformat requires) is inconsistent with the registry type, so a ref cannot satisfy the proof via an inactive inherited field. Adds unit tests for marker extraction (including duplicate markers), the table parser, and the type-named version/source/namespace properties. Reconciles the BOM-freshness guidance across docs/contributor/recipe.md, docs/contributor/tests.md, ADR-006, and .claude/CLAUDE.md / AGENTS.md: name/version and component-set freshness is now gated; only rendered-image drift and full `make bom-check` remain opt-in.
yuanchen8911
added a commit
to yuanchen8911/aicr
that referenced
this pull request
Jul 1, 2026
Follow-up hardening for the version-freshness gate added in NVIDIA#1572. The gate's core path (a registry bump that forgets `make bom-docs` fails CI) works; this closes edge cases where it could false-pass or misbehave. Freshness check (tools/bom/freshness_test.go) — make the committed BOM an exact registry projection: parse only the generated AICR-BOM marker section (requiring exactly one begin/end pair, so a second stale section cannot be appended), reject duplicate component rows, compare the registry and doc component sets bidirectionally, and check EVERY row — pinned components by effective type, unpinned components against the "—" sentinel so a fabricated version on an unpinned row cannot slip through. Merge gate (.github/workflows/merge-gate.yaml) — run the freshness check on docs-only PRs, which skip the full tests job. Add a path-filtered bom-freshness job (gated on the BOM doc, registry, and generator sources) plus its skip companion, wired into the aggregate gate. The job asserts the named test actually executed, so a future rename/removal fails the gate instead of silently passing (`go test -run` that matches nothing exits 0). Type-safe BOM metadata (tools/bom/main.go, pkg/bom/bom.go, pkg/evidence/attestation/bom.go) — the standalone generator and the recipe-bound attestation builder both emit the pinned version and source by effective component type (Helm defaultVersion/repository or Kustomize defaultTag/source), and pkg/bom names the CycloneDX properties by type (aicr:kustomize:tag / :source / :namespace vs aicr:helm:*, matched case-insensitively) so Kustomize metadata is not mislabeled as Helm. Strict pinning stays Helm-only per ADR-006. Exemption sole-consumer proof (pkg/recipe/version_pin_guard_test.go) — select the active field by the registry type and fail closed when a resolved ref's declared type OR its deploy shape (a Tag/Path that the deployers classify as Kustomize, or a Kustomize ref lacking the Path localformat requires) is inconsistent with the registry type, so a ref cannot satisfy the proof via an inactive inherited field. Adds unit tests for marker extraction (including duplicate markers), the table parser, and the type-named version/source/namespace properties. Reconciles the BOM-freshness guidance across docs/contributor/recipe.md, docs/contributor/tests.md, ADR-006, and .claude/CLAUDE.md / AGENTS.md: name/version and component-set freshness is now gated; only rendered-image drift and full `make bom-check` remain opt-in.
This was referenced Jul 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The container-images BOM renders each chart at its registry
defaultVersion, but acomponentRefversion/tag pin inbase.yaml/an overlay/a mixin overrides that default at resolution. Add a guard test so an overlay pin can no longer silently diverge from the registry default (which the BOM reads), and fix the one live divergence it caught.Motivation / Context
The BOM (
docs/user/container-images.md) is the user-facing source of truth for shipped chart/image versions, yet it renders from the registrydefaultVersion— a fallback that overlay/mixinversionpins override. So the BOM equals what recipes actually install only when no pin diverges. When a component's sole consumer diverges, the registry default is installed by zero recipes and the BOM is pure fiction (the #1418aws-efabug).The guard caught a live instance on
main:base.yamlpinnednvsentinel v1.3.0while the registry default — and the committed BOM (6 images) — advertisedv1.9.0. Every recipe shipped v1.3.0.Fixes: #1424
Related: #1418
Type of Change
make test→make qualify)Component(s) Affected
pkg/recipe)docs/,examples/)Implementation Notes
pkg/recipe/version_pin_guard_test.go—TestOverlayVersionPinsMatchRegistryasserts everybase/overlay/mixincomponentRefs[].version(Helm) /.tag(Kustomize) equals the component's registry default, unless declared inversionPinExemptionswith a justification. Also fails on stale exemptions (pin re-aligned) and vacuous discovery (no pins found). ReusesloadMetadataStore+GetComponentRegistry; runs undermake test, so no new gate wiring.recipes/overlays/base.yaml— nvsentinelv1.3.0 → v1.9.0. This changes what every recipe deploys; the BOM already advertised v1.9.0, so this makes reality match the BOM (regeneratedmake bom-docs→ zero diff).aks/kube-prometheus-stack 83.7.0(intentional, validated cluster state in feat(recipes): update AKS H100 Dynamo recipe to match working cluster state #700; the registry default 84.4.0 is still installed by every non-AKS recipe, so the BOM's default is not fictional).examples/recipes/*.yaml) — refreshed the hand-maintained example snapshots so their pins match the current registry defaults: nvsentinelv1.3.0 → v1.9.0, gpu-operatorv25.10.1 → v26.3.2, and (AKS) kube-prometheus-stack84.4.0 → 83.7.0, plus dropping a staledriver.version: 580.82.07override in the gb200 example. Examples bypass overlay resolution and are not covered by the new guards (a follow-up will track guarding them); these edits keep them consistent for now.docs/contributor/recipe.md.Testing
Guard verified RED (caught the live nvsentinel divergence) before the fix, GREEN after.
Risk Assessment
v1.3.0 → v1.9.0changes the deployed chart for every recipe.Rollout notes: The registry default and the rendered BOM were already v1.9.0; this aligns the recipes to match. nvsentinel values render cleanly at v1.9.0 (BOM regen produced no diff). Easy to revert (single-line pin + test).
Checklist
make testwith-race)make lint)git commit -S)