chore(mirror-plugins.sh) Add fallback to quay.io for unreleased plugins in mirror script

[Fortune-Ndlovu]

Description

When mirroring plugins from catalog index, if a plugin references registry.access.redhat.com/rhdh but is not yet released there, the script now falls back to pulling from quay.io/rhdh instead.

Which issue(s) does this PR fix or relate to

https://issues.redhat.com/browse/RHDHBUGS-2622

PR acceptance criteria

• Tests
• Documentation

How to test changes / Special notes to the reviewer

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gazarenkov 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

[rhdh-qodo-merge]

PR Reviewer Guide 🔍

(Review updated until commit e21114c)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 RHDHBUGS-2622 - Partially compliant Compliant requirements: Add fallback to pulling from quay.io/rhdh when registry.access.redhat.com/rhdh plugin images are missing/unavailable Prefer the primary registry when the image exists there Apply the same effective-source logic when resolving/mirroring the catalog index image itself Non-compliant requirements: Requires further human verification: Validate the original repro command works end-to-end against a catalog index containing unreleased/unauthed registry.access.redhat.com/rhdh refs Validate behavior when the primary image exists but requires auth (ensure fallback behavior is what you want vs. hard-failing)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🔒 Security concerns Destination pushes use --dest-tls-verify=false (line 377). If this script can be used outside of strictly trusted/internal registries, disabling TLS verification enables MITM risk when pushing mirrored artifacts. Consider making this opt-in via a flag, or documenting the security tradeoff prominently.

⚡ Recommended focus areas for review Fallback correctness check_image_exists() uses skopeo inspect as an existence check; for registries that require auth, "unauthorized" may be interpreted as "does not exist", which could incorrectly trigger fallback to quay.io. Confirm desired behavior for auth failures vs true 404/missing, and consider distinguishing these cases (e.g., inspect error code/message parsing, or allowing explicit auth handling). # Check if an image exists in a registry using skopeo inspect # Returns 0 if image exists, 1 otherwise # This is more reliable than parsing error strings from skopeo copy function check_image_exists() { local docker_ref="$1" skopeo inspect "docker://$docker_ref" &>/dev/null return $? } # Get the effective registry reference, trying fallback if primary doesn't exist # Sets global EFFECTIVE_REF to the working registry reference # Sets global USED_FALLBACK=1 if fallback was used # Returns 0 on success, 1 on failure function get_effective_registry_ref() { local docker_ref="$1" EFFECTIVE_REF="" USED_FALLBACK=0 # Check if the image exists at the primary location if check_image_exists "$docker_ref"; then EFFECTIVE_REF="$docker_ref" return 0 fi # If the source is registry.access.redhat.com/rhdh, try fallback to quay.io/rhdh if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/rhdh/"* ]]; then local fallback_ref="${docker_ref/${PRIMARY_SOURCE_REGISTRY}/${FALLBACK_SOURCE_REGISTRY}}" debugf "Primary registry image not found, checking fallback: $fallback_ref" if check_image_exists "$fallback_ref"; then warnf "Image not found at ${PRIMARY_SOURCE_REGISTRY}, using fallback: ${FALLBACK_SOURCE_REGISTRY}" EFFECTIVE_REF="$fallback_ref" USED_FALLBACK=1 return 0 fi # Neither primary nor fallback exists errorf "Image not found at both ${PRIMARY_SOURCE_REGISTRY} and ${FALLBACK_SOURCE_REGISTRY}" return 1 fi # For non-RHDH registries, just report the failure return 1 } Regex/logging risk The sed replacements for oci://... references are broad and may rewrite unintended strings/fields in YAML (and could behave differently across GNU/BSD sed). Also, printing the full dynamic-plugins.default.yaml to logs may leak content and create very large logs in CI. Consider tightening the match scope (or using yq/structured parsing if feasible) and avoid unconditional cat (gate behind debugf or a flag). # Update OCI references in dynamic-plugins.default.yaml infof "Updating OCI references in dynamic-plugins.default.yaml..." if [[ -f "$catalog_data_dir/dynamic-plugins.default.yaml" ]]; then # Replace OCI registry references, keeping only the last 2 path elements # to ensure compatibility with OCP internal registry (2-level paths). # Pattern: oci://REG/[extra/]ns/image:TAG -> oci://INTERNAL_REGISTRY/ns/image:TAG sed -i -E "s|oci://[^/]+(/[^/]+)*(/[^/]+/[^[:space:]\"']+)|oci://$internal_registry\2|g" "$catalog_data_dir/dynamic-plugins.default.yaml" debugf "Updated OCI references in dynamic-plugins.default.yaml" infof "=== dynamic-plugins.default.yaml after update ===" cat "$catalog_data_dir/dynamic-plugins.default.yaml" infof "=== end dynamic-plugins.default.yaml ===" fi

📄 References redhat-developer/rhdh/scripts/compare-wrappers-with-published-oci-images.sh [1-23] redhat-developer/rhdh/scripts/install-dynamic-plugins/install-dynamic-plugins.sh [1-18] redhat-developer/rhdh/e2e-tests/local-run.sh [279-320] redhat-developer/rhdh/e2e-tests/local-run.sh [216-261] redhat-developer/rhdh/e2e-tests/local-run.sh [123-153] redhat-developer/rhdh/e2e-tests/container-init.sh [86-117] redhat-developer/rhdh-operator/config/profile/rhdh/plugin-infra/plugin-infra.sh [1-49] redhat-developer/rhdh-operator/config/profile/rhdh/plugin-infra/gitops-secret-setup.sh [1-35]

[rhdh-qodo-merge]

PR Type

Bug fix, Enhancement

---

Description

• Add fallback mechanism to quay.io when registry.access.redhat.com unavailable

• Enables mirroring of unreleased plugins not yet published to primary registry

• Implements graceful error handling with informative logging for fallback attempts

• Maintains digest preservation and signature removal for both primary and fallback sources

---

File Walkthrough

Relevant files

Enhancement

mirror-plugins.sh Implement registry fallback logic for plugin mirroring      .rhdh/scripts/mirror-plugins.sh Define fallback and primary source registry constants for plugin mirroring Enhance mirror_image() function with two-stage copy strategy: attempt primary registry first, then fallback to quay.io Add conditional logic to detect registry.access.redhat.com references and substitute with quay.io on failure Implement detailed logging (warn, debug, error, info) for each fallback attempt and outcome +30/-2

---

[Fortune-Ndlovu]

/cc @rm3l for review please

[openshift-ci]

@Fortune-Ndlovu: GitHub didn't allow me to request PR reviews from the following users: for, review, please.

Note that only redhat-developer members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @rm3l for review please

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.

[rhdh-qodo-merge]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Limit fallback to the rhdh namespace Suggestion Impact: Updated the fallback condition to match only ${PRIMARY_SOURCE_REGISTRY}/rhdh/* and adjusted the accompanying comments to reflect the narrowed scope. code diff: - # If the source is registry.access.redhat.com, try falling back to quay.io - # This handles unreleased plugins that are only available on quay.io - if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/"* ]]; then + return 0 + fi + + # If the source is registry.access.redhat.com/rhdh, try falling back to quay.io/rhdh + # This handles unreleased RHDH plugins that are only available on quay.io + if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/rhdh/"* ]]; then Restrict the fallback logic to apply only to the rhdh namespace by changing the condition to match ${PRIMARY_SOURCE_REGISTRY}/rhdh/*. .rhdh/scripts/mirror-plugins.sh [312-324] -if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/"* ]]; then +if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/rhdh/"* ]]; then local fallback_ref="${docker_ref/${PRIMARY_SOURCE_REGISTRY}/${FALLBACK_SOURCE_REGISTRY}}" warnf "Failed to pull from ${PRIMARY_SOURCE_REGISTRY}, trying fallback: ${FALLBACK_SOURCE_REGISTRY}..." debugf "Fallback reference: $fallback_ref" if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$fallback_ref" "$dest"; then infof "Successfully mirrored using fallback registry: ${FALLBACK_SOURCE_REGISTRY}" return 0 else errorf "Failed to mirror from both ${PRIMARY_SOURCE_REGISTRY} and ${FALLBACK_SOURCE_REGISTRY}" return 1 fi fi [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a potential issue where the fallback logic is too broad and proposes a more specific condition, preventing incorrect fallback attempts for other images on the primary registry.	Medium
High-level	✅ Consider a more robust failure check Suggestion Impact: The script now captures skopeo stderr/exit code, reports detailed errors, and restricts fallback to specific "not found"/inaccessible patterns (e.g., "manifest unknown", "unauthorized", "not found") for images under the primary registry path, avoiding fallback on unrelated failures like network/auth issues. code diff: - # Try to copy from the original source - if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$docker_ref" "$dest" 2>/dev/null; then + # Try to copy from the original source, capturing error output for diagnostics + # Use || true to prevent set -e from exiting before we can check the exit code + local error_output + local exit_code + error_output=$(skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$docker_ref" "$dest" 2>&1) && exit_code=0 || exit_code=$? + + if [[ $exit_code -eq 0 ]]; then return 0 fi - # If the source is registry.access.redhat.com, try falling back to quay.io - # This handles unreleased plugins that are only available on quay.io - if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/"* ]]; then + # If the source is registry.access.redhat.com/rhdh, try falling back to quay.io/rhdh + # This handles unreleased RHDH plugins that are only available on quay.io + # Only attempt fallback for "not found" errors, not for network issues + # Note: registries may return "unauthorized" for non-existent repos (security measure) + if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/rhdh/"* ]] && \ + [[ "$error_output" == *"manifest unknown"* || "$error_output" == *"unauthorized"* || "$error_output" == *"not found"* ]]; then local fallback_ref="${docker_ref/${PRIMARY_SOURCE_REGISTRY}/${FALLBACK_SOURCE_REGISTRY}}" - warnf "Failed to pull from ${PRIMARY_SOURCE_REGISTRY}, trying fallback: ${FALLBACK_SOURCE_REGISTRY}..." + warnf "Image not accessible in ${PRIMARY_SOURCE_REGISTRY}, trying fallback: ${FALLBACK_SOURCE_REGISTRY}..." debugf "Fallback reference: $fallback_ref" if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$fallback_ref" "$dest"; then @@ -323,7 +331,8 @@ fi fi - # For other registries, just report the failure + # For all other errors (network, auth, etc.), report the original failure with details + errorf "Failed to mirror $docker_ref: $error_output" return 1 Improve error handling by inspecting the skopeo error message. The script should only attempt a fallback for "not found" errors, rather than any failure, to avoid masking issues like network or authentication problems. Examples: .rhdh/scripts/mirror-plugins.sh [306-324] if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$docker_ref" "$dest" 2>/dev/null; then return 0 fi # If the source is registry.access.redhat.com, try falling back to quay.io # This handles unreleased plugins that are only available on quay.io if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/"* ]]; then local fallback_ref="${docker_ref/${PRIMARY_SOURCE_REGISTRY}/${FALLBACK_SOURCE_REGISTRY}}" warnf "Failed to pull from ${PRIMARY_SOURCE_REGISTRY}, trying fallback: ${FALLBACK_SOURCE_REGISTRY}..." debugf "Fallback reference: $fallback_ref" ... (clipped 9 lines) Solution Walkthrough: Before: function mirror_image() { local src_image="$1" local dest="$2" ... # Try to copy, silencing all errors if skopeo copy ... "docker://$docker_ref" "$dest" 2>/dev/null; then return 0 fi # If the source is the primary registry, try falling back on ANY failure if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/"* ]]; then local fallback_ref="..." warnf "Failed to pull..., trying fallback..." if skopeo copy ... "docker://$fallback_ref" "$dest"; then return 0 else errorf "Failed to mirror from both..." return 1 fi fi return 1 } After: function mirror_image() { local src_image="$1" local dest="$2" ... # Try to copy, capturing error output error_output=$(skopeo copy ... "docker://$docker_ref" "$dest" 2>&1) exit_code=$? if [[ $exit_code -eq 0 ]]; then return 0 fi # Only try fallback if it's a "not found" error from the primary registry if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/"* && "$error_output" == *"manifest unknown"* ]]; then local fallback_ref="..." warnf "Failed to pull..., trying fallback..." skopeo copy ... "docker://$fallback_ref" "$dest" # ... handle success/failure of fallback else # For all other errors, report the original failure errorf "Failed to mirror: $error_output" return 1 fi } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that silencing all skopeo errors with 2>/dev/null can mask the true cause of failure, improving the script's robustness and diagnostic feedback.	Medium
General	✅ Add an explicit error message Suggestion Impact: The commit adds an explicit error log before returning 1 on failure, expanding it to include captured skopeo error output ("Failed to mirror $docker_ref: $error_output") to improve diagnostics for non-fallback cases. code diff: - # For other registries, just report the failure + # For all other errors (network, auth, etc.), report the original failure with details + errorf "Failed to mirror $docker_ref: $error_output" return 1 Add an explicit error message before returning on failure for non-primary registries to clarify that mirroring failed and no fallback was attempted. .rhdh/scripts/mirror-plugins.sh [326-327] # For other registries, just report the failure +errorf "Failed to mirror from ${docker_ref}" return 1 [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that failures for non-primary registries are silent due to error suppression, and proposes adding an explicit error message, which significantly improves debuggability.	Medium
Organization best practice	✅ Use arrays for command args Suggestion Impact: Replaced string-based optional flags (dest_flags/preserve_digests_flag) with an array (skopeo_flags) and invoked skopeo using "${skopeo_flags[@]}" for both the primary and fallback copy commands, preventing word-splitting problems. code diff: - # Add appropriate flags based on destination type - local dest_flags="" - local preserve_digests_flag="" + # Build skopeo flags as arrays to prevent word-splitting issues + local -a skopeo_flags=(--remove-signatures --all) if [[ "$dest" == docker://* ]]; then - dest_flags="--dest-tls-verify=false" + skopeo_flags+=(--dest-tls-verify=false) # Don't preserve digests for registry destinations to allow format conversion # This ensures compatibility with registries that require manifest format conversion # (e.g., OpenShift internal registry requiring OCI format) - preserve_digests_flag="" infof "Mirroring $src_image to ${dest#docker://}..." else # Preserve digests for directory destinations (offline transfer integrity) - preserve_digests_flag="--preserve-digests" + skopeo_flags+=(--preserve-digests) debugf "Saving $src_image to ${dest#dir:}..." fi - # Try to copy from the original source - if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$docker_ref" "$dest" 2>/dev/null; then + # Try to copy from the original source, capturing error output for diagnostics + # Use &&/|| to prevent set -e from exiting before we can check the exit code + local error_output + local exit_code + error_output=$(skopeo copy "${skopeo_flags[@]}" "docker://$docker_ref" "$dest" 2>&1) && exit_code=0 || exit_code=$? + + if [[ $exit_code -eq 0 ]]; then return 0 fi - # If the source is registry.access.redhat.com, try falling back to quay.io - # This handles unreleased plugins that are only available on quay.io - if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/"* ]]; then + # If the source is registry.access.redhat.com/rhdh, try falling back to quay.io/rhdh + # This handles unreleased RHDH plugins that are only available on quay.io + # Only attempt fallback for "not found" errors, not for network issues + if [[ "$docker_ref" == "${PRIMARY_SOURCE_REGISTRY}/rhdh/"* ]] && \ + [[ "$error_output" == *"manifest unknown"* || "$error_output" == *"unauthorized"* || "$error_output" == *"not found"* ]]; then local fallback_ref="${docker_ref/${PRIMARY_SOURCE_REGISTRY}/${FALLBACK_SOURCE_REGISTRY}}" - warnf "Failed to pull from ${PRIMARY_SOURCE_REGISTRY}, trying fallback: ${FALLBACK_SOURCE_REGISTRY}..." + + # Emit specific warning for "unauthorized" - could be credentials issue or non-existent repo + if [[ "$error_output" == *"unauthorized"* ]]; then + warnf "Got 'unauthorized' from ${PRIMARY_SOURCE_REGISTRY} - this may indicate missing credentials OR an unreleased image" + warnf "If fallback fails, verify your registry credentials are configured correctly" + fi + + warnf "Trying fallback registry: ${FALLBACK_SOURCE_REGISTRY}..." debugf "Fallback reference: $fallback_ref" - if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$fallback_ref" "$dest"; then + if skopeo copy "${skopeo_flags[@]}" "docker://$fallback_ref" "$dest"; then infof "Successfully mirrored using fallback registry: ${FALLBACK_SOURCE_REGISTRY}" Build the skopeo copy arguments as an array and append optional flags conditionally, so empty or multi-word flags don’t break the command line. .rhdh/scripts/mirror-plugins.sh [306-319] -if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$docker_ref" "$dest" 2>/dev/null; then +local -a skopeo_args=(copy --remove-signatures --all) +[[ -n "${preserve_digests_flag:-}" ]] && skopeo_args+=("${preserve_digests_flag}") +# If dest_flags is already an array elsewhere, keep it as such; otherwise split it into an array once upstream. +skopeo_args+=(${dest_flags}) + +if skopeo "${skopeo_args[@]}" "docker://${docker_ref}" "${dest}" 2>/dev/null; then return 0 fi ... -if skopeo copy $preserve_digests_flag --remove-signatures --all $dest_flags "docker://$fallback_ref" "$dest"; then +if skopeo "${skopeo_args[@]}" "docker://${fallback_ref}" "${dest}"; then infof "Successfully mirrored using fallback registry: ${FALLBACK_SOURCE_REGISTRY}" return 0 [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Avoid unquoted variable expansions in shell commands (SC2086); use arrays for optional flags to prevent word-splitting/globbing bugs.	Low
Update

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rm3l]

/review

[rhdh-qodo-merge]

Persistent review updated to latest commit e21114c

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud