fix(bom): reject bare scalars without tag, digest, or registry host#776
Merged
Conversation
Closes #765. Tightens \`pkg/bom.isLikelyImage\` so chart-default placeholders like \`vgpu-manager\` (lifted from \`vgpuManager: { image: vgpu-manager, enabled: false }\` in upstream charts) no longer dilute the published BOM. Before: the helm-rendered output of \`gpu-operator\` produced a bare \`- vgpu-manager\` entry in docs/user/container-images.md — no registry, no tag, no digest. A reader can't tell what bytes that deploys, and downstream tooling (#749 digest-pinning, #745 admission verification) would have nothing to attest against. After: \`isLikelyImage\` requires at least one of: - a registry host as the first path segment (contains \`.\`/\`:\` or is \`localhost\`), - a \`:tag\` after the last \`/\`, - an \`@<digest>\` suffix. Bare single-segment scalars are rejected. Tag-only single-segment refs (\`busybox:1.36\`) still pass; their \`library/\` canonicalization in ParseImageRef is unaffected because canonicalization runs at parse time, not extraction time. BOM doc image count drops from 70 → 69 (one bare placeholder removed). All other entries unchanged. \`make bom-docs\` is idempotent. The deeper fix the issue mentioned — extending the regression test to walk \`make bom\`'s helm-rendered output (against a kwok cluster?) — is intentionally not in this PR. That's a meaningful test-infrastructure addition; the behavioral fix here is small enough to land cleanly on its own and unblock #749.
Contributor
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Addresses CodeRabbit review on PR #776. Adds \`nvidia/cuda\` to the rejected set in TestIsLikelyImage to lock in that the boundary covers two-segment refs without tag/digest/host, not just single-segment. The intended distinction is: ParseImageRef applies the \`library/\` fallback only to true single-segment refs at parse time, so \`isLikelyImage\` rejecting a \`namespace/name\` shape without further signal is consistent with the policy — the BOM extractor should not emit an image whose registry/tag/digest is left to downstream inference.
Contributor
Coverage Report ✅
Coverage BadgeMerging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
lalitadithya
approved these changes
May 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
Closes #765. Tightens
pkg/bom.isLikelyImageso chart-default placeholders likevgpu-manager(lifted fromvgpuManager: { image: vgpu-manager, enabled: false }in upstream charts) stop appearing in the published BOM.Refs #739, #749 (unblocks digest-pinning by ensuring there are no bare placeholders to try to resolve).
Type of Change
Component(s) Affected
pkg/bom)docs/user/container-images.md— auto-regenerated)Implementation Notes
Before. The helm-rendered output of
gpu-operatorincluded a bare- vgpu-managerentry indocs/user/container-images.md— no registry, no tag, no digest. The CRD-style triplet collector picked upimage: vgpu-managerfrom a section whoseenabled: falsemade the image moot, but the extractor had no way to tell.After.
isLikelyImagenow requires at least one of:./:or islocalhost),:tagafter the last/,@<digest>suffix.Bare single-segment scalars without any of those signals are rejected. Tag-only single-segment refs (
busybox:1.36) still pass; theirlibrary/canonicalization inParseImageRefis unaffected because canonicalization runs at parse time, not extraction time.BOM doc: image count 70 → 69 (one bare placeholder removed); auto-regenerated by
make bom-docs.make bom-docsis idempotent (md5 stable across runs).Out of scope. The issue mentioned extending the regression test to walk
make bom's helm-rendered output (against a kwok cluster) so this class of drift fails in CI rather than only surfacing on manual inspection. That's a meaningful test-infrastructure addition; this PR ships the behavioral fix small enough to land cleanly on its own and unblock #749.Testing
pkg/bomcoverage held with expandedTestIsLikelyImagecases (registry host / tag-only / digest-only / bare-scalar) and updated case forvgpu-manageranddriverregression scenarios.Risk Assessment
vgpu-managerplaceholder. Easy to revert.Rollout notes: Future contributors who introduce new components with chart sections that have bare placeholder images will see those entries excluded from the BOM doc by design. If a real bare-name image is ever needed (rare in production K8s manifests), it would need a tag, digest, or registry-host first segment.
Checklist
make testwith-race)make lint)git commit -S)