ART-21809: Add doozer verify-image-consistency command - #3213
ART-21809: Add doozer verify-image-consistency command#3213tomasdavidorg wants to merge 5 commits into
Conversation
Verify that every image in the release payload is present in the shipment MR or has already been released in the Red Hat catalog. Compares payload images (oc adm release info --pullspecs) against shipment components from GitLab MR YAML files. RHCOS images are skipped. Images matched by digest, list digest, or VCS reference, with Red Hat Catalog API as fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
openshift-ci-robot
added
the
jira/valid-reference
|
@tomasdavidorg: This pull request references ART-21809 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change adds a ChangesImage consistency verification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Verification
participant OpenShift
participant GitLab
participant Catalog
CLI->>Verification: start image consistency verification
Verification->>OpenShift: fetch payload images and identifiers
Verification->>GitLab: fetch shipment components
Verification->>Verification: match image identifiers
Verification->>Catalog: check unmatched image digest
Catalog-->>Verification: return catalog match
Verification-->>CLI: render pass/fail result
Possibly related PRs
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
doozer/doozerlib/cli/verify_image_consistency.py (1)
160-176: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSet an explicit request timeout for the catalog lookup.
session.get(url)relies on aiohttp's session-level default; the library's own docs note aiohttpClientSessionhas "Default client timeouts, ClientTimeout instance. The value can be tuned by passing timeout parameter to ClientSession constructor.", and that default is commonly cited as "the default timeout set by the library is 300 seconds". Without an explicit shorter timeout, a slow/unreachable catalog endpoint can stall each unmatched image's check for minutes.♻️ Proposed fix
try: - async with aiohttp.ClientSession() as session: + timeout = aiohttp.ClientTimeout(total=15) + async with aiohttp.ClientSession(timeout=timeout) as session: async with session.get(url) as resp:🤖 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 `@doozer/doozerlib/cli/verify_image_consistency.py` around lines 160 - 176, Update the catalog request in check_catalog to use an explicit shorter aiohttp timeout, passing the configured timeout directly to session.get (or the ClientSession constructor) instead of relying on aiohttp’s default. Preserve the existing response handling and exception fallback behavior.
🤖 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.
Inline comments:
In `@doozer/doozerlib/cli/verify_image_consistency.py`:
- Around line 179-183: Update the async flows around fetch_payload_images and
fetch_shipment_components to schedule both operations with asyncio.gather or
asyncio.create_task before awaiting results, preserving their existing result
ordering. Likewise, change the identifiers_tasks handling around
fetch_image_identifiers so all unique pullspec lookups are scheduled before
collecting results, allowing them to execute concurrently rather than
sequentially.
- Around line 113-114: Update the diff retrieval flow around
mr.diffs.list(all=True) to check whether the returned list is empty before
accessing its first element. Handle the no-diff-version case with a clear
descriptive error while preserving the existing mr.diffs.get call for available
diff versions.
- Around line 160-176: Update check_catalog() to query the Red Hat Catalog API
using the docker_image_digest filter instead of image_id, while preserving the
existing response handling and failure behavior.
---
Nitpick comments:
In `@doozer/doozerlib/cli/verify_image_consistency.py`:
- Around line 160-176: Update the catalog request in check_catalog to use an
explicit shorter aiohttp timeout, passing the configured timeout directly to
session.get (or the ClientSession constructor) instead of relying on aiohttp’s
default. Preserve the existing response handling and exception fallback
behavior.
🪄 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-eng/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b8ebef29-5f76-44c6-8d07-dbc8b96e1fba
📒 Files selected for processing (3)
doozer/doozerlib/cli/__main__.pydoozer/doozerlib/cli/verify_image_consistency.pydoozer/tests/cli/test_verify_image_consistency.py
- Guard against empty MR diff versions list with descriptive error - Fix Catalog API filter: use docker_image_digest instead of image_id - Use asyncio.gather for concurrent payload/shipment fetch and image identifier lookups instead of sequential awaits Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
There was a problem hiding this comment.
🧹 Nitpick comments (1)
doozer/doozerlib/cli/verify_image_consistency.py (1)
219-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the
zipinvariant explicit. Usestrict=Truehere so any future length mismatch betweenpullspec_listandfetchedfails fast.🤖 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 `@doozer/doozerlib/cli/verify_image_consistency.py` at line 219, Update the zip call that builds identifiers in the verify-image consistency flow to pass strict=True, ensuring mismatched pullspec_list and fetched lengths fail immediately while preserving the existing dictionary construction.Source: Linters/SAST tools
🤖 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 `@doozer/doozerlib/cli/verify_image_consistency.py`:
- Line 219: Update the zip call that builds identifiers in the verify-image
consistency flow to pass strict=True, ensuring mismatched pullspec_list and
fetched lengths fail immediately while preserving the existing dictionary
construction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4aeb5cf8-39b0-49ca-a0b7-ad2fe64b78b8
📒 Files selected for processing (1)
doozer/doozerlib/cli/verify_image_consistency.py
- Add explicit 15s timeout for Red Hat Catalog API requests - Use strict=True in zip() for pullspec/identifiers mapping Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
doozer/doozerlib/cli/verify_image_consistency.py (1)
234-236: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftBound catalog fallback concurrency.
Each unmatched image waits for its catalog request to finish before the next request starts. With the 15-second timeout, a catalog outage can make runtime grow to roughly
15 seconds × unmatched images; creating a newClientSessionper lookup also prevents connection reuse. Reuse a shared session and perform bounded-concurrency catalog checks.🤖 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 `@doozer/doozerlib/cli/verify_image_consistency.py` around lines 234 - 236, Update the unmatched-image handling around check_catalog so catalog lookups reuse a shared aiohttp ClientSession and run with bounded concurrency instead of being awaited serially per image. Add a concurrency limiter for the catalog fallback requests, preserve setting check.found_in to "catalog" only when the lookup succeeds, and ensure the shared session is properly closed.
🤖 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.
Inline comments:
In `@doozer/doozerlib/cli/verify_image_consistency.py`:
- Around line 167-168: Validate the external digest with an allow-list matching
the documented digest grammar before constructing the catalog URL in the
verify-image flow. Reject malformed or empty values before the f-string
interpolation, and only build the RSQL filter after validation succeeds.
---
Outside diff comments:
In `@doozer/doozerlib/cli/verify_image_consistency.py`:
- Around line 234-236: Update the unmatched-image handling around check_catalog
so catalog lookups reuse a shared aiohttp ClientSession and run with bounded
concurrency instead of being awaited serially per image. Add a concurrency
limiter for the catalog fallback requests, preserve setting check.found_in to
"catalog" only when the lookup succeeds, and ensure the shared session is
properly closed.
🪄 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-eng/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 97a6b020-9098-4b0f-a0f4-3881faab4cd8
📒 Files selected for processing (1)
doozer/doozerlib/cli/verify_image_consistency.py
| url = f"{CATALOG_API_URL}?filter=docker_image_digest=={digest}" | ||
| try: |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate digest before interpolating it into the catalog filter.
digest originates from external image metadata but is only checked for emptiness before being inserted into the RSQL query. A malformed value containing query or RSQL delimiters could alter the filter and produce a false catalog match. Enforce the documented digest grammar with an allow-list before constructing the URL.
As per path instructions, validate at trust boundaries with allow-lists, not deny-lists.
🤖 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 `@doozer/doozerlib/cli/verify_image_consistency.py` around lines 167 - 168,
Validate the external digest with an allow-list matching the documented digest
grammar before constructing the catalog URL in the verify-image flow. Reject
malformed or empty values before the f-string interpolation, and only build the
RSQL filter after validation succeeds.
Source: Path instructions
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
tomasdavidorg
added
the
tide/merge-method-squash
… verify-image-consistency Resolve shipment MR URL from assembly config and construct payload pullspec from assembly name, consistent with verify-image-grades pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
@tomasdavidorg: The following test 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. |
|
Closing — elliott already has verify-payload which does the same check (payload vs advisory/shipment). For post-release ERT verification we'll reuse verify-payload instead of adding a new command. |
2 participants
Verify that every image in the release payload is present in the shipment MR or has already been released in the Red Hat catalog.
Compares payload images (oc adm release info --pullspecs) against shipment components from GitLab MR YAML files. RHCOS images are skipped. Images matched by digest, list digest, or VCS reference, with Red Hat Catalog API as fallback.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Summary by CodeRabbit
New Features
verify-image-consistencycommand to compare release payload images with shipment components.Tests