Skip to content

fix(sync-ci-images): stop assuming oc image mirror preserves manifest digests #3196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ashwindasr merged 1 commit into from
Jul 23, 2026
+6 −13
Merged

fix(sync-ci-images): stop assuming oc image mirror preserves manifest digests #3196

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions doozer/doozerlib/cli/images_streams.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def images_streams_mirror(
elif runtime.registry_config_dir is not None:
registry_config_file = get_docker_config_json(runtime.registry_config_dir)

def mirror_image(cmd_start: str, upstream_dest: str, source_digest: Optional[str] = None):
def mirror_image(cmd_start: str, upstream_dest: str):
if upstream_dest.startswith('registry.ci.openshift.org/'):
# Images targeting CI imagestreams must be mirrored to quay.io/openshift/ci (QCI) first,
# then imagestreams updated to reference the QCI image by digest.
Expand Down Expand Up @@ -206,12 +206,10 @@ def mirror_image(cmd_start: str, upstream_dest: str, source_digest: Optional[str
f'Failed to mirror {upstream_entry_name}: {stderr}', (rc, stdout, stderr)
)

# Use source digest when available (oc image mirror preserves manifest digests).
# Falling back to get_image_digest for tag-based sources where digest isn't known.
if source_digest:
qci_digest = source_digest
else:
qci_digest = get_image_digest(floating_qci_dest, registry_config_file)
# Always query the actual digest at QCI after mirroring.
# oc image mirror may convert manifest formats (OCI↔Docker v2s2),
# which changes the digest, so the source digest cannot be trusted.
qci_digest = get_image_digest(floating_qci_dest, registry_config_file)
Comment on lines +209 to +212

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not silently continue when the QCI digest refresh fails.

get_image_digest can return None, after which the current warning-only path leaves the imagestream on its previous digest while reporting no failure. Retry the lookup and raise if it still cannot be confirmed.

🤖 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/images_streams.py` around lines 209 - 212, Update the
QCI digest refresh around get_image_digest so a None result triggers a retry; if
the second lookup still returns None, raise an error instead of continuing with
the previous digest. Preserve the existing successful digest flow and ensure the
failure is surfaced to the caller.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep dry-run side-effect free.

The dry_run branch at Lines 191-193 only prints, so this unconditional lookup can fall through to the real GC mirror and imagestream patch. Return before querying, or guard the entire post-mirror block with if not dry_run.

Proposed fix 
-                qci_digest = get_image_digest(floating_qci_dest, registry_config_file)
+                if dry_run:
+                    return
+                qci_digest = get_image_digest(floating_qci_dest, registry_config_file)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Always query the actual digest at QCI after mirroring.
# oc image mirror may convert manifest formats (OCI↔Docker v2s2),
# which changes the digest, so the source digest cannot be trusted.
qci_digest = get_image_digest(floating_qci_dest, registry_config_file)
# Always query the actual digest at QCI after mirroring.
# oc image mirror may convert manifest formats (OCI↔Docker v2s2),
# which changes the digest, so the source digest cannot be trusted.
if dry_run:
return
qci_digest = get_image_digest(floating_qci_dest, registry_config_file)
🤖 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/images_streams.py` around lines 209 - 212, Keep the
dry_run path side-effect free by preventing the post-mirror processing around
get_image_digest from executing when dry_run is enabled. Return immediately
after the dry-run print in the image mirroring flow, or guard the digest lookup
and subsequent GC mirror and imagestream patch operations with a not-dry-run
condition; preserve the existing behavior for real runs.


if qci_digest:
# Mirror to GC-prevention tag: art__<digest>
Expand Down Expand Up @@ -357,13 +355,9 @@ def mirror_image(cmd_start: str, upstream_dest: str, source_digest: Optional[str
if registry_config_file is not None:
cmd += f" --registry-config={registry_config_file}"

# Extract digest from source pullspec when available (e.g. Konflux @sha256: refs).
# oc image mirror preserves manifest bytes, so source digest == destination digest.
src_digest = src_image_pullspec.split('@')[1] if '@' in src_image_pullspec else None

# Mirror to main destination only if not in only-if-missing mode OR destination doesn't exist
if not only_if_missing or not destinations_to_check.get(upstream_dest, False):
mirror_image(cmd, upstream_dest, source_digest=src_digest)
mirror_image(cmd, upstream_dest)

# mirror arm64 builder and base images for CI
if mirror_arm:
Expand All @@ -372,7 +366,6 @@ def mirror_image(cmd_start: str, upstream_dest: str, source_digest: Optional[str
if registry_config_file is not None:
arm_cmd += f" --registry-config={registry_config_file}"
# Mirror ARM64 only if not in only-if-missing mode OR destination doesn't exist
# ARM64 filter produces a single-arch image, not the original manifest list, so don't pass source_digest
if not only_if_missing or not destinations_to_check.get(f'{upstream_dest}-arm64', False):
mirror_image(arm_cmd, f'{upstream_dest}-arm64')

Expand Down