fix(k8s): name artifacts by image reference when the runtime reports an image ID

[dangrondahl]

Closes #1203.

NewPodData keyed the digests map on cs.Image, the name the container runtime reports. containerd names an image record by the reference it was pulled with, so an image pulled by digest has no tagged name in the store and the runtime reports the image ID instead — and the artifact ends up named after a bare sha256:, with nothing to say which image it is.

The pod still carries readable references when that happens, so artifactName now falls back, most informative first:

1. the runtime's own name, when it has one — unchanged behaviour for every tag-pulled image
2. this container's image in the pod spec, normalized to the form the runtime would have reported had it held the image under its tag, e.g. docker.io/library/nginx:1.27
3. the image ID — keeps at least the repository, e.g. docker.io/library/nginx@sha256:…

Normalizing rather than taking the spec reference verbatim means one image gets one name: a digest-pinned pod on a node with a cold image cache and the same pod on a warm one now agree. The digest is dropped from the name because the fingerprint already records it, and the two are independent values that can disagree while an in-place image update is rolling. internal/cloudrun resolves digest-pinned images to a tag-shaped name for the same reason.

The fingerprint is untouched: it always came from cs.ImageID and still does. Since Kosli keys artifact identity, diffing and provenance on the fingerprint rather than the name, this cannot produce spurious replacements or split artifacts — see the second comment on #1203 for the experiment behind that.

reference.Named does the bare-digest test, so no new dependency. The docker-pullable:// stripping moved out of imageFingerprint into trimRuntimePrefix, which both paths now use.

Behaviour

Same pod, same moment, reported to two environments — a name-only change produces no new snapshot, so one environment would have shown nothing:

	artifact name	fingerprint
released v2.42.0	sha256:7791402a0bf5…	6784fb0834aa…
this branch	docker.io/library/nginx:1.27	6784fb0834aa…

A tag-pulled nginx:1.30 in the same namespace reports as docker.io/library/nginx:1.30, so the two are now the same shape — which is the point.

What a user sees after upgrading

One environment across the upgrade boundary, with two digest-pinned workloads on a node that holds neither under a tag. Between #1 and #2 only one of them changed image (1.27 → 1.24); the other kept running untouched.

snapshot	reported by	events
#1	v2.42.0	sha256:b005e885… started · sha256:7791402a… started
#2	this branch	nginx:1.24 started · sha256:7791402a… stopped
#3	this branch	nginx:1.27 started · nginx:1.24 stopped

• Everything running is corrected at the first snapshot after the upgrade, not only the artifact that changed. The untouched workload is reported under its new name and annotated unchanged (was=1 now=1): Kosli matches on fingerprint, so a rename causes no exit/start churn and no duplicate artifact.
• History keeps the old names. Snapshots are immutable, and anything that exited at or before the boundary keeps its sha256: name on that exit event. Snapshot #2 reads oddly for exactly that reason — nginx:1.24 started sits directly above sha256:7791402a… stopped, and those two lines are one workload's transition.
• One image can therefore appear under two names in a single environment's history: sha256:7791402a… in #1 and #2, nginx:1.27 in #3, fingerprint 6784fb0834aa… throughout. Anything keyed on the fingerprint is unaffected; name-based search across the boundary is not.
• The rename is silent in the event log, since unchanged produces no event. The untouched workload appears once, in #1, under its sha, and never again — its corrected name is visible only in the snapshot itself.

The UI renders the familiar short form (nginx:1.27) while the stored name is docker.io/library/nginx:1.27, so in the UI the fix lands as cleanly as it can. The full form is what API consumers and name-based search see, which is why the fallback normalizes to the runtime's exact form rather than to the spec string: a digest-pinned artifact and a tag-pulled one are then byte-identical there too, not merely similar on screen.

Screenshots

Before — released v2.42.0 (name-fix-before)

---

After — this branch (name-fix-after2)

---

Verification

• go vet and golangci-lint clean on internal/kube
• internal/kube unit tests pass, including the existing TestNewPodData and TestImageFingerprint
• nine cases in TestNewPodDataArtifactName: runtime name wins when present, spec fallback keeps the tag, image-ID fallback, a spec image that is itself a bare digest, a spec container whose name does not match, an untagged spec reference falling through rather than defaulting to :latest, an empty container image name, and two digest-pinned containers keeping distinct map keys — cs.Image is the map key as well as the name, so a fallback that collided would merge two containers' digests
• end-to-end against a kind cluster with a locally built binary, as in the table above

Not run: make test_integration, which needs the local server.

Note for whoever verifies this after release

A name change alone does not create a snapshot, so upgrading the CLI changes nothing visible until the environment next takes a snapshot for some other reason — a new digest, a scale change. Forcing a change is the quickest way to see it. When that snapshot does happen, every running artifact is corrected at once, not just the changed one, as the timeline above shows. Background in #1203.

Checklist

• Docs are autogenerated from CLI help — no help text changes here, this is reporter internals
• Helm chart (charts/k8s-reporter/) — not needed, no chart-visible change
• Terraform provider and related changes — not needed

[claude]

Claude finished @dangrondahl's task in 4m 8s —— View job

---

Review: fix(k8s): name artifacts by image reference when the runtime reports an image ID

• Static analysis (go build, go vet, go mod tidy)
• Read the diff and surrounding code
• Review correctness, edge cases, tests
• Post review (2 inline comments)

Static analysis

check	result
go build ./...	clean
go vet ./internal/kube/...	clean
go mod tidy + git diff --exit-code go.mod go.sum	clean (no new dependency, as the PR says)
go test ./internal/kube/	not run — go test isn't in this job's allowed tools, so I reviewed the tests by reading them. Worth adding to --allowedTools if you want unit tests executed on review runs.

Verdict

The fix is sound and the reasoning in the description matches what the code does. imageFingerprint still reads cs.ImageID and nothing else, so artifact identity is untouched — this is a naming change only, which is what makes the rename safe across the upgrade boundary. Two rounds of review feedback were addressed properly, and the mutation check on the last one (revert the fallback, watch the case fail) is the right way to prove a branch is covered rather than just present.

Three details I checked and liked:

• The empty map key is now impossible twice over. artifactName runs only after imageFingerprint succeeded, which means trimRuntimePrefix(cs.ImageID) already parsed as a digest-bearing reference and cannot be empty. The explicit cs.Image != "" guard is belt-and-braces on top of that — fine to keep, it makes the function correct in isolation rather than correct by caller.
• isNamed gating normalizedTagName is the right fix for the ParseNormalizedNamed permissiveness the author found (sha256:8dd… parsing as repository sha256, tag 8dd…). That bug would have been easy to ship.
• Not using TagNameOnly. Defaulting an untagged reference to :latest would invent a name the image never had; falling through to the image ID is more honest, and the test pins it.

Also: pod.Status.ContainerStatuses is the only status list NewPodData walks, so pod.Spec.Containers is exactly the right list to match against — init and ephemeral containers are never reported, so there's no missing lookup.

Findings

Neither is a blocker; nothing here needs to hold the merge.

1. internal/kube/kube.go:94 — the "one image gets one name" property is scoped to the fallback path. When the runtime reports a digest-form named reference (docker.io/library/nginx@sha256:…, which containerd does produce for a digest-pinned pod whose image it holds under that reference), rule 1 returns it verbatim, and the same image on a cold-cache node resolves to docker.io/library/nginx:1.27. Same fingerprint, two names — the same split one rung up. I'd keep rule 1 as it is (normalizing cs.Image would rename artifacts for everyone currently seeing repo@sha256:…, far wider than bug: k8s reporter names an artifact by its image ID when the pod image is digest-pinned #1203), but the doc comment should say the normalization lives in the fallback so the next reader doesn't read the invariant as unconditional. Plus two nits: reference.WithTag(...).String() instead of hand-concatenating name and tag, and the closing comment reads wider than the code (an unparseable cs.Image now loses to a named ImageID, which is an improvement but not what "beats no name at all" describes).

2. internal/kube/kube_test.go:632 — two coverage gaps. Every case uses a spec digest equal to the ImageID digest, so nothing distinguishes "name came from the spec tag" from "name came from the same digest as the fingerprint" — the divergent case (in-place update, spec ahead of status) is the one where dropping the digest is load-bearing, and it's the assertion the earlier round asked for that didn't land. Second, a registry with a port (registry.local:5000/app:v1) would catch a future refactor to a naive cut at the last :. Suggested cases are in the comment.

Also worth a thought, no change requested

NewPodData keys digests by name, so two containers resolving to the same name silently collapse to one entry, last write winning. That was already true (two containers on the same tag collided identically), but the fallback adds a way for it to happen with different fingerprints: a tag-pulled container reporting docker.io/library/nginx:1.25 alongside a digest-pinned one falling back to the same name while its ImageID is an older digest the tag has since moved off. Rare, and the resulting name isn't wrong — but the pod is reported with one of the two digests and no warning. If you want it visible, a logger.Warn on overwrite in the loop is a one-liner; the "two digest-pinned containers keep distinct names" test shows you were already thinking about this axis.

No security or performance concerns — a handful of reference parses per container per snapshot, against regexes compiled once at package init.
· branch fix/k8s-reporter-artifact-name

[claude]

Follow-up review of 30ea69e (b9cdc71..30ea69e)

Finding 1 — empty cs.Image fallback produced an empty map key
Status: partially addressed. The production fix in internal/kube/kube.go:125-130 matches the suggestion exactly (cs.Image only when non-empty, else trimRuntimePrefix(cs.ImageID)) — the empty-key bug is fixed. However, the new test case (kube_test.go:668-671, "a container status with no name at all...") uses containerStatus{"", nginxImageID}, and nginxImageID trims to a named reference (nginx@sha256:...). That means artifactName returns earlier, at the isNamed(imageID) check (kube.go:122-124), and never reaches the branch the fix touches. Reverting the fix locally and re-running this test still passes, confirming the new fallback branch is currently untested despite the commit message describing a "bare-digest image ID" case. Posted as an inline comment with the fix (use the existing bare-digest imageID constant as ImageID instead).

Finding 2 — digest-in-name mismatch with fingerprint, and untested non-matching-container branch
Status: resolved. normalizedTagName (kube.go:133-155) now strips the digest and returns the tagged form the runtime would report, matching the Cloud Run reporter's convention and eliminating the name/fingerprint disagreement. The non-matching-spec-container branch is now covered by the new "a spec container whose name does not match is not used" test case.

New issues in the touched lines: 1 inline comment posted (test-coverage gap noted above under Finding 1; no functional bug found in the production code itself).

[claude]

Not a defect — a scope note on the "one image gets one name" property, so it's a decision rather than an accident.

Normalization only runs on the fallback path, so the property holds across nodes that report a bare image ID. The other shape the runtime reports for a digest-pinned pod is a digest-form named reference (docker.io/library/nginx@sha256:…), which containerd does produce when it holds the image under the reference the pod pinned. That is Named, so it returns here verbatim, and the same image on a cold-cache node in the same deployment resolves to docker.io/library/nginx:1.27 via normalizedTagName. Same fingerprint, two names — the split this PR is closing, just one rung up.

Leaving rule 1 alone is the conservative choice and I'd keep it: normalizing cs.Image too would rename artifacts for every user currently seeing repo@sha256:…, which is a much wider blast radius than the bare-sha256: case in #1203. Worth a sentence in the artifactName doc comment noting the fallback is where the normalization happens, so the next reader doesn't assume the invariant is unconditional.

Two small things while in here:

• reference.TrimNamed(ref).Name() + ":" + tagged.Tag() (line 134) hand-rolls what reference.WithTag(reference.TrimNamed(ref), tagged.Tag()) returns from String(). Same output, and it can't drift from the library's formatting.
• An unparseable-but-non-empty cs.Image now loses to a named ImageID rather than being used as the name, since isNamed is false for anything ParseAnyReference rejects. That's an improvement, but it does make the closing comment ("the runtime's name still beats no name at all") narrower than it reads — it beats no name, not a readable image ID.

[claude]

Two cases that would pin behaviour the table currently leaves implicit.

1. A spec digest that differs from the ImageID digest. Every case here uses nginxDigest, whose digest is the same 644a… that nginxImageID carries, so nothing distinguishes "the name came from the spec tag" from "the name came from the same digest as the fingerprint". The divergent case is the one the digest-dropping decision was made for — an in-place image update, where the spec points at the new reference while the status still reports the old ImageID:

{
	// the spec and the status disagree mid-update: the name comes from the
	// spec's tag, the fingerprint from the status's image ID
	name: "a spec digest that differs from the image ID does not reach the name",
	pod:  podWithSpec("pod", []string{"nginx:1.25@sha256:" + imageIDSha}, containerStatus{imageID, nginxImageID}),
	wantDigests: map[string]string{"docker.io/library/nginx:1.25": nginxSha},
},

That is the assertion the earlier review round asked for and it didn't land — worth having, since it's the one case where dropping the digest is load-bearing rather than cosmetic.

2. A registry with a port. normalizedTagName splits name from tag through reference, so registry.local:5000/app:v1 normalizes correctly, but nothing here would catch a future refactor to a naive cut at the last ::

{
	name:        "a registry port is not mistaken for a tag",
	pod:         podWithSpec("pod", []string{"registry.local:5000/app:v1"}, containerStatus{imageID, nginxImageID}),
	wantDigests: map[string]string{"registry.local:5000/app:v1": nginxSha},
},

Also worth considering, lower value: a multi-container pod whose spec order differs from the status order, so the container.Name == cs.Name match is doing real work rather than agreeing with position. podWithSpec pairs them positionally, so today the loop finds the right entry either way.

Fix this →