NO-JIRA: Handle unexpected image configuration media type in getArchitectures#31039
NO-JIRA: Handle unexpected image configuration media type in getArchitectures#31039bitoku wants to merge 1 commit intoopenshift:mainfrom
Conversation
Skip images that don't have the expected image configuration media type instead of treating them as errors, since they are artifacts rather than runnable container images. Assisted-by: Claude Code <https://claude.com/claude-code>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@bitoku: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bitoku The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/clioptions/imagesetup/images.go`:
- Around line 141-143: The current branch in pkg/clioptions/imagesetup/images.go
returns (nil, nil) for artifact images which makes them indistinguishable from
missing-manifest cases; change this to return a sentinel error (e.g., declare
var ErrImageIsArtifact = errors.New("image is an artifact") in the package) and
replace the current return nil, nil with return nil, ErrImageIsArtifact in the
branch that detects "does not have the expected image configuration media type";
also update the caller/consumer (e.g., VerifyManifestLists) to check for
ErrImageIsArtifact and explicitly skip processing when that sentinel is
returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ed461195-7523-4380-8c3d-e5ed391b5a63
📒 Files selected for processing (1)
pkg/clioptions/imagesetup/images.go
| } else if err != nil && strings.Contains(string(output), "does not have the expected image configuration media type") { | ||
| logrus.Debugf(" Image is an artifact") | ||
| return nil, nil |
There was a problem hiding this comment.
Artifact path is not actually skipped by downstream logic.
At Line 143, returning (nil, nil) makes artifacts indistinguishable from the non-manifest-list path, so VerifyManifestLists still classifies them as problematic instead of skipping them. This conflicts with the stated PR behavior.
Proposed fix (preserve artifact intent via sentinel error)
diff --git a/pkg/clioptions/imagesetup/images.go b/pkg/clioptions/imagesetup/images.go
@@
import (
+ "errors"
"fmt"
"os"
"os/exec"
"strings"
"time"
@@
)
+
+var errImageArtifact = errors.New("image is an artifact")
@@
if err != nil && strings.Contains(string(output), "the image is a manifest list") {
logrus.Debugf(" Image is a manifest list")
} else if err != nil && strings.Contains(string(output), "does not have the expected image configuration media type") {
logrus.Debugf(" Image is an artifact")
- return nil, nil
+ return nil, errImageArtifact
} else if err != nil {
return nil, fmt.Errorf("failed to check image %s: %w\nOutput: %s", image, err, string(output))
} else {
@@
- availableArchs, err := getArchitectures(img, requiredArchs)
- if err != nil {
+ availableArchs, err := getArchitectures(img, requiredArchs)
+ if err != nil {
+ if errors.Is(err, errImageArtifact) {
+ logrus.Debugf(" Image is an artifact, skipping")
+ skippedImages = append(skippedImages, img)
+ continue
+ }
return fmt.Errorf("failed to get image info for %s: %w", img, err)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/clioptions/imagesetup/images.go` around lines 141 - 143, The current
branch in pkg/clioptions/imagesetup/images.go returns (nil, nil) for artifact
images which makes them indistinguishable from missing-manifest cases; change
this to return a sentinel error (e.g., declare var ErrImageIsArtifact =
errors.New("image is an artifact") in the package) and replace the current
return nil, nil with return nil, ErrImageIsArtifact in the branch that detects
"does not have the expected image configuration media type"; also update the
caller/consumer (e.g., VerifyManifestLists) to check for ErrImageIsArtifact and
explicitly skip processing when that sentinel is returned.
|
Scheduling required tests: |
|
@bitoku: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
bitoku
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
2 participants
Skip images that don't have the expected image configuration media type instead of treating them as errors, since they are artifacts rather than runnable container images.
Assisted-by: Claude Code https://claude.com/claude-code
Summary by CodeRabbit