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

[Fortune-Ndlovu]

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 \
  --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]

PR Summary by Qodo

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

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]

Code Review by Qodo

🐞 Bugs (0) 📘 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

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

---

5. catalogd timeout too short ✓ Resolved 🐞 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

---

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

---

Previous review results

Review updated until commit 4bcf836

Results up to commit cd08778

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

1. 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

---

2. 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

---

Results up to commit c860d35

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

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)

1. 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

---

2. --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

---

[Fortune-Ndlovu]

/agentic_review

[rhdh-qodo-merge]

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

[Fortune-Ndlovu]

/agentic_review

[rhdh-qodo-merge]

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

[Fortune-Ndlovu]

Tested again, script works as expected:

[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