Skip to content

Adding Digest specification for test images - #97

Merged
tommyd450 merged 2 commits into
mainfrom
tdalton/TestImageDigests
Aug 5, 2026
Merged

Adding Digest specification for test images#97
tommyd450 merged 2 commits into
mainfrom
tdalton/TestImageDigests

Conversation

@tommyd450

Copy link
Copy Markdown
Contributor

No description provided.

@qodo-for-securesign

Copy link
Copy Markdown

PR Summary by Qodo

Return digest-pinned reference for pushed test images

✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Compute and return a digest-pinned image reference after pushing the test image.
• Log both the tag-based and digest-based references for easier debugging.
• Ensure downstream tests can use immutable image identifiers instead of unique tags.
Diagram

graph TD
  A["E2E tests"] --> B["PushTestImage()"] --> C[("Base registry")]
  B --> D[("Quay test repo")] --> E["Digest ref (@sha256)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fetch canonical digest from registry after push (remote.Head/Get)
  • ➕ Guarantees the returned digest matches what the registry stored/serves
  • ➕ Avoids any theoretical mismatch if the registry mutates manifests/layers
  • ➖ Adds an extra network round-trip after push
  • ➖ Requires using the tag ref for lookup before converting to digest ref
2. Return both tag ref and digest ref
  • ➕ Preserves current behavior for any tag-based workflows
  • ➕ Digest available for immutability while keeping the tag for human tracing
  • ➖ Requires changing call sites to handle a tuple/struct return
  • ➖ More API surface for a helper intended to be simple

Recommendation: The PR’s approach (compute digest from the in-memory image and return a digest-pinned reference) is a good fit for test determinism with minimal complexity. If you ever observe digest mismatches in practice, consider switching to a post-push registry lookup (e.g., remote.Head on the pushed tag) to return the registry’s canonical digest.

Files changed (1) +8 / -2

Enhancement (1) +8 / -2
image.goReturn digest reference from PushTestImage +8/-2

Return digest reference from PushTestImage

• After pushing the test image, the helper now computes the image digest and returns a digest-pinned reference (repo@sha256:...). Logging was updated to include both the tag-based ref and the digest-based ref for traceability.

test/testsupport/image.go

@qodo-for-securesign

qodo-for-securesign Bot commented Aug 5, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Post-push digest failure ✓ Resolved 🐞 Bug ☼ Reliability
Description
PushTestImage computes img.Digest() after remote.Write(); if Digest() fails, the function returns an
error even though the image was already pushed, causing test failures and leaving a pushed tag/image
behind. This new partial-success behavior is introduced by the added digest computation/return path.
Code

test/testsupport/image.go[R63-66]

+	digest, err := img.Digest()
+	if err != nil {
+		return "", fmt.Errorf("computing image digest: %w", err)
+	}
Relevance

●●● Strong

Team commonly accepts reliability changes to prevent CI/test flakiness; post-success error path
likely to be removed/softened.

PR-#76
PR-#95

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new code performs the registry write, then introduces a new error-returning operation
(img.Digest()) whose failure returns an error after a successful push.

test/testsupport/image.go[58-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`PushTestImage` calls `remote.Write(...)` and only afterwards calls `img.Digest()`. If `Digest()` returns an error, the function returns an error even though the push already succeeded, creating a partial-success outcome (registry side effect but failing test).

### Issue Context
This behavior was introduced when switching the return value from `targetRef` to a digest reference.

### Fix Focus Areas
- test/testsupport/image.go[58-70]

### Suggested fix
- Move `digest, err := img.Digest()` (and `digestRef` construction) *before* `remote.Write(...)` so any digest-related failure happens before the push.
- Alternatively, treat `remote.Write` success as authoritative: if digest computation fails, return the pushed `targetRef` (or return both tag+digest) instead of failing after a successful push.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Misleading PushTestImage docs ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The function comment says it pushes with a unique tag, but the function now returns a digest
reference (no tag), which can mislead callers about the returned value’s format and intended use.
This mismatch is caused by returning digestRef instead of targetRef.
Code

test/testsupport/image.go[R68-70]

+	digestRef := testImageRegistry + "@" + digest.String()
+	logrus.Infof("Pushed test image: %s (digest: %s)", targetRef, digestRef)
+	return digestRef, nil
Relevance

●●● Strong

Low-risk doc/comment fix; team has accepted multiple maintainability cleanups and test support
clarifications.

PR-#53
PR-#95

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The comment emphasizes a unique tag push, but the new return value is built as repo@digest and
returned, so the function’s observable output no longer matches what the comment implies.

test/testsupport/image.go[20-26]
test/testsupport/image.go[68-71]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`PushTestImage` now returns a digest reference, but the comment describes pushing with a unique tag and does not document that the returned string is *not* the tagged reference.

### Issue Context
The push still uses a unique tag (`targetRef`), but callers receive `digestRef`.

### Fix Focus Areas
- test/testsupport/image.go[20-25]
- test/testsupport/image.go[68-71]

### Suggested fix
- Update the doc comment to explicitly state the return value, e.g. “pushes with a unique tag and returns the corresponding digest reference (`repo@sha256:...`)”.
- (Optional) Consider renaming to clarify behavior (e.g., `PushTestImageDigest`) or returning both tag and digest if callers need both.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@osmman

osmman commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Please squash commits into single change when you will merge it

Updating e2e action to pull in secrets

Updating address name

Adding Digest specification for test images

Adding label to ensure it is unique
@tommyd450
tommyd450 force-pushed the tdalton/TestImageDigests branch 2 times, most recently from b61d883 to 9ff6a17 Compare August 5, 2026 09:37
@tommyd450
tommyd450 merged commit 8f23e15 into main Aug 5, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants