Skip to content

RHIDP-14785: Add OLM v1 code path to prepare-restricted-environment.sh#3046

Open
Fortune-Ndlovu wants to merge 31 commits into
redhat-developer:mainfrom
Fortune-Ndlovu:RHIDP-14785-operator-add-olm-v-1-path-to-prepare-restricted-environment-sh
Open

RHIDP-14785: Add OLM v1 code path to prepare-restricted-environment.sh#3046
Fortune-Ndlovu wants to merge 31 commits into
redhat-developer:mainfrom
Fortune-Ndlovu:RHIDP-14785-operator-add-olm-v-1-path-to-prepare-restricted-environment-sh

Conversation

@Fortune-Ndlovu

@Fortune-Ndlovu Fortune-Ndlovu commented Jun 23, 2026

Copy link
Copy Markdown
Member

Adds an --olm-version v0|v1|auto CLI flag (default: auto) to prepare-restricted-environment.sh with CRD-based OLM v1 detection. When OLM v1 is detected (or forced), the script creates a ClusterCatalog and ClusterExtension with a ServiceAccount and ClusterRoleBinding, instead of the OLM v0 CatalogSource, Subscription, and OperatorGroup. Resolution is pinned to the custom catalog via selector.matchLabels, and CRD upgrade safety preflight is disabled (enforcement: None) per the known blocker RHIDP-8656. The OLM v0 path is fully preserved for backward compatibility on older clusters.

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

Resolves: https://redhat.atlassian.net/browse/RHIDP-14785

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

launch 4.22.0-0.nightly aws,techpreview

# Export to dir
bash .rhdh/scripts/prepare-restricted-environment.sh \
  --to-dir /tmp/rhdh-export-test \
  --olm-version v1 \
  --filter-versions '1.10' \
  --install-operator false

# Push from dir to OCP internal registry and install
/tmp/rhdh-export-test/install.sh --from-dir /tmp/rhdh-export-test/ --install-operator true

Signed-off-by: Fortune-Ndlovu fndlovu@redhat.com
Assisted-by: Claude (claude-opus-4-6)

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Add OLM v1 support to install-rhdh-catalog-source.sh with auto-detection
✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

Description

• Add --olm-version v0|v1|auto to select or auto-detect OLM installation mode.
• Auto-detect OLM v1 via ClusterExtension CRD and switch resource types accordingly.
• Preserve existing OLM v0 CatalogSource/Subscription behavior for backward compatibility.
Diagram

graph TD
  A["install-rhdh-catalog-source.sh"] --> B["Parse flags"] --> C["resolve_olm_version()"] --> D["Render/Rebuild IIB"] --> E{"OLM v1?"}
  E -->|"Yes"| F["Create ClusterCatalog"] --> G["Create SA + CRB"] --> H["Create ClusterExtension"]
  E -->|"No"| I["Create CatalogSource"] --> J["Create OperatorGroup"] --> K["Create Subscription"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use least-privilege RBAC for the OLM v1 installer SA
  • ➕ Reduces security exposure versus granting cluster-admin
  • ➕ More aligned with typical cluster hardening policies
  • ➖ Requires deeper knowledge of exact permissions ClusterExtension needs
  • ➖ May increase maintenance as OLM v1 evolves
2. Split OLM v0 and OLM v1 flows into separate scripts/subcommands
  • ➕ Simplifies each code path and reduces branching complexity
  • ➕ Easier to test and reason about version-specific behavior
  • ➖ Duplicates shared setup/IIB logic unless refactored into a library file
  • ➖ Worse UX if users must pick the correct script manually
3. Generate manifests from templates and apply as a single bundle
  • ➕ Reduces inline heredoc volume and improves readability
  • ➕ Easier to diff/validate YAML and reuse across automation
  • ➖ Introduces template management overhead
  • ➖ May complicate parameter substitution in pure bash

Recommendation: The current approach (auto-detect + branching while sharing the IIB phase) is the right compatibility strategy for mixed clusters. The main strategic follow-up to consider is tightening the OLM v1 RBAC: the script currently grants cluster-admin to the installer ServiceAccount, which is operationally convenient but high-risk. If least-privilege is not feasible immediately, document the rationale/constraints and consider gating that behavior behind an explicit flag.

Files changed (1) +244 / -51

Enhancement (1) +244 / -51
install-rhdh-catalog-source.shAdd OLM v1 auto-detect + ClusterCatalog/ClusterExtension install path +244/-51

Add OLM v1 auto-detect + ClusterCatalog/ClusterExtension install path

• Introduces a new '--olm-version v0|v1|auto' flag (default auto) and resolves the effective OLM mode via CRD detection on OpenShift. Adds a full OLM v1 installation path that creates ClusterCatalog (catalogd), plus ServiceAccount/ClusterRoleBinding and ClusterExtension, while preserving the existing OLM v0 CatalogSource/OperatorGroup/Subscription flow and keeping the IIB rebuild/render step shared.

.rhdh/scripts/install-rhdh-catalog-source.sh

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 18 rules

Grey Divider


Action required

1. --olm-version says catalogd running ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The --olm-version help text claims auto-detection checks for a running catalogd deployment, but
the implementation effectively treats the existence of a namespace (including defaulting to
openshift-catalogd) as sufficient, even when catalogd may not be installed or ready. This
mismatch can mislead users and can cause auto mode to incorrectly select OLM v1 and generate/apply
v1 resources on clusters where catalogd isn’t actually running, breaking the installation flow.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R119-120]

+                                            'auto' detects OLM v1 by checking for the ClusterExtension CRD and
+                                            a running catalogd deployment on the cluster.
Relevance

⭐⭐⭐ High

Team has accepted multiple fixes aligning usage/help text with real behavior in this script (e.g.,
PR #1646, #1480).

PR-#1646
PR-#1480

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The usage/help text explicitly promises that auto-detection checks for a “running catalogd
deployment,” but the current detect_olm_v1_catalogd logic returns success based on a `get
namespace check rather than confirming a catalogd` Deployment exists and is Available; it derives
a namespace from a matching deployment if present, otherwise falls back to openshift-catalogd and
still succeeds if that namespace exists. Because resolve_olm_version uses this success signal in
auto mode, a namespace that exists without a running/ready catalogd can incorrectly drive
selection of v1 and lead to v1 resources being generated/applied.

.rhdh/scripts/prepare-restricted-environment.sh[118-120]
.rhdh/scripts/prepare-restricted-environment.sh[356-364]
.rhdh/scripts/prepare-restricted-environment.sh[392-400]
Best Practice: Repository guidelines

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

## Issue description
The `--olm-version` usage text states that auto-detection checks for a running `catalogd` deployment, but `detect_olm_v1_catalogd()` currently succeeds based only on namespace existence (including defaulting to `openshift-catalogd`). This creates a documentation/behavior mismatch and can also cause `resolve_olm_version` to choose `v1` in `auto` mode even when no `catalogd` Deployment is present/ready, breaking the installation flow.

## Issue Context
- The usage text promises auto-detection checks for “a running catalogd deployment”.
- `detect_olm_v1_catalogd` derives a namespace from the first matching deployment (if any) and falls back to `openshift-catalogd`, then returns success if the namespace exists.
- This success result is used by `resolve_olm_version` as a deciding signal to select OLM v1.

## Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[118-122]
- .rhdh/scripts/prepare-restricted-environment.sh[356-365]

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


2. v1 instructions miss ClusterCatalog ✓ Resolved 🐞 Bug ≡ Correctness
Description
When RESOLVED_OLM_VERSION==v1 and INSTALL_OPERATOR is false, the script prints manual apply steps
that omit clusterCatalog.yaml even though it is generated for the v1 path. Users following the
printed commands can end up applying a ClusterExtension that references a catalog that was never
created, leaving the extension unresolved.
Code

.rhdh/scripts/prepare-restricted-environment.sh[1614]

+      kubectl apply -f ${manifestsTargetDir}/clusterRole.yaml
Relevance

⭐⭐⭐ High

They frequently accept correctness fixes preventing broken install flows in this script (e.g., PR
#2423, #1646).

PR-#2423
PR-#1646

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script generates a ClusterCatalog manifest for OLM v1, but the user-facing v1 install
instructions only list namespace/serviceAccount/clusterRole/clusterRoleBinding/clusterExtension and
omit ClusterCatalog.

.rhdh/scripts/prepare-restricted-environment.sh[1276-1288]
.rhdh/scripts/prepare-restricted-environment.sh[1609-1617]

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

### Issue description
The OLM v1 manual install instructions printed when `INSTALL_OPERATOR!=true` do not include `clusterCatalog.yaml`, even though the script generates it for the v1 flow. Following the instructions as-is can produce an unresolved `ClusterExtension` due to the missing catalog.

### Issue Context
`clusterCatalog.yaml` is generated under the v1 manifest generation branch, but the printed list of `kubectl apply -f ...` commands omits it.

### Fix Focus Areas
- Update the printed OLM v1 instructions to include applying `clusterCatalog.yaml` (ideally before `clusterExtension.yaml`).
- (Optional but recommended) Also include `clusterCatalog` in the v1 manifest apply loop for consistency/idempotency.

### Fix Focus Areas (code references)
- .rhdh/scripts/prepare-restricted-environment.sh[1609-1617]
- .rhdh/scripts/prepare-restricted-environment.sh[1635-1638]

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


3. Hardcoded ClusterCatalog pullSecret ✓ Resolved 🐞 Bug ≡ Correctness
Description
The OLM v1 ClusterCatalog manifest hardcodes pullSecret: internal-reg-auth-for-rhdh, but the
script only creates that secret in the OCP_INTERNAL registry flow. With `--to-registry
<external-registry>` (or any non-OCP_INTERNAL mirror), the script can apply a ClusterCatalog that
references a secret that was never created, preventing catalogd from pulling the catalog image.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R1240-1250]

+apiVersion: catalogd.operatorframework.io/v1
+kind: ClusterCatalog
+metadata:
+  name: rhdh-catalog
+spec:
+  source:
+    type: Image
+    image:
+      ref: ${my_operator_index}
+      pullSecret: internal-reg-auth-for-rhdh
+EOF
Relevance

⭐⭐⭐ High

Team often accepts correctness fixes in restricted-env scripts; similar flow-breaking issues were
fixed in PRs 2423/1646.

PR-#2423
PR-#1646
PR-#963

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script always writes ClusterCatalog with pullSecret: internal-reg-auth-for-rhdh, but only
creates that secret when using the OCP internal registry (via ocp_prepare_internal_registry /
prepare_olm_v1_secrets, which are invoked only for TO_REGISTRY=OCP_INTERNAL). Therefore, for
non-OCP_INTERNAL --to-registry flows, the ClusterCatalog can reference a missing secret.

.rhdh/scripts/prepare-restricted-environment.sh[1238-1250]
.rhdh/scripts/prepare-restricted-environment.sh[964-980]
.rhdh/scripts/prepare-restricted-environment.sh[382-419]
.rhdh/scripts/prepare-restricted-environment.sh[494-534]

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

### Issue description
The OLM v1 `ClusterCatalog` YAML is generated (and also created during oc-mirror conversion) with a hardcoded `pullSecret: internal-reg-auth-for-rhdh`. That secret is only created when `TO_REGISTRY=OCP_INTERNAL`, so for `--to-registry <external>` the applied `ClusterCatalog` can reference a non-existent secret and fail to pull.

### Issue Context
- In OLM v0, the CatalogSource includes multiple secrets including a user-provided `reg-pull-secret` option; the v1 path currently doesn’t provide an equivalent.
- For OLM v1, the pull secret likely needs to exist in the namespace where catalogd expects it (the script already special-cases this for OCP internal by creating secrets in the detected catalogd namespace).

### Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[1238-1250]
- .rhdh/scripts/prepare-restricted-environment.sh[1136-1151]
- .rhdh/scripts/prepare-restricted-environment.sh[964-980]
- .rhdh/scripts/prepare-restricted-environment.sh[382-419]

### Suggested implementation direction
- Introduce a variable (and optionally a CLI flag) for the v1 catalog pull secret name, e.g. `CATALOG_PULL_SECRET`.
- Default behavior:
 - If `TO_REGISTRY=OCP_INTERNAL`, keep using/creating `internal-reg-auth-for-rhdh` (current behavior).
 - Otherwise, either:
   - omit `pullSecret` entirely (if you want to support public registries by default), or
   - default it to `reg-pull-secret` and document that the user must create it in the catalogd namespace.
- Ensure any auto-created secret is created in the correct namespace for OLM v1 (catalogd namespace), not just the operator namespace.

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


View more (1)
4. cluster-admin ClusterRoleBinding created ✓ Resolved 📘 Rule violation ⛨ Security
Description
The script generates OLM v1 install manifests that create a ClusterRoleBinding binding the
installer namespaced ServiceAccount to the built-in cluster-admin ClusterRole, granting full
cluster-wide privileges. This RBAC is overly broad for managed resources and substantially increases
the blast radius and risk of privilege escalation if the ServiceAccount token is misused, especially
compared to the OLM v0 flow where no such cluster-admin binding is created.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R1402-1415]

+  cat <<EOF >"${manifestsTargetDir}/clusterRoleBinding.yaml"
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: ${CRB_NAME}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: cluster-admin
+subjects:
+- kind: ServiceAccount
+  name: ${SA_NAME}
+  namespace: ${NAMESPACE_OPERATOR}
+EOF
Relevance

⭐⭐ Medium

No direct precedent on rejecting cluster-admin bindings; “least-privilege” requests sometimes
rejected (e.g., PR 2141).

PR-#2141
PR-#1187

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 8 requires RBAC to be aligned to required operations and not overly broad; the
generated OLM v1 manifest violates this by explicitly setting roleRef.name: cluster-admin in the
ClusterRoleBinding for the installer ServiceAccount. Because this creates a persistent,
cluster-wide privilege grant to that ServiceAccount, any actor/workload able to use its token would
inherit full cluster-admin permissions beyond the minimal verbs/resources needed.

Rule 8: Ensure operator RBAC includes required verbs for managed Kubernetes resource kinds
.rhdh/scripts/prepare-restricted-environment.sh[1402-1415]

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

## Issue description
The generated OLM v1 installer RBAC creates a `ClusterRoleBinding` (via `clusterRoleBinding.yaml`) that binds the installer namespaced ServiceAccount to the built-in `cluster-admin` ClusterRole, granting permanent, full cluster-wide privileges; this is overly privileged, increases blast radius, and enables privilege escalation if the ServiceAccount token is used outside its intended purpose.

## Issue Context
Per compliance requirements, shipped RBAC should be scoped to the minimal apiGroups/resources/verbs needed by the installer/ClusterExtension installation workflow rather than using broad `cluster-admin` permissions. This binding is created whenever OLM v1 manifests are generated/applied, and even if the script is typically run by cluster admins, the resulting RBAC object persists and can be abused later by any principal that can run pods as that ServiceAccount in the operator namespace; additionally, this is a regression in security posture compared to the OLM v0 flow, where no cluster-admin binding is created.

## Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[1402-1415]

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



Remediation recommended

5. catalogd timeout too short 🐞 Bug ☼ Reliability ⭐ New
Description
detect_olm_v1_catalogd uses rollout status ... --timeout=5s, which can easily time out on
normal/slow clusters and cause --olm-version auto to incorrectly resolve to OLM v0. This leads the
script to generate/apply the wrong OLM resource types (v0) even though the cluster supports v1.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R359-367]

+function detect_olm_v1_catalogd() {
+  local ns
+  ns=$(invoke_cluster_cli get deployment -A -l 'app.kubernetes.io/name=catalogd' \
+    -o jsonpath='{.items[0].metadata.namespace}' 2>/dev/null || true)
+  if [[ -z "${ns}" ]]; then
+    return 1
+  fi
+  invoke_cluster_cli rollout status deployment -n "${ns}" -l 'app.kubernetes.io/name=catalogd' --timeout=5s &>/dev/null
+}
Relevance

⭐⭐⭐ High

Team previously fixed script flakiness by increasing waits/timeouts (e.g., 300s) for cluster
readiness.

PR-#2082

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The readiness check is explicitly capped at 5 seconds, and resolve_olm_version treats any failure
as “catalogd not ready” and falls back to v0, which can be triggered by a timeout rather than actual
absence/unreadiness.

.rhdh/scripts/prepare-restricted-environment.sh[359-367]
.rhdh/scripts/prepare-restricted-environment.sh[388-395]

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

### Issue description
`detect_olm_v1_catalogd()` uses a hardcoded `--timeout=5s` for `rollout status`. This is brittle and can produce false negatives, causing `resolve_olm_version()` to fall back to v0 even when catalogd is present but taking slightly longer to become ready.

### Issue Context
In `--olm-version auto` mode, this timeout directly controls whether the script selects the OLM v1 code path. A short timeout increases flakiness and can make installs non-deterministic depending on cluster load.

### Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[359-367]
- .rhdh/scripts/prepare-restricted-environment.sh[388-395]

### Suggested fix approach
- Replace the `--timeout=5s` readiness check with a more tolerant approach (e.g., `--timeout=60s`, or a small retry loop totaling ~60s).
- Prefer `kubectl/oc wait --for=condition=Available ...` (or an equivalent readiness condition) to avoid `rollout status` edge cases.
- Keep the failure fast in `--olm-version v1` (forced) mode, but allow reasonable time in `auto` mode before falling back to v0.

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


6. Incomplete OLM v1 detection ✓ Resolved 🐞 Bug ☼ Reliability
Description
--olm-version auto resolves to v1 solely by checking the ClusterExtension CRD, but the v1 code
path later requires a catalogd namespace/deployment and exits if it’s missing. This can select v1
and then fail late (after other work), instead of deterministically falling back to v0 or failing
early with a clear readiness check.
Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R118-153]

+function detect_olm_v1() {
+  invoke_cluster_cli get crd clusterextensions.olm.operatorframework.io &> /dev/null
+}
+
+function resolve_olm_version() {
+  set -euo pipefail
+
+  if [[ "${IS_OPENSHIFT}" != "true" ]]; then
+    if [[ "${OLM_VERSION}" == "v1" ]]; then
+      warnf "OLM v1 is not supported on Kubernetes clusters; falling back to v0"
+    fi
+    RESOLVED_OLM_VERSION="v0"
+    return
+  fi
+
+  if [[ "${OLM_VERSION}" == "v0" ]]; then
+    RESOLVED_OLM_VERSION="v0"
+    infof "Using OLM v0 (forced via --olm-version)"
+  elif [[ "${OLM_VERSION}" == "v1" ]]; then
+    if ! detect_olm_v1; then
+      errorf "OLM v1 requested but ClusterExtension CRD not found on this cluster"
+      exit 1
+    fi
+    RESOLVED_OLM_VERSION="v1"
+    infof "Using OLM v1 (forced via --olm-version)"
+  else
+    # auto-detect
+    if detect_olm_v1; then
+      RESOLVED_OLM_VERSION="v1"
+      infof "Auto-detected OLM v1 (ClusterExtension CRD found)"
+    else
+      RESOLVED_OLM_VERSION="v0"
+      infof "Auto-detected OLM v0 (ClusterExtension CRD not found)"
+    fi
+  fi
+}
Relevance

⭐⭐⭐ High

Team often adds early preflight checks to avoid late failures in scripts (fail-fast pattern).

PR-#1037
PR-#2082

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The detection function checks only the ClusterExtension CRD, but the v1 branch subsequently requires
a catalogd namespace to exist and exits 1 if it does not. Therefore auto-detection can choose v1 in
states where the script cannot proceed with v1 resources.

.rhdh/scripts/install-rhdh-catalog-source.sh[118-153]
.rhdh/scripts/install-rhdh-catalog-source.sh[877-890]

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

### Issue description
Auto-detection considers OLM v1 “present” if the ClusterExtension CRD exists, but the v1 execution path also depends on catalogd being installed/running and discoverable. This mismatch causes avoidable late failures.

### Issue Context
- `detect_olm_v1()` only checks for `clusterextensions.olm.operatorframework.io`.
- The v1 path later discovers/validates catalogd namespace and aborts if it’s missing.

### Fix
- Strengthen `detect_olm_v1()` (or the `auto` branch in `resolve_olm_version`) to validate all prerequisites used by the v1 path, e.g.:
 - ClusterExtension CRD exists, **and**
 - a catalogd deployment is discoverable (label query returns at least one item), **and**
 - the discovered/default catalogd namespace exists.
- If prerequisites are not met:
 - for `auto`: set `RESOLVED_OLM_VERSION=v0` and log a warning about incomplete v1 installation;
 - for forced `v1`: keep the current hard error.

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[118-153]
- .rhdh/scripts/install-rhdh-catalog-source.sh[877-890]

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


7. Unguarded olm-version value ✓ Resolved 🐞 Bug ≡ Correctness
Description
The new --olm-version option dereferences $2 under set -u without verifying an argument
exists, so ... --olm-version (missing value) will terminate with an “unbound variable” shell error
instead of showing a controlled validation/usage message. This makes the script brittle for
automation that conditionally appends flags.
Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R747-755]

+    '--olm-version')
+      if [[ "$2" != "v0" && "$2" != "v1" && "$2" != "auto" ]]; then
+        errorf "Unknown OLM version: $2. Must be v0, v1, or auto."
+        usage
+        exit 1
+      fi
+      OLM_VERSION="$2"
+      shift 1
+      ;;
Relevance

⭐⭐⭐ High

Repo frequently accepts hardening CLI arg validation under set -euo (avoid brittle runtime shell
errors).

PR-#2870
PR-#1037

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script enables set -euo pipefail globally, and the new --olm-version parsing branch reads
$2 unconditionally, which triggers an unbound-variable error when the flag is provided without a
value.

.rhdh/scripts/install-rhdh-catalog-source.sh[7-8]
.rhdh/scripts/install-rhdh-catalog-source.sh[719-767]

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

### Issue description
`--olm-version` reads `$2` without checking that it exists. With `set -euo pipefail`, missing `$2` crashes the script with an unbound-variable error.

### Issue Context
This impacts CLI robustness and is newly introduced by adding the `--olm-version` flag.

### Fix
- In the `--olm-version` case arm, add an argc check before accessing `$2` (e.g., `if [[ $# -lt 2 ]]; then ... usage; exit 1; fi`).
- (Optional but consistent) apply the same pattern to other flags that require a value (`--install-operator`, `--catalog-source`, `--install-plan-approval`, `-v`).

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[719-767]

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



Informational

8. oc policy via wrapper ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New
Description
prepare_olm_v1_secrets calls invoke_cluster_cli policy add-role-to-user ..., but
invoke_cluster_cli may execute kubectl when oc is not installed; kubectl has no policy
subcommand, yielding a confusing failure mode. This makes the helper abstraction unsafe for
OpenShift-only commands and can break the OLM v1 internal-registry setup path.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R448-449]

+  invoke_cluster_cli policy add-role-to-user system:image-puller "system:serviceaccount:${NAMESPACE_CATALOGD}:default" -n "${NAMESPACE_CATALOGD}" >&2 || true
+  invoke_cluster_cli policy add-role-to-user system:image-puller "system:serviceaccount:${NAMESPACE_CATALOGD}:${catalogd_sa}" -n "${NAMESPACE_CATALOGD}" >&2 || true
Relevance

⭐⭐⭐ High

OpenShift-specific flows already call oc directly; likely accept guarding policy when wrapper
may use kubectl.

PR-#3001

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new code calls an OpenShift-only policy add-role-to-user subcommand through a wrapper that can
dispatch to kubectl when oc is missing, making this command path non-portable and prone to
confusing failures.

.rhdh/scripts/prepare-restricted-environment.sh[336-352]
.rhdh/scripts/prepare-restricted-environment.sh[440-449]

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

### Issue description
`prepare_olm_v1_secrets()` uses `invoke_cluster_cli policy add-role-to-user ...` to grant `system:image-puller` to catalogd service accounts. The wrapper can fall back to `kubectl`, which will fail because `kubectl policy ...` is not a thing.

### Issue Context
This logic is OpenShift-specific (it’s about pulling from the OpenShift internal registry). The safest/most portable way to do this is to apply an RBAC `RoleBinding` in the catalogd namespace, which works via both `oc` and `kubectl`.

### Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[440-449]

### Suggested fix approach
- Replace the `oc policy add-role-to-user` calls with creation/apply of a `RoleBinding` in `${NAMESPACE_CATALOGD}` that binds `ClusterRole system:image-puller` to the `default` and `${catalogd_sa}` service accounts.
 - This avoids reliance on OpenShift CLI subcommands and keeps `invoke_cluster_cli` semantics consistent.
- Alternatively (less preferred), explicitly call `oc policy ...` here and hard-fail early with a clear error if `oc` is unavailable.

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


Grey Divider

Previous review results

Review updated until commit 7a37760

Results up to commit cd08778


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Incomplete OLM v1 detection ✓ Resolved 🐞 Bug ☼ Reliability
Description
--olm-version auto resolves to v1 solely by checking the ClusterExtension CRD, but the v1 code
path later requires a catalogd namespace/deployment and exits if it’s missing. This can select v1
and then fail late (after other work), instead of deterministically falling back to v0 or failing
early with a clear readiness check.
Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R118-153]

+function detect_olm_v1() {
+  invoke_cluster_cli get crd clusterextensions.olm.operatorframework.io &> /dev/null
+}
+
+function resolve_olm_version() {
+  set -euo pipefail
+
+  if [[ "${IS_OPENSHIFT}" != "true" ]]; then
+    if [[ "${OLM_VERSION}" == "v1" ]]; then
+      warnf "OLM v1 is not supported on Kubernetes clusters; falling back to v0"
+    fi
+    RESOLVED_OLM_VERSION="v0"
+    return
+  fi
+
+  if [[ "${OLM_VERSION}" == "v0" ]]; then
+    RESOLVED_OLM_VERSION="v0"
+    infof "Using OLM v0 (forced via --olm-version)"
+  elif [[ "${OLM_VERSION}" == "v1" ]]; then
+    if ! detect_olm_v1; then
+      errorf "OLM v1 requested but ClusterExtension CRD not found on this cluster"
+      exit 1
+    fi
+    RESOLVED_OLM_VERSION="v1"
+    infof "Using OLM v1 (forced via --olm-version)"
+  else
+    # auto-detect
+    if detect_olm_v1; then
+      RESOLVED_OLM_VERSION="v1"
+      infof "Auto-detected OLM v1 (ClusterExtension CRD found)"
+    else
+      RESOLVED_OLM_VERSION="v0"
+      infof "Auto-detected OLM v0 (ClusterExtension CRD not found)"
+    fi
+  fi
+}
Relevance

⭐⭐⭐ High

Team often adds early preflight checks to avoid late failures in scripts (fail-fast pattern).

PR-#1037
PR-#2082

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The detection function checks only the ClusterExtension CRD, but the v1 branch subsequently requires
a catalogd namespace to exist and exits 1 if it does not. Therefore auto-detection can choose v1 in
states where the script cannot proceed with v1 resources.

.rhdh/scripts/install-rhdh-catalog-source.sh[118-153]
.rhdh/scripts/install-rhdh-catalog-source.sh[877-890]

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

### Issue description
Auto-detection considers OLM v1 “present” if the ClusterExtension CRD exists, but the v1 execution path also depends on catalogd being installed/running and discoverable. This mismatch causes avoidable late failures.

### Issue Context
- `detect_olm_v1()` only checks for `clusterextensions.olm.operatorframework.io`.
- The v1 path later discovers/validates catalogd namespace and aborts if it’s missing.

### Fix
- Strengthen `detect_olm_v1()` (or the `auto` branch in `resolve_olm_version`) to validate all prerequisites used by the v1 path, e.g.:
 - ClusterExtension CRD exists, **and**
 - a catalogd deployment is discoverable (label query returns at least one item), **and**
 - the discovered/default catalogd namespace exists.
- If prerequisites are not met:
 - for `auto`: set `RESOLVED_OLM_VERSION=v0` and log a warning about incomplete v1 installation;
 - for forced `v1`: keep the current hard error.

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[118-153]
- .rhdh/scripts/install-rhdh-catalog-source.sh[877-890]

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


2. Unguarded olm-version value ✓ Resolved 🐞 Bug ≡ Correctness
Description
The new --olm-version option dereferences $2 under set -u without verifying an argument
exists, so ... --olm-version (missing value) will terminate with an “unbound variable” shell error
instead of showing a controlled validation/usage message. This makes the script brittle for
automation that conditionally appends flags.
Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R747-755]

+    '--olm-version')
+      if [[ "$2" != "v0" && "$2" != "v1" && "$2" != "auto" ]]; then
+        errorf "Unknown OLM version: $2. Must be v0, v1, or auto."
+        usage
+        exit 1
+      fi
+      OLM_VERSION="$2"
+      shift 1
+      ;;
Relevance

⭐⭐⭐ High

Repo frequently accepts hardening CLI arg validation under set -euo (avoid brittle runtime shell
errors).

PR-#2870
PR-#1037

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script enables set -euo pipefail globally, and the new --olm-version parsing branch reads
$2 unconditionally, which triggers an unbound-variable error when the flag is provided without a
value.

.rhdh/scripts/install-rhdh-catalog-source.sh[7-8]
.rhdh/scripts/install-rhdh-catalog-source.sh[719-767]

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

### Issue description
`--olm-version` reads `$2` without checking that it exists. With `set -euo pipefail`, missing `$2` crashes the script with an unbound-variable error.

### Issue Context
This impacts CLI robustness and is newly introduced by adding the `--olm-version` flag.

### Fix
- In the `--olm-version` case arm, add an argc check before accessing `$2` (e.g., `if [[ $# -lt 2 ]]; then ... usage; exit 1; fi`).
- (Optional but consistent) apply the same pattern to other flags that require a value (`--install-operator`, `--catalog-source`, `--install-plan-approval`, `-v`).

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[719-767]

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


Results up to commit c860d35


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Hardcoded ClusterCatalog pullSecret ✓ Resolved 🐞 Bug ≡ Correctness
Description
The OLM v1 ClusterCatalog manifest hardcodes pullSecret: internal-reg-auth-for-rhdh, but the
script only creates that secret in the OCP_INTERNAL registry flow. With `--to-registry
<external-registry>` (or any non-OCP_INTERNAL mirror), the script can apply a ClusterCatalog that
references a secret that was never created, preventing catalogd from pulling the catalog image.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R1240-1250]

+apiVersion: catalogd.operatorframework.io/v1
+kind: ClusterCatalog
+metadata:
+  name: rhdh-catalog
+spec:
+  source:
+    type: Image
+    image:
+      ref: ${my_operator_index}
+      pullSecret: internal-reg-auth-for-rhdh
+EOF
Relevance

⭐⭐⭐ High

Team often accepts correctness fixes in restricted-env scripts; similar flow-breaking issues were
fixed in PRs 2423/1646.

PR-#2423
PR-#1646
PR-#963

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script always writes ClusterCatalog with pullSecret: internal-reg-auth-for-rhdh, but only
creates that secret when using the OCP internal registry (via ocp_prepare_internal_registry /
prepare_olm_v1_secrets, which are invoked only for TO_REGISTRY=OCP_INTERNAL). Therefore, for
non-OCP_INTERNAL --to-registry flows, the ClusterCatalog can reference a missing secret.

.rhdh/scripts/prepare-restricted-environment.sh[1238-1250]
.rhdh/scripts/prepare-restricted-environment.sh[964-980]
.rhdh/scripts/prepare-restricted-environment.sh[382-419]
.rhdh/scripts/prepare-restricted-environment.sh[494-534]

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

### Issue description
The OLM v1 `ClusterCatalog` YAML is generated (and also created during oc-mirror conversion) with a hardcoded `pullSecret: internal-reg-auth-for-rhdh`. That secret is only created when `TO_REGISTRY=OCP_INTERNAL`, so for `--to-registry <external>` the applied `ClusterCatalog` can reference a non-existent secret and fail to pull.

### Issue Context
- In OLM v0, the CatalogSource includes multiple secrets including a user-provided `reg-pull-secret` option; the v1 path currently doesn’t provide an equivalent.
- For OLM v1, the pull secret likely needs to exist in the namespace where catalogd expects it (the script already special-cases this for OCP internal by creating secrets in the detected catalogd namespace).

### Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[1238-1250]
- .rhdh/scripts/prepare-restricted-environment.sh[1136-1151]
- .rhdh/scripts/prepare-restricted-environment.sh[964-980]
- .rhdh/scripts/prepare-restricted-environment.sh[382-419]

### Suggested implementation direction
- Introduce a variable (and optionally a CLI flag) for the v1 catalog pull secret name, e.g. `CATALOG_PULL_SECRET`.
- Default behavior:
 - If `TO_REGISTRY=OCP_INTERNAL`, keep using/creating `internal-reg-auth-for-rhdh` (current behavior).
 - Otherwise, either:
   - omit `pullSecret` entirely (if you want to support public registries by default), or
   - default it to `reg-pull-secret` and document that the user must create it in the catalogd namespace.
- Ensure any auto-created secret is created in the correct namespace for OLM v1 (catalogd namespace), not just the operator namespace.

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


2. cluster-admin ClusterRoleBinding created ✓ Resolved 📘 Rule violation ⛨ Security
Description
The script generates OLM v1 install manifests that create a ClusterRoleBinding binding the
installer namespaced ServiceAccount to the built-in cluster-admin ClusterRole, granting full
cluster-wide privileges. This RBAC is overly broad for managed resources and substantially increases
the blast radius and risk of privilege escalation if the ServiceAccount token is misused, especially
compared to the OLM v0 flow where no such cluster-admin binding is created.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R1402-1415]

+  cat <<EOF >"${manifestsTargetDir}/clusterRoleBinding.yaml"
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: ${CRB_NAME}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: cluster-admin
+subjects:
+- kind: ServiceAccount
+  name: ${SA_NAME}
+  namespace: ${NAMESPACE_OPERATOR}
+EOF
Relevance

⭐⭐ Medium

No direct precedent on rejecting cluster-admin bindings; “least-privilege” requests sometimes
rejected (e.g., PR 2141).

PR-#2141
PR-#1187

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 8 requires RBAC to be aligned to required operations and not overly broad; the
generated OLM v1 manifest violates this by explicitly setting roleRef.name: cluster-admin in the
ClusterRoleBinding for the installer ServiceAccount. Because this creates a persistent,
cluster-wide privilege grant to that ServiceAccount, any actor/workload able to use its token would
inherit full cluster-admin permissions beyond the minimal verbs/resources needed.

Rule 8: Ensure operator RBAC includes required verbs for managed Kubernetes resource kinds
.rhdh/scripts/prepare-restricted-environment.sh[1402-1415]

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

## Issue description
The generated OLM v1 installer RBAC creates a `ClusterRoleBinding` (via `clusterRoleBinding.yaml`) that binds the installer namespaced ServiceAccount to the built-in `cluster-admin` ClusterRole, granting permanent, full cluster-wide privileges; this is overly privileged, increases blast radius, and enables privilege escalation if the ServiceAccount token is used outside its intended purpose.

## Issue Context
Per compliance requirements, shipped RBAC should be scoped to the minimal apiGroups/resources/verbs needed by the installer/ClusterExtension installation workflow rather than using broad `cluster-admin` permissions. This binding is created whenever OLM v1 manifests are generated/applied, and even if the script is typically run by cluster admins, the resulting RBAC object persists and can be abused later by any principal that can run pods as that ServiceAccount in the operator namespace; additionally, this is a regression in security posture compared to the OLM v0 flow, where no cluster-admin binding is created.

## Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[1402-1415]

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


Results up to commit 50bb1df


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. --olm-version says catalogd running ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The --olm-version help text claims auto-detection checks for a running catalogd deployment, but
the implementation effectively treats the existence of a namespace (including defaulting to
openshift-catalogd) as sufficient, even when catalogd may not be installed or ready. This
mismatch can mislead users and can cause auto mode to incorrectly select OLM v1 and generate/apply
v1 resources on clusters where catalogd isn’t actually running, breaking the installation flow.
Code

.rhdh/scripts/prepare-restricted-environment.sh[R119-120]

+                                            'auto' detects OLM v1 by checking for the ClusterExtension CRD and
+                                            a running catalogd deployment on the cluster.
Relevance

⭐⭐⭐ High

Team has accepted multiple fixes aligning usage/help text with real behavior in this script (e.g.,
PR #1646, #1480).

PR-#1646
PR-#1480

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The usage/help text explicitly promises that auto-detection checks for a “running catalogd
deployment,” but the current detect_olm_v1_catalogd logic returns success based on a `get
namespace check rather than confirming a catalogd` Deployment exists and is Available; it derives
a namespace from a matching deployment if present, otherwise falls back to openshift-catalogd and
still succeeds if that namespace exists. Because resolve_olm_version uses this success signal in
auto mode, a namespace that exists without a running/ready catalogd can incorrectly drive
selection of v1 and lead to v1 resources being generated/applied.

.rhdh/scripts/prepare-restricted-environment.sh[118-120]
.rhdh/scripts/prepare-restricted-environment.sh[356-364]
.rhdh/scripts/prepare-restricted-environment.sh[392-400]
Best Practice: Repository guidelines

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

## Issue description
The `--olm-version` usage text states that auto-detection checks for a running `catalogd` deployment, but `detect_olm_v1_catalogd()` currently succeeds based only on namespace existence (including defaulting to `openshift-catalogd`). This creates a documentation/behavior mismatch and can also cause `resolve_olm_version` to choose `v1` in `auto` mode even when no `catalogd` Deployment is present/ready, breaking the installation flow.

## Issue Context
- The usage text promises auto-detection checks for “a running catalogd deployment”.
- `detect_olm_v1_catalogd` derives a namespace from the first matching deployment (if any) and falls back to `openshift-catalogd`, then returns success if the namespace exists.
- This success result is used by `resolve_olm_version` as a deciding signal to select OLM v1.

## Fix Focus Areas
- .rhdh/scripts/prepare-restricted-environment.sh[118-122]
- .rhdh/scripts/prepare-restricted-environment.sh[356-365]

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


2. v1 instructions miss ClusterCatalog ✓ Resolved 🐞 Bug ≡ Correctness
Description
When RESOLVED_OLM_VERSION==v1 and INSTALL_OPERATOR is false, the script prints manual apply steps
that omit clusterCatalog.yaml even though it is generated for the v1 path. Users following the
printed commands can end up applying a ClusterExtension that references a catalog that was never
created, leaving the extension unresolved.
Code

.rhdh/scripts/prepare-restricted-environment.sh[1614]

+      kubectl apply -f ${manifestsTargetDir}/clusterRole.yaml
Relevance

⭐⭐⭐ High

They frequently accept correctness fixes preventing broken install flows in this script (e.g., PR
#2423, #1646).

PR-#2423
PR-#1646

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script generates a ClusterCatalog manifest for OLM v1, but the user-facing v1 install
instructions only list namespace/serviceAccount/clusterRole/clusterRoleBinding/clusterExtension and
omit ClusterCatalog.

.rhdh/scripts/prepare-restricted-environment.sh[1276-1288]
.rhdh/scripts/prepare-restricted-environment.sh[1609-1617]

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

### Issue description
The OLM v1 manual install instructions printed when `INSTALL_OPERATOR!=true` do not include `clusterCatalog.yaml`, even though the script generates it for the v1 flow. Following the instructions as-is can produce an unresolved `ClusterExtension` due to the missing catalog.

### Issue Context
`clusterCatalog.yaml` is generated under the v1 manifest generation branch, but the printed list of `kubectl apply -f ...` commands omits it.

### Fix Focus Areas
- Update the printed OLM v1 instructions to include applying `clusterCatalog.yaml` (ideally before `clusterExtension.yaml`).
- (Optional but recommended) Also include `clusterCatalog` in the v1 manifest apply loop for consistency/idempotency.

### Fix Focus Areas (code references)
- .rhdh/scripts/prepare-restricted-environment.sh[1609-1617]
- .rhdh/scripts/prepare-restricted-environment.sh[1635-1638]

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


Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 23, 2026
Comment thread .rhdh/scripts/prepare-restricted-environment.sh Fixed
Add OLM version detection and branching to the airgap script so it
generates the correct manifests for both OLM v0 and OLM v1 clusters.

When OLM v1 is detected (or forced via --olm-version), the script
produces ClusterCatalog, ClusterExtension, ServiceAccount, and
ClusterRoleBinding instead of CatalogSource, OperatorGroup, and
Subscription. The oc-mirror path converts CatalogSource output to
ClusterCatalog. Export-only mode generates both manifest sets.

Signed-off-by: Fortune-Ndlovu <fndlovu@redhat.com>
Assisted-by: Claude (Opus 4.6)
@Fortune-Ndlovu Fortune-Ndlovu force-pushed the RHIDP-14785-operator-add-olm-v-1-path-to-prepare-restricted-environment-sh branch from 0137c0e to c860d35 Compare June 23, 2026 03:27
@Fortune-Ndlovu Fortune-Ndlovu changed the title RHIDP-14790: Add OLM v1 code path to install-rhdh-catalog-source.sh RHIDP-14785: Add OLM v1 code path to prepare-restricted-environment.sh Jun 23, 2026
@Fortune-Ndlovu

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread .rhdh/scripts/prepare-restricted-environment.sh
Comment thread .rhdh/scripts/prepare-restricted-environment.sh Outdated
@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit c860d35

Fortune-Ndlovu and others added 4 commits June 25, 2026 03:12
…m-v-1-path-to-prepare-restricted-environment-sh
…nt.sh

- Introduced functions to determine whether to generate v1 or v0 manifests based on the resolved OLM version and registry settings.
- Simplified conditional checks for OLM v1 and v0 manifest generation, improving code readability and maintainability.
- Updated error handling for OLM installation checks to reflect the new logic.
- Introduced a variable CATALOG_PULL_SECRET to dynamically set the pull secret based on the TO_REGISTRY value.
- Updated manifest generation logic to use the new CATALOG_PULL_SECRET variable, enhancing flexibility for different registry configurations.
- Improved code readability by consolidating pull secret assignments.
- Updated the buildRegistryUrl function to directly execute the oc command for retrieving the default route, improving clarity.
- Added a shellcheck directive to suppress warnings for the catalogImage variable assignment, enhancing code quality and maintainability.
@Fortune-Ndlovu

Copy link
Copy Markdown
Member Author

/agentic_review

@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5cb6ebd

…environment.sh

- Introduced a new ClusterRole for the rhdh-operator-installer, defining permissions for various Kubernetes resources.
- Updated the manifest generation logic to include the new ClusterRole and adjusted the ClusterRoleBinding to reference it.
- Enhanced the script to apply the ClusterRole during the installation process, improving operator functionality and security.
OLM v1 auto-detection is now platform-agnostic, based solely on
CRD presence and catalogd deployment status regardless of cluster type.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
Explain when each function returns true: either the resolved OLM
version matches, or we are exporting to disk with auto-detect mode
and need to generate both sets of manifests.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
…type

Replace hardcoded kubectl references in the OLM v1 manual install
instructions with the appropriate CLI tool for the detected cluster.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
oc-mirror v2 already generates ClusterCatalog resources natively
with the cc-*.yaml naming pattern. Use those directly instead of
manually converting CatalogSource manifests, applying our name
override, image URL rewrite, and pullSecret on top. Fall back to
the CatalogSource conversion only if no cc-*.yaml files are found.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
Comment thread .rhdh/scripts/prepare-restricted-environment.sh Fixed
@Fortune-Ndlovu

Copy link
Copy Markdown
Member Author

/agentic_review

'local' is only valid inside functions. These variables are in
the top-level script body, so drop the local qualifier.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

Grey Divider

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again

Grey Divider

Qodo Logo

@Fortune-Ndlovu

Copy link
Copy Markdown
Member Author

/agentic_review

@Fortune-Ndlovu Fortune-Ndlovu requested a review from rm3l July 6, 2026 09:48
@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

Grey Divider

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again

Grey Divider

Qodo Logo

This update grants the image-puller role to both the default and catalogd service accounts in the catalogd namespace, allowing them to pull images from the internal registry. This change enhances the permissions necessary for OLM v1 deployments.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@Fortune-Ndlovu

Copy link
Copy Markdown
Member Author

/agentic_review

…etes

When RESOLVED_OLM_VERSION is v1 on a non-OpenShift cluster, the
CatalogSource CRD check is irrelevant and would incorrectly fail.
Guard it behind a v1 check so --olm-version v1 works on Kubernetes
clusters that have OLM v1 installed.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
The script already sets -euo pipefail at the top level (line 8).
Functions run in the same shell, so repeating it inside
resolve_olm_version and prepare_olm_v1_secrets is a no-op.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
detect_olm_v1_catalogd and prepare_olm_v1_secrets both queried the
cluster for the catalogd deployment namespace. Move the result into
the global NAMESPACE_CATALOGD variable during detection so
prepare_olm_v1_secrets can reuse it without a redundant API call.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 3ef9065

CATALOG_PULL_SECRET is already set in the main flow before the
oc-mirror/non-oc-mirror split (when TO_REGISTRY is set). The
identical assignment inside the non-oc-mirror path was redundant.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
Reference RHIDP-8656 in the generated ClusterExtension manifest so
users inspecting the YAML understand why enforcement is set to None.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
RHIDP-8656 was the spike investigation, not a blocker. The actual
reason for disabling CRD upgrade safety preflight is upstream OLM v1
bug OCPBUGS-60693, which is fixed in OCP 4.22.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
The upstream OLM v1 bug (OCPBUGS-60693) that required disabling CRD
upgrade safety validation is fixed in OCP 4.22. Remove the workaround
so the default Strict enforcement applies.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
The template-mode code at line 1270 overwrites CATALOG_PULL_SECRET
with a literal '$CATALOG_PULL_SECRET' placeholder for export-to-dir.
When TO_REGISTRY is set, this must be re-assigned to the actual
secret name so the generated ClusterCatalog manifest has a valid
pullSecret reference. Without this, catalogd gets 'authentication
required' errors pulling from the internal registry.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
The olm.operatorframework.io/v1 ClusterCatalog CRD does not have a
pullSecret field — it was silently dropped, causing 'authentication
required' errors when catalogd tried to pull from the internal
registry.

OLM v1 catalogd and operator-controller both authenticate via the
global pull secret (openshift-config/pull-secret). Merge internal
registry credentials there instead of relying on a per-resource
pullSecret field that does not exist in the CRD schema.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
Comment thread .rhdh/scripts/prepare-restricted-environment.sh Fixed
Comment thread .rhdh/scripts/prepare-restricted-environment.sh Fixed
…-element loop

Remove all CATALOG_PULL_SECRET assignments since the variable is no longer
used after switching OLM v1 to global pull secret authentication. Also
replace single-element for loop with direct variable usage (SC2066).

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@Fortune-Ndlovu

Copy link
Copy Markdown
Member Author

/agentic_review

@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit eaeaec4

The image-puller role bindings were being created in the catalogd SA's
own namespace instead of the namespace where mirrored images are stored
(e.g. rhdh). This caused authentication failures when catalogd and
operator-controller tried to pull catalog and bundle images from the
internal registry.

Also add image-puller grants for the operator-controller service account,
which needs to pull bundle images during ClusterExtension installation.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

@Fortune-Ndlovu

Fortune-Ndlovu commented Jul 8, 2026

Copy link
Copy Markdown
Member Author
image
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"olm.operatorframework.io/v1","kind":"ClusterExtension","metadata":{"annotations":{},"name":"rhdh-operator"},"spec":{"namespace":"rhdh-operator","serviceAccount":{"name":"rhdh-operator-installer"},"source":{"catalog":{"channels":["fast"],"packageName":"rhdh","selector":{"matchLabels":{"olm.operatorframework.io/metadata.name":"rhdh-catalog"}}},"sourceType":"Catalog"}}}
  creationTimestamp: '2026-07-08T01:03:03Z'
  finalizers:
    - olm.operatorframework.io/cleanup-unpack-cache
    - olm.operatorframework.io/cleanup-contentmanager-cache
  generation: 1
  managedFields:
    - apiVersion: olm.operatorframework.io/v1
      fieldsType: FieldsV1
      fieldsV1:
        'f:spec':
          .: {}
          'f:namespace': {}
          'f:serviceAccount':
            .: {}
            'f:name': {}
          'f:source':
            .: {}
            'f:catalog':
              .: {}
              'f:channels': {}
              'f:packageName': {}
              'f:selector': {}
              'f:upgradeConstraintPolicy': {}
            'f:sourceType': {}
      manager: kubectl-client-side-apply
      operation: Update
      time: '2026-07-08T01:03:03Z'
    - apiVersion: olm.operatorframework.io/v1
      fieldsType: FieldsV1
      fieldsV1:
        'f:metadata':
          'f:finalizers':
            .: {}
            'v:"olm.operatorframework.io/cleanup-contentmanager-cache"': {}
            'v:"olm.operatorframework.io/cleanup-unpack-cache"': {}
      manager: operator-controller
      operation: Update
      time: '2026-07-08T01:03:03Z'
    - apiVersion: olm.operatorframework.io/v1
      fieldsType: FieldsV1
      fieldsV1:
        'f:status':
          .: {}
          'f:activeRevisions':
            .: {}
            'k:{"name":"rhdh-operator-1"}':
              .: {}
              'f:name': {}
          'f:conditions':
            .: {}
            'k:{"type":"Available"}':
              .: {}
              'f:lastTransitionTime': {}
              'f:message': {}
              'f:observedGeneration': {}
              'f:reason': {}
              'f:status': {}
              'f:type': {}
            'k:{"type":"BundleDeprecated"}':
              .: {}
              'f:lastTransitionTime': {}
              'f:message': {}
              'f:observedGeneration': {}
              'f:reason': {}
              'f:status': {}
              'f:type': {}
            'k:{"type":"ChannelDeprecated"}':
              .: {}
              'f:lastTransitionTime': {}
              'f:message': {}
              'f:observedGeneration': {}
              'f:reason': {}
              'f:status': {}
              'f:type': {}
            'k:{"type":"Deprecated"}':
              .: {}
              'f:lastTransitionTime': {}
              'f:message': {}
              'f:observedGeneration': {}
              'f:reason': {}
              'f:status': {}
              'f:type': {}
            'k:{"type":"Installed"}':
              .: {}
              'f:lastTransitionTime': {}
              'f:message': {}
              'f:observedGeneration': {}
              'f:reason': {}
              'f:status': {}
              'f:type': {}
            'k:{"type":"PackageDeprecated"}':
              .: {}
              'f:lastTransitionTime': {}
              'f:message': {}
              'f:observedGeneration': {}
              'f:reason': {}
              'f:status': {}
              'f:type': {}
            'k:{"type":"Progressing"}':
              .: {}
              'f:lastTransitionTime': {}
              'f:message': {}
              'f:observedGeneration': {}
              'f:reason': {}
              'f:status': {}
              'f:type': {}
          'f:install':
            .: {}
            'f:bundle':
              .: {}
              'f:name': {}
              'f:version': {}
      manager: operator-controller
      operation: Update
      subresource: status
      time: '2026-07-08T01:03:22Z'
  name: rhdh-operator
  resourceVersion: '82370'
  uid: 5def2ed0-aadf-4d99-82e9-9b4aab9e1c66
spec:
  namespace: rhdh-operator
  serviceAccount:
    name: rhdh-operator-installer
  source:
    catalog:
      channels:
        - fast
      packageName: rhdh
      selector:
        matchLabels:
          olm.operatorframework.io/metadata.name: rhdh-catalog
      upgradeConstraintPolicy: CatalogProvided
    sourceType: Catalog
status:
  activeRevisions:
    - name: rhdh-operator-1
  conditions:
    - lastTransitionTime: '2026-07-08T01:03:21Z'
      message: not deprecated
      observedGeneration: 1
      reason: NotDeprecated
      status: 'False'
      type: Deprecated
    - lastTransitionTime: '2026-07-08T01:03:21Z'
      message: package not deprecated
      observedGeneration: 1
      reason: NotDeprecated
      status: 'False'
      type: PackageDeprecated
    - lastTransitionTime: '2026-07-08T01:03:21Z'
      message: channel not deprecated
      observedGeneration: 1
      reason: NotDeprecated
      status: 'False'
      type: ChannelDeprecated
    - lastTransitionTime: '2026-07-08T01:03:21Z'
      message: bundle not deprecated
      observedGeneration: 1
      reason: NotDeprecated
      status: 'False'
      type: BundleDeprecated
    - lastTransitionTime: '2026-07-08T01:03:04Z'
      message: Revision 1.10.1 has rolled out.
      observedGeneration: 1
      reason: Succeeded
      status: 'True'
      type: Progressing
    - lastTransitionTime: '2026-07-08T01:03:22Z'
      message: 'Installed bundle image-registry.openshift-image-registry.svc:5000/rhdh/rhdh-operator-bundle:78a87e6ef97be4e534a5e824c57c032b41b6ece1880289542a08e5ff3a3e9185 successfully'
      observedGeneration: 1
      reason: Succeeded
      status: 'True'
      type: Installed
    - lastTransitionTime: '2026-07-08T01:03:21Z'
      message: Objects are available and pass all probes.
      observedGeneration: 1
      reason: ProbesSucceeded
      status: 'True'
      type: Available
  install:
    bundle:
      name: rhdh-operator.v1.10.1
      version: 1.10.1
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request Review effort 4/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants