Fix resolveUpstreamRef to properly propagate not-found errors

[akesarka]

Summary

• Fix resolveUpstreamRef() to use convertImageStreamGetterError() instead of hardcoding ErrImageStreamUnknownErrorCode, so that not-found errors from imageStreamGetter.layers() are properly converted to ErrImageStreamNotFoundCode.
• This fixes HTTP 500 responses on manifest HEAD requests for non-existent ImageStreams, which broke ImageStream auto-creation with Docker Engine 29+ clients.

Problem

Docker Engine 29+ (with containerd image store) wraps single-platform images in OCI indexes and issues HEAD requests to check sub-manifest existence before PUT. When the ImageStream does not exist, manifestService.Exists() -> GetImageOfImageStream() -> resolveUpstreamRef() returns ErrImageStreamUnknownErrorCode instead of ErrImageStreamNotFoundCode, causing the registry to respond with HTTP 500 instead of 404.

This prevents the Docker client from proceeding to the manifest PUT, which is where CreateImageStreamMapping() auto-provisions the ImageStream.

Root Cause

resolveUpstreamRef() was the only method in pkg/imagestream/imagestream.go that did not use convertImageStreamGetterError() to translate getter errors. All other methods (ResolveImageID, TagIsInsecure, Exists, localRegistry, IdentifyCandidateRepositories, Tags) already use this pattern.

Test plan

• Verify the fix compiles and passes existing unit tests
• Push an image with Docker 29+ to a non-existent ImageStream and confirm auto-creation works
• Confirm existing pull-through and manifest operations are unaffected

Fixes: https://redhat.atlassian.net/browse/OCPBUGS-99398

Summary by CodeRabbit

• Bug Fixes
  • Improved error reporting when image layers cannot be retrieved, providing more accurate error classification and handling.

[coderabbitai]

Warning

Review limit reached

@akesarka, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 54 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c673b39a-69be-44c2-a2fd-650bfc4ec345

📥 Commits

Reviewing files that changed from the base of the PR and between 1698d6a and 27ddd45.

📒 Files selected for processing (1)

• pkg/imagestream/imagestream.go

Walkthrough

resolveUpstreamRef now uses the existing image stream getter error conversion when retrieving image layers, preserving error-code mappings from underlying errors instead of always returning an unknown error code.

Changes

Image stream error mapping

Layer / File(s)	Summary
Preserve layer retrieval error codes pkg/imagestream/imagestream.go	resolveUpstreamRef converts layer-fetch failures with convertImageStreamGetterError rather than wrapping them as ErrImageStreamUnknownErrorCode.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix in resolveUpstreamRef and its not-found error propagation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	No test files or Ginkgo titles were changed; the PR only edits pkg/imagestream/imagestream.go.
Test Structure And Quality	✅ Passed	PR only changes pkg/imagestream/imagestream.go; no Ginkgo tests were added or modified, so the test-structure checklist is not applicable.
Microshift Test Compatibility	✅ Passed	PR changes only pkg/imagestream/imagestream.go; no new Ginkgo e2e tests or MicroShift-incompatible APIs were added.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Only pkg/imagestream/imagestream.go changed; no Ginkgo It/Describe/Context/When tests were added or modified.
Topology-Aware Scheduling Compatibility	✅ Passed	Only pkg/imagestream/imagestream.go changed, and the patch only alters error mapping in resolveUpstreamRef(); no scheduling or topology logic was added.
Ote Binary Stdout Contract	✅ Passed	The PR only changes resolveUpstreamRef’s error conversion in a library file; no main/init/TestMain/suite setup or stdout/logging writes were added.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Only pkg/imagestream/imagestream.go changed; no new Ginkgo e2e tests or network/IP-handling test code were added.
No-Weak-Crypto	✅ Passed	The patch only changes error mapping in resolveUpstreamRef; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons were added.
Container-Privileges	✅ Passed	The PR only changes pkg/imagestream/imagestream.go; no container/K8s manifests or privileged settings appear in the diff.
No-Sensitive-Data-In-Logs	✅ Passed	Changed code only converts getter errors in resolveUpstreamRef; no new logs or sensitive fields were added.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akesarka
Once this PR has been reviewed and has the lgtm label, please assign ricardomaraschini for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

Hi @akesarka. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.