Skip to content

fix: use oc-mirror native catalogs for OLM v1 install - #3259

Merged
openshift-merge-bot[bot] merged 3 commits into
redhat-developer:mainfrom
zdrapela:fix/oc-mirror-olm-v1-native-catalog
Jul 24, 2026
Merged

fix: use oc-mirror native catalogs for OLM v1 install#3259
openshift-merge-bot[bot] merged 3 commits into
redhat-developer:mainfrom
zdrapela:fix/oc-mirror-olm-v1-native-catalog

Conversation

@zdrapela

Copy link
Copy Markdown
Member

Skip re-applying a synthetic clusterCatalog.yaml after oc-mirror already applied cc-.yaml, harden missing catalog errors for v0/v1, and document --olm-version plus native cc-/cs-* manifests in the airgap docs.

Description

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

  • Fixes #issue_number

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

Building Container Images for Testing

Need to test container images from this PR?

For Maintainers: To trigger a test image build, review the code and comment /build-images.
This always builds the HEAD of the PR branch.

For Contributors: Ask a maintainer to run /build-images.

Images will be built and pushed to Quay with links posted in comments.

Skip re-applying a synthetic clusterCatalog.yaml after oc-mirror already
applied cc-*.yaml, harden missing catalog errors for v0/v1, and document
--olm-version plus native cc-*/cs-* manifests in the airgap docs.
zdrapela added 2 commits July 23, 2026 10:06
Match cc-*/cs-* by targetCatalog, fail when cluster-resources is missing,
skip synthetic clusterCatalog only after a successful apply, and clarify
INSTALL_OPERATOR=false hints for cc/cs fallbacks and ephemeral TMPDIR.
…ests

Fail on ambiguous cc/cs catalogs, use prefix-style oc-mirror naming, and keep synthetic install resources aligned with TARGET_CATALOG.
@sonarqubecloud

Copy link
Copy Markdown

@zdrapela
zdrapela marked this pull request as ready for review July 24, 2026 10:08
@zdrapela
zdrapela requested a review from a team as a code owner July 24, 2026 10:08
@openshift-ci
openshift-ci Bot requested review from rm3l and subhashkhileri July 24, 2026 10:08
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Fix oc-mirror native catalog handling for OLM v1 installs

🐞 Bug fix 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Prefer oc-mirror native cc-*/cs-* catalogs and skip synthetic ClusterCatalog re-apply.
• Fail fast when cluster-resources/catalog manifests are missing or ambiguous.
• Document --olm-version behavior and oc-mirror catalog manifest locations for airgapped installs.
Diagram

graph TD
  u([User]) --> s["prepare-restricted-environment.sh"] --> om["oc-mirror v2"] --> cr[("cluster-resources/")] --> sel{"Match TARGET_CATALOG"}
  sel -->|"OLM v1"| ccv1["Apply cc-* (or convert cs-*)"] --> inst["Apply install manifests"]
  sel -->|"OLM v0"| csv0["Apply cs-*"] --> inst
  s --> docs["airgap.adoc"]
  subgraph Legend
    direction LR
    _proc["Process"] ~~~ _dec{"Decision"} ~~~ _dir[("Directory")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Derive targetCatalog dynamically from imageset-config.yaml
  • ➕ Avoids needing TARGET_CATALOG to be kept in sync with ImageSetConfiguration generation
  • ➕ Allows future multi-catalog support by iterating configured entries
  • ➖ Requires more parsing/validation logic and careful handling for multiple configured catalogs
  • ➖ More moving parts for a shell script; harder to debug than a single explicit variable
2. Match catalog manifests by YAML content (metadata.name/spec.image) rather than filename
  • ➕ More resilient if oc-mirror naming patterns change
  • ➕ Can validate that the selected manifest actually points at the expected mirrored image
  • ➖ Still needs strict ambiguity handling when multiple manifests exist
  • ➖ May require more yq/jq logic and can be slower/more error-prone in bash

Recommendation: The PR’s approach (strict filename-based selection using oc-mirror’s documented cc|cs--*.yaml convention, plus hard failures on missing/ambiguous catalogs) is a good fit for an airgap install script: it reduces silent misconfiguration and avoids re-applying a synthetic ClusterCatalog after oc-mirror already applied the authoritative manifest. Consider dynamic derivation of targetCatalog only if/when multi-catalog or configurable catalogs become a requirement.

Files changed (2) +184 / -62

Bug fix (1) +170 / -59
prepare-restricted-environment.shSelect and apply oc-mirror catalog manifests by targetCatalog; skip synthetic re-apply +170/-59

Select and apply oc-mirror catalog manifests by targetCatalog; skip synthetic re-apply

• Introduces TARGET_CATALOG and a strict selector for oc-mirror catalog manifests (cc|cs-<targetCatalog>-*.yaml) with hard errors for missing/ambiguous matches. Applies the selected cc/cs manifest (or converts cs→ClusterCatalog for v1) and sets a flag so the install step skips applying the synthetic clusterCatalog.yaml only after a successful oc-mirror catalog apply; also hard-fails when cluster-resources is missing and improves install hints for durable export paths.

.rhdh/scripts/prepare-restricted-environment.sh

Documentation (1) +14 / -3
airgap.adocDocument --olm-version and oc-mirror native cc/cs manifests +14/-3

Document --olm-version and oc-mirror native cc/cs manifests

• Adds guidance for the new/clarified --olm-version behavior (auto/v0/v1). Documents that when using oc-mirror, catalog resources come from working-dir/cluster-resources/ as cc-*.yaml (OLM v1) or cs-*.yaml (OLM v0).

.rhdh/docs/airgap.adoc

@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 18 rules
✅ Cross-repo context
  Explored: repo: redhat-developer/rhdh (sha: d090bd1a)
  Not relevant to this PR: redhat-developer/rhdh-plugins

Grey Divider


Action required

1. CatalogSource manifests ignored 🐞 Bug ≡ Correctness
Description
select_target_catalog_manifest() only treats basenames starting with cc- or cs- as matches, but it
is called with catalogSource*.yaml candidates. If the oc-mirror workspace contains only
catalogSource*.yaml (or no cs-* files), the selector returns no match and the script exits with a
missing-catalog error, breaking the oc-mirror install flow.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R535-537]

+      # Canonical: cc-<TARGET_CATALOG>-<suffix>.yaml / cs-<TARGET_CATALOG>-<suffix>.yaml
+      if [[ "${base}" == cc-"${TARGET_CATALOG}"-*.yaml || "${base}" == cs-"${TARGET_CATALOG}"-*.yaml ]]; then
+        matched+=("${candidate}")
Relevance

⭐⭐⭐ High

Matches recent oc-mirror/OLM catalog handling work; team accepts correctness fixes around
cc/cs/catalogSource selection.

PR-#3046

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The selector function’s matching condition only accepts cc-/cs- prefixed filenames, but the
oc-mirror catalog selection passes catalogSource*.yaml as candidates and then hard-errors when no
match is found.

.rhdh/scripts/prepare-restricted-environment.sh[515-563]
.rhdh/scripts/prepare-restricted-environment.sh[1325-1370]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`select_target_catalog_manifest` currently only recognizes candidate files whose **filename** matches `cc-${TARGET_CATALOG}-*.yaml` or `cs-${TARGET_CATALOG}-*.yaml`. However, the oc-mirror code path also passes `catalogSource*.yaml` candidates (to support alternate/legacy oc-mirror output naming), which the selector will never match. This can cause hard failures in both:
- OLM v0 catalog application, and
- OLM v1 fallback path that converts a CatalogSource into a ClusterCatalog.

## Issue Context
The script explicitly tries to support both `cs-*.yaml` and `catalogSource*.yaml` patterns, but the selection predicate only matches `cc-*/cs-*` basenames.

## Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[515-563]
- .rhdh/scripts/prepare-restricted-environment.sh[1311-1371]

## Implementation guidance
Update `select_target_catalog_manifest` so that it can successfully select a manifest when the only available catalog manifest is named like `catalogSource*.yaml`. Prefer one of:
1. **Content-based match (recommended):** use `yq` to read each candidate and match on `.kind` and `.metadata.name == "${TARGET_CATALOG}"` (and optionally ensure kind is `CatalogSource`/`ClusterCatalog` depending on the caller), rather than relying solely on filename.
2. **Filename-based support:** extend the basename pattern checks to also accept `catalogSource-${TARGET_CATALOG}-*.yaml` (and/or whatever exact naming you intend to support).

Also ensure the warning/error messaging stays consistent with the supported naming forms.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation Bug fix labels Jul 24, 2026
@openshift-ci openshift-ci Bot added the lgtm label Jul 24, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit e45875e into redhat-developer:main Jul 24, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix documentation Improvements or additions to documentation lgtm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants