OCPBUGS-100066: skip wildcard sources in IDMS/ICSP image reference parsing - #2351
OCPBUGS-100066: skip wildcard sources in IDMS/ICSP image reference parsing#2351savio87 wants to merge 2 commits into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@savio87: This pull request references Jira Issue OCPBUGS-100066, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
WalkthroughICSP and IDMS mirror resolution now logs and skips wildcard sources beginning with ChangesMirror source filtering
Estimated code review effort: 2 (Simple) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
This fixes a crash in oc adm node-image create when IDMS contains wildcard source entries like *.redhat.com. Could a maintainer please set the target version on OCPBUGS-100066 and run /ok-to-test? Thank you! |
|
Hi @savio87. 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 Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: savio87 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/cli/image/strategy/onerror.go (1)
98-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for both wildcard filtering paths.
The implementation matches the PR objective. Add tests for
alternativeImageSourcesICSPandalternativeImageSourcesIDMSthat verify:
- wildcard sources do not cause a parse error;
- wildcard-derived mirrors are not returned;
- valid non-wildcard sources still work;
- IDMS preserves the original-source fallback when all entries are skipped.
Also applies to: 236-239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/cli/image/strategy/onerror.go` around lines 98 - 101, Add regression coverage for alternativeImageSourcesICSP and alternativeImageSourcesIDMS covering wildcard sources without parse errors, excluding wildcard-derived mirrors, and retaining valid non-wildcard sources. For IDMS, also verify the original-source fallback remains when every entry is skipped; exercise both wildcard filtering paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/cli/image/strategy/onerror.go`:
- Around line 98-101: Add regression coverage for alternativeImageSourcesICSP
and alternativeImageSourcesIDMS covering wildcard sources without parse errors,
excluding wildcard-derived mirrors, and retaining valid non-wildcard sources.
For IDMS, also verify the original-source fallback remains when every entry is
skipped; exercise both wildcard filtering paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d566006-1940-49f1-bf66-1656bb5f179d
📒 Files selected for processing (1)
pkg/cli/image/strategy/onerror.go
Bug
https://redhat.atlassian.net/browse/OCPBUGS-100066
oc adm node-image createfails withinvalid source "*.redhat.com": invalid reference formatwhen the cluster has an ImageDigestMirrorSet (IDMS) containing wildcard domain sources like*.redhat.comor*.redhat.io.Root Cause
In
pkg/cli/image/strategy/onerror.go, thealternativeImageSourcesIDMS()andalternativeImageSourcesICSP()functions callreference.Parse(rdm.Source)on every IDMS/ICSP source entry. Wildcard sources (e.g.*.redhat.com) are valid in the IDMS CRD schema and handled correctly by CRI-O at runtime, butreference.Parse()rejects them because*is not a valid character in a Docker image reference.Fix
Skip IDMS/ICSP source entries that start with
*.before attempting to parse them as Docker image references. These wildcard entries cannot be meaningfully matched to a specific image reference anyway, so skipping them is the correct behavior. A debug-level log message is emitted when a wildcard source is skipped.Summary by CodeRabbit
*.are now skipped during image mirror resolution.