Summary
Follow-up hardening for the BOM version-freshness gate introduced in PR #1572 (which resolved #1424). The gate's core path works — a registry version bump that forgets make bom-docs now fails CI — but three edge cases remain where it can false-pass or misbehave. None block #1572; tracking them here.
F1 — Freshness test is one-way and duplicate-unsafe
TestCommittedBOMVersionsMatchRegistry (tools/bom/freshness_test.go) only checks registry → document. It never compares the reverse direction, so:
- Stale/removed rows go undetected. Removing a component from
recipes/registry.yaml while leaving its row in docs/user/container-images.md still passes.
- Duplicate rows, last-wins.
parseBOMVersionTable walks every pipe table matching the Component/Pinned Version headers and does out[name] = ver, so a later duplicate row silently overwrites the generated value. It parses the whole doc, not just the generated markers.
Fix: parse only the generated BOM section (between its markers), reject duplicate component rows, and compare the registry and document component sets bidirectionally before comparing versions.
F2 — Freshness test never runs on docs-only PRs
.github/workflows/merge-gate.yaml classifies a PR as code=false when every changed file is **.md/docs/**/LICENSE, routing it to tests-skip. A PR that edits only docs/user/container-images.md therefore never executes the freshness test, despite docs/contributor/recipe.md stating version freshness is gated. The intended registry-bump path is covered (it touches registry.yaml → code=true), so this is the narrower docs-only hand-edit hole.
Fix: add a lightweight path-filtered job that runs the freshness test when docs/user/container-images.md or recipes/registry.yaml change, or soften the doc's "gated" wording to match reality.
F4 — Not deployment-type-safe
- (a) Latent: the BOM generator (
tools/bom/main.go) emits Version only from Helm.DefaultVersion and never from Kustomize.DefaultTag, but the freshness test's expected value falls back to Kustomize.DefaultTag. The first Kustomize component carrying a defaultTag would fail the freshness test even after make bom-docs. No such component exists today, so it cannot trigger yet.
- (b) Hardening:
assertExemptionDefaultsInstalled (pkg/recipe/version_pin_guard_test.go) accepts ref.Version == def || ref.Tag == def. For a Helm component it should compare only the active field; an inactive Tag matching def could spoof the sole-consumer proof.
Fix: select the active version field by effective component type and reject the incompatible field, in both the generator/test expectation and the exemption proof.
F3 — Example recipes are guarded by neither gate
The hand-maintained example recipes in examples/recipes/*.yaml bypass overlay resolution, and neither new guard examines them: TestOverlayVersionPinsMatchRegistry scans only store.Base/store.Overlays/store.Mixins, and TestCommittedBOMVersionsMatchRegistry compares only the registry against docs/user/container-images.md. PR #1572 refreshed the examples by hand (nvsentinel, gpu-operator, AKS kube-prometheus-stack pins, and a stale driver.version override), so they are consistent today — but a future registry or overlay bump can leave the examples advertising a stale chart version while both gates stay green, reintroducing the exact drift class #1424 targets, on the examples surface.
Fix: cover examples/recipes/* with a guard or a regeneration/consistency check (e.g. resolve each example against the registry and fail on undeclared divergence), or explicitly document them as an unguarded illustrative snapshot boundary if that is the intended contract.
F5 — Base is not considered a standalone consumer in the sole-consumer proof
assertExemptionDefaultsInstalled (pkg/recipe/version_pin_guard_test.go) resolves only overlays with non-nil Spec.Criteria; base.yaml pins kube-prometheus-stack to the 84.4.0 registry default but carries no criteria, so it is excluded from the resolved results. The AKS exemption therefore stays satisfied only because a criteria-bearing non-AKS overlay inherits base's default. This holds today (and the test fails loudly if no overlay carries criteria), but the sole-consumer guarantee is coupled to overlay structure rather than stated explicitly.
Fix: either count store.Base as a standalone consumer in the proof, or add a code comment documenting why base is intentionally excluded and what invariant keeps the exemption honest. Related to F4(b) (active-field selection in the same proof).
Context
Summary
Follow-up hardening for the BOM version-freshness gate introduced in PR #1572 (which resolved #1424). The gate's core path works — a registry version bump that forgets
make bom-docsnow fails CI — but three edge cases remain where it can false-pass or misbehave. None block #1572; tracking them here.F1 — Freshness test is one-way and duplicate-unsafe
TestCommittedBOMVersionsMatchRegistry(tools/bom/freshness_test.go) only checks registry → document. It never compares the reverse direction, so:recipes/registry.yamlwhile leaving its row indocs/user/container-images.mdstill passes.parseBOMVersionTablewalks every pipe table matching theComponent/Pinned Versionheaders and doesout[name] = ver, so a later duplicate row silently overwrites the generated value. It parses the whole doc, not just the generated markers.Fix: parse only the generated BOM section (between its markers), reject duplicate component rows, and compare the registry and document component sets bidirectionally before comparing versions.
F2 — Freshness test never runs on docs-only PRs
.github/workflows/merge-gate.yamlclassifies a PR ascode=falsewhen every changed file is**.md/docs/**/LICENSE, routing it totests-skip. A PR that edits onlydocs/user/container-images.mdtherefore never executes the freshness test, despitedocs/contributor/recipe.mdstating version freshness is gated. The intended registry-bump path is covered (it touchesregistry.yaml→code=true), so this is the narrower docs-only hand-edit hole.Fix: add a lightweight path-filtered job that runs the freshness test when
docs/user/container-images.mdorrecipes/registry.yamlchange, or soften the doc's "gated" wording to match reality.F4 — Not deployment-type-safe
tools/bom/main.go) emitsVersiononly fromHelm.DefaultVersionand never fromKustomize.DefaultTag, but the freshness test's expected value falls back toKustomize.DefaultTag. The first Kustomize component carrying adefaultTagwould fail the freshness test even aftermake bom-docs. No such component exists today, so it cannot trigger yet.assertExemptionDefaultsInstalled(pkg/recipe/version_pin_guard_test.go) acceptsref.Version == def || ref.Tag == def. For a Helm component it should compare only the active field; an inactiveTagmatchingdefcould spoof the sole-consumer proof.Fix: select the active version field by effective component type and reject the incompatible field, in both the generator/test expectation and the exemption proof.
F3 — Example recipes are guarded by neither gate
The hand-maintained example recipes in
examples/recipes/*.yamlbypass overlay resolution, and neither new guard examines them:TestOverlayVersionPinsMatchRegistryscans onlystore.Base/store.Overlays/store.Mixins, andTestCommittedBOMVersionsMatchRegistrycompares only the registry againstdocs/user/container-images.md. PR #1572 refreshed the examples by hand (nvsentinel, gpu-operator, AKS kube-prometheus-stack pins, and a staledriver.versionoverride), so they are consistent today — but a future registry or overlay bump can leave the examples advertising a stale chart version while both gates stay green, reintroducing the exact drift class #1424 targets, on the examples surface.Fix: cover
examples/recipes/*with a guard or a regeneration/consistency check (e.g. resolve each example against the registry and fail on undeclared divergence), or explicitly document them as an unguarded illustrative snapshot boundary if that is the intended contract.F5 — Base is not considered a standalone consumer in the sole-consumer proof
assertExemptionDefaultsInstalled(pkg/recipe/version_pin_guard_test.go) resolves only overlays with non-nilSpec.Criteria;base.yamlpins kube-prometheus-stack to the84.4.0registry default but carries no criteria, so it is excluded from the resolvedresults. The AKS exemption therefore stays satisfied only because a criteria-bearing non-AKS overlay inherits base's default. This holds today (and the test fails loudly if no overlay carries criteria), but the sole-consumer guarantee is coupled to overlay structure rather than stated explicitly.Fix: either count
store.Baseas a standalone consumer in the proof, or add a code comment documenting why base is intentionally excluded and what invariant keeps the exemption honest. Related to F4(b) (active-field selection in the same proof).Context