fix: wait for OLM v1 install readiness before success

[zdrapela]

Description

Fixes the OLM v1 path in install-rhdh-catalog-source.sh so RHDH operator install does not exit 0 before the operator is actually ready (the failure mode in OCP 4.18 operator nightlies after #3047).

--olm-version auto still prefers v1 when the ClusterExtension CRD is present (OpenShift and Kubernetes).

Changes

• Align ClusterCatalog/ClusterExtension manifests with working samples (catalog selector label + crdUpgradeSafety: None)
• Wait for ClusterCatalog Serving, ClusterExtension Installed, and (for RHDH) CRD backstages.rhdh.redhat.com Established before printing Done; on timeout dump slim diagnostics and exit non-zero
• On OpenShift, merge internal (and optional external) registry credentials into openshift-config/pull-secret so catalogd can pull the rebuilt IIB (authentication required without this)
• Derive IMAGE_NAMESPACE from the rebuilt IIB image ref for image-puller grants (IIB project, not the app deploy ns); ensure that namespace exists before grants
• Resolve OLM controller SAs for image-puller grants and fail hard if grants fail on OpenShift
• Hardcoded 300s wait timeouts; external registry default-route is optional (warn + continue)
• Document OLM auto-detect and the Serving / Installed / Established readiness gates

Verification

Live path matrix on workshop OCP 4.20 (auto v1 install, explicit v1, v1 catalog-only, v0 install, v0 catalog-only) — all exited 0 with expected resources.

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

• Fixes RHDHBUGS-3510
• Relates to RHIDP-14790: Add OLM v1 code path to install-rhdh-catalog-source.sh #3047 (OLM v1 auto-detect); failure example: e2e-ocp-operator-nightly #2081968295161892864

PR acceptance criteria

• Tests
• Documentation

How to test changes / Special notes to the reviewer

On an OCP 4.18+ cluster where the ClusterExtension CRD exists:

bash .rhdh/scripts/install-rhdh-catalog-source.sh --next --install-operator rhdh
oc get crd backstages.rhdh.redhat.com
oc get clustercatalog rhdh-fast -o jsonpath='{.status.conditions[?(@.type=="Serving")].status}{"\n"}'
oc get clusterextension rhdh -o jsonpath='{.status.conditions[?(@.type=="Installed")].status}{"\n"}'

# Explicit v0 still works
bash .rhdh/scripts/install-rhdh-catalog-source.sh --next --install-operator rhdh --olm-version v0

Expect either a real install (CRD Established + operator pod Running) or a non-zero exit with OLM v1 diagnostics — never silent success.

Building Container Images for Testing

N/A (script/docs only).

Made with Cursor

[zdrapela]

Follow-up: catalogd auth fix

CI failure root cause was ClusterCatalog stuck on:
authentication required for image-registry.openshift-image-registry.svc:5000/rhdh/iib:...

Catalogd uses --global-pull-secret=openshift-config/pull-secret; image-puller alone is not enough. Aligned with prepare-restricted-environment.sh prepare_olm_v1_secrets():

• merge internal/external registry creds into the cluster pull secret
• create internal-reg-*-auth-for-rhdh in rhdh-operator
• keep SA image-puller grants

Verified on workshop OCP 4.20

bash .rhdh/scripts/install-rhdh-catalog-source.sh --next --install-operator rhdh → exit 0

• ClusterCatalog rhdh-fast Serving=True
• ClusterExtension rhdh Installed=True
• CRD backstages.rhdh.redhat.com present
• operator pod Running in rhdh-operator

[zdrapela]

/agentic_review

[rhdh-qodo-merge]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 18 rules

✅ Cross-repo context

  Explored: repo: redhat-developer/rhdh (sha: 6bcb0141)

  Not relevant to this PR: redhat-developer/rhdh-plugins

1. Hardcoded image namespace fails ✓ Resolved 🔗 Cross-repo conflict ≡ Correctness

Description

The OLM v1 path now fails the install if it cannot grant system:image-puller in a hardcoded rhdh
namespace, but redhat-developer/rhdh CI defaults to OPERATOR_MANAGER=rhdh-operator and deploy
namespaces like showcase, not rhdh. This can cause operator installation to exit non-zero on
OpenShift clusters where the rhdh namespace doesn’t exist (even if the operator/catalog images are
not in that namespace).

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R1119-1122]

+  if ! prepare_olm_v1_secrets "${NAMESPACE_CATALOGD}" "${NAMESPACE_OLM_CONTROLLER}" "rhdh"; then
+    dump_olm_v1_diagnostics "${CATALOGSOURCE_NAME}" "" "${NAMESPACE_CATALOGD}" "${NAMESPACE_OLM_CONTROLLER}"
+    exit 1
  fi

Relevance

⭐⭐⭐ High

Hardcoded namespace likely seen as CI-breaking correctness issue; team often accepts
hardening/portability fixes in scripts.

PR-#3047

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR script calls prepare_olm_v1_secrets(..., "rhdh"), and that function (plus a separate block)
grants system:image-puller in the hardcoded rhdh namespace and now treats failures as fatal. In
the rhdh repo, CI defaults to NAME_SPACE=showcase and OPERATOR_MANAGER=rhdh-operator, and its
operator job flow calls prepare_operator, which downloads and executes this script—meaning the
hardcoded rhdh namespace assumption can break CI/e2e runs.

.rhdh/scripts/install-rhdh-catalog-source.sh[191-261]
.rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]
.rhdh/scripts/install-rhdh-catalog-source.sh[1187-1193]
External repo: redhat-developer/rhdh, .ci/pipelines/env_variables.sh [78-87]
External repo: redhat-developer/rhdh, .ci/pipelines/jobs/ocp-operator.sh [87-111]
External repo: redhat-developer/rhdh, .ci/pipelines/install-methods/operator.sh [24-43]

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 installer’s OLM v1 flow hardcodes the image-puller grant target namespace to `rhdh` (both for catalogd/operator-controller SAs and the installer SA). In RHDH CI (redhat-developer/rhdh), the operator is installed in `rhdh-operator` and deployments default to `showcase` namespaces, so `rhdh` may not exist and the script now exits 1.

### Issue Context
RHDH CI downloads and runs this script as part of the operator install. The script should not assume the existence of a `rhdh` namespace; it should either (a) detect the correct image namespace, (b) default to a safe existing namespace (e.g., `${NAMESPACE_SUBSCRIPTION}`), or (c) make the target namespace configurable and only enforce the RBAC when actually needed.

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[191-262]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1187-1193]

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

---

2. CRD wait too weak ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

The OLM v1 RHDH readiness gate only waits until the CRD object exists, so the script can still exit
0 before the CRD is actually Established/served (or immediately pass if the CRD pre-exists from a
prior run). This undermines the stated goal of not reporting success until the operator API is ready
to use.

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R1216-1217]

+    infof "Waiting for CRD backstages.rhdh.redhat.com (timeout 300s)..."
+    deadline=$((SECONDS + 300))

Relevance

⭐⭐⭐ High

Strengthening readiness checks matches PR intent; similar install scripts changes for correctness
are usually accepted.

PR-#3047
PR-#3046

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code currently polls only for CRD existence and does not check any CRD status conditions
(Established/served), so it can declare readiness too early or instantly succeed if the CRD already
exists.

.rhdh/scripts/install-rhdh-catalog-source.sh[1214-1228]

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 script’s OLM v1 RHDH readiness check treats `kubectl/oc get crd backstages.rhdh.redhat.com` success as readiness. Existence is not the same as being Established/served, and it also trivially passes when the CRD already exists.

## Issue Context
This check is intended to prevent silent success before the operator is actually ready.

## Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[1216-1226]

## Suggested fix
- Replace the polling `get crd ...` loop with an explicit wait on CRD conditions, e.g.:
 - `invoke_cluster_cli wait --for=condition=Established crd/backstages.rhdh.redhat.com --timeout=300s`
- Optionally also verify served/accepted status via jsonpath on `.status.conditions` (e.g., `NamesAccepted=True`) if you’ve seen clusters where `Established` is delayed.
- Keep the existing diagnostics + non-zero exit on timeout.

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

---

3. Pull-secret mutation breaks CI 🔗 Cross-repo conflict ☼ Reliability ⭐ New

Description

The OLM v1 OpenShift path now constructs and applies a modified openshift-config/pull-secret (via
jq) and will fail the install if that update can’t be performed. redhat-developer/rhdh CI
downloads and executes this script to install the operator from quay.io, so introducing a
cluster-scoped pull-secret write requirement can break CI installs when OLM v1 is auto-detected and
the CI identity lacks that permission.

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R203-213]

+    --arg url "${internal_registry_url}" \
+    --arg auth "${internal_auth}" \
+    '.auths[$url] = {auth: $auth}')
+  if [[ -n "${external_registry_url}" ]]; then
+    merged=$(echo "${merged}" | jq \
+      --arg url "${external_registry_url}" \
+      --arg auth "${internal_auth}" \
+      '.auths[$url] = {auth: $auth}')
+  else
+    warnf "OpenShift registry default-route not found; merged credentials for ${internal_registry_url} only"
+  fi

Relevance

⭐⭐⭐ High

CI/permission-regression risk; repo has history of accepting script changes that harden OLM v1
flows.

PR-#3046
PR-#3047

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds logic to merge internal registry credentials into the global pull secret using jq, and
later applies it to openshift-config/pull-secret, making this a cluster-scoped write requirement
on the OLM v1 OpenShift path. In the rhdh repo, CI explicitly downloads and executes this script to
install the operator and notes that the operator is installed from quay.io, so the new pull-secret
mutation requirement can cause cross-repo CI breakage if permissions are insufficient.

.rhdh/scripts/install-rhdh-catalog-source.sh[187-216]
.rhdh/scripts/install-rhdh-catalog-source.sh[1107-1109]
External repo: redhat-developer/rhdh, .ci/pipelines/install-methods/operator.sh [17-22]
External repo: redhat-developer/rhdh, .ci/pipelines/install-methods/operator.sh [24-41]

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 installer’s OLM v1 OpenShift flow now modifies the cluster-wide `openshift-config/pull-secret` and (via the caller) treats any failure in that process as a hard install failure. This script is consumed by `redhat-developer/rhdh` CI, which installs the operator from quay.io; requiring a cluster-scoped pull-secret update (even when quay.io is used) can cause CI failures if the CI token lacks permission to update `openshift-config/pull-secret`.

### Issue Context
`redhat-developer/rhdh` downloads and runs this script during CI operator installation. The script should avoid adding new cluster-admin-only side effects unless strictly necessary, or at least provide an opt-out / best-effort behavior for callers.

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[187-216]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1107-1109]

### Suggested remediation approaches
- Gate the global pull-secret mutation behind a condition (e.g., only when the catalog image ref is the internal registry / requires internal-registry auth).
- Alternatively, make the pull-secret mutation best-effort (warn + continue) when the catalog image is not internal.
- Add an explicit flag or env var to disable global pull-secret mutation for CI callers that don’t need it (like quay.io-based installs).

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

---

4. Unvalidated v1 wait timeout ✓ Resolved 🐞 Bug ☼ Reliability

Description

OLM_V1_WAIT_TIMEOUT is read from the environment but never validated as a positive integer before
being used in wait --timeout=... and in arithmetic (deadline=$((...))), so invalid values can
cause bash arithmetic errors (terminating the script under set -euo pipefail) or malformed CLI
timeout arguments.

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R19-20]

+# Timeout (seconds) for OLM v1 readiness waits (ClusterCatalog Serving, ClusterExtension Installed, CRD).
+OLM_V1_WAIT_TIMEOUT="${OLM_V1_WAIT_TIMEOUT:-300}"

Relevance

⭐⭐⭐ High

Team previously accepted validating numeric env vars under set -euo pipefail; same pattern as
MAX_PARALLEL.

PR-#2870
PR-#3047

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR introduces OLM_V1_WAIT_TIMEOUT but does not validate it, while the script does validate a
similar numeric env var (MAX_PARALLEL). The timeout is later used both in CLI timeouts and in
arithmetic for the CRD polling deadline, which requires a numeric value.

.rhdh/scripts/install-rhdh-catalog-source.sh[13-25]
.rhdh/scripts/install-rhdh-catalog-source.sh[1140-1142]
.rhdh/scripts/install-rhdh-catalog-source.sh[1228-1232]

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_V1_WAIT_TIMEOUT` is used as both a duration string (`--timeout="${OLM_V1_WAIT_TIMEOUT}s"`) and as a number in arithmetic (`deadline=$((SECONDS + OLM_V1_WAIT_TIMEOUT))`), but it is not validated. If it is set to a non-integer (or 0/negative), the script can fail immediately or behave unexpectedly.

## Issue Context
The script already validates `MAX_PARALLEL` as a positive integer but does not apply similar validation to `OLM_V1_WAIT_TIMEOUT`.

## Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[19-25]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1140-1142]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1230-1232]

## Suggested fix
Add a validation block right after `OLM_V1_WAIT_TIMEOUT` is initialized, similar to `MAX_PARALLEL`, e.g.:

```bash
if ! [[ "${OLM_V1_WAIT_TIMEOUT}" =~ ^[0-9]+$ ]] || [[ "${OLM_V1_WAIT_TIMEOUT}" -lt 1 ]]; then
 echo "[ERROR] OLM_V1_WAIT_TIMEOUT must be a positive integer, got: '${OLM_V1_WAIT_TIMEOUT}'" >&2
 exit 1
fi
```

This prevents arithmetic evaluation errors and ensures wait timeouts are well-formed.

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

---

View more (1)

5. Registry default-route required ✓ Resolved 🔗 Cross-repo conflict ☼ Reliability

Description

The script’s OLM v1 OpenShift flow now requires openshift-image-registry/default-route to exist
and fails otherwise, even though redhat-developer/rhdh CI installs the operator from quay.io and
doesn’t provision this route as part of its operator job setup. This introduces a new cluster
prerequisite that can break RHDH CI/e2e on OpenShift/OSD environments where the registry route is
not exposed.

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R204-208]

+  external_registry_url=$(oc get route default-route -n openshift-image-registry --template='{{ .spec.host }}' 2>/dev/null || true)
+  if [[ -z "${external_registry_url}" ]]; then
+    errorf "Could not resolve OpenShift internal registry default route; is the registry exposed?"
+    return 1
+  fi

Relevance

⭐⭐ Medium

No close precedent; may be intentional OpenShift prereq, but could break CI if route absent.

PR-#1844

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds a hard dependency on querying openshift-image-registry/default-route and returns an
error if it’s absent. The rhdh repo’s operator CI flow downloads and runs this script during
prepare_operator without any preceding registry-route provisioning, so this new prerequisite can
cause cross-repo failures depending on cluster defaults.

.rhdh/scripts/install-rhdh-catalog-source.sh[202-208]
.rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]
External repo: redhat-developer/rhdh, .ci/pipelines/jobs/ocp-operator.sh [87-111]
External repo: redhat-developer/rhdh, .ci/pipelines/install-methods/operator.sh [24-43]

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()` fails if it can’t resolve the OpenShift internal registry `default-route`. This is an implicit new prerequisite for OLM v1 installs that can break consumers (like redhat-developer/rhdh CI) on clusters where the registry route isn’t exposed.

### Issue Context
RHDH CI’s operator install path downloads and runs this script and doesn’t show any setup to expose the internal registry route prior to running it. The script should avoid failing hard on missing `default-route` unless it is strictly required for the selected image source.

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[202-208]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]

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

---

6. Docs readiness claim imprecise ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New

Description

The docs now say the install script “waits for the operator to be ready”, but the implementation
specifically waits for ClusterCatalog Serving, ClusterExtension Installed, and (for RHDH) the
Backstage CRD to exist. This mismatch can mislead users about what “ready” means in the OLM v1 flow.

Code

.rhdh/docs/installing-ci-builds.adoc[36]

+. Run the link:../scripts/install-rhdh-catalog-source.sh[installation script] to create the RHDH Operator CatalogSource in your cluster. By default, it installs the Release Candidate or GA version (from the `release-1.yy` branch), but the `--next` option allows to install the current development build (from the `main` branch). The script auto-detects OLM v0 or v1 and waits for the operator to be ready before exiting. For example:

Relevance

⭐⭐⭐ High

Team often accepts doc precision/clarifications to avoid misleading guidance.

PR-#1844
PR-#2434

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The docs sentence is broad, while the script’s new waits are explicitly for ClusterCatalog Serving /
ClusterExtension Installed and a CRD existence check, which is a narrower definition than generic
operator readiness.

.rhdh/docs/installing-ci-builds.adoc[34-37]
.rhdh/scripts/install-rhdh-catalog-source.sh[1128-1133]
.rhdh/scripts/install-rhdh-catalog-source.sh[1207-1226]

Agent prompt

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

## Issue description
Documentation promises a broad readiness guarantee (“operator ready”), but the script implements a narrower, explicit set of waits/conditions.

## Issue Context
Keeping docs aligned to the exact readiness checks avoids user confusion (especially in CI).

## Fix Focus Areas
- .rhdh/docs/installing-ci-builds.adoc[36-36]

## Suggested fix
- Update the sentence to explicitly name the conditions the script waits for (e.g., “waits for ClusterCatalog Serving, ClusterExtension Installed, and for RHDH the Backstage CRD to appear/establish”).
- If you truly intend “operator runtime readiness”, adjust the script to wait for a stronger signal and then the current docs wording is fine.

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

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit 28c67cd

Results up to commit 5dbbad0 ⚖️ Balanced

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

1. Hardcoded image namespace fails 🔗 Cross-repo conflict ≡ Correctness

Description

The OLM v1 path now fails the install if it cannot grant system:image-puller in a hardcoded rhdh
namespace, but redhat-developer/rhdh CI defaults to OPERATOR_MANAGER=rhdh-operator and deploy
namespaces like showcase, not rhdh. This can cause operator installation to exit non-zero on
OpenShift clusters where the rhdh namespace doesn’t exist (even if the operator/catalog images are
not in that namespace).

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R1119-1122]

+  if ! prepare_olm_v1_secrets "${NAMESPACE_CATALOGD}" "${NAMESPACE_OLM_CONTROLLER}" "rhdh"; then
+    dump_olm_v1_diagnostics "${CATALOGSOURCE_NAME}" "" "${NAMESPACE_CATALOGD}" "${NAMESPACE_OLM_CONTROLLER}"
+    exit 1
  fi

Relevance

⭐⭐⭐ High

Hardcoded namespace likely seen as CI-breaking correctness issue; team often accepts
hardening/portability fixes in scripts.

PR-#3047

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR script calls prepare_olm_v1_secrets(..., "rhdh"), and that function (plus a separate block)
grants system:image-puller in the hardcoded rhdh namespace and now treats failures as fatal. In
the rhdh repo, CI defaults to NAME_SPACE=showcase and OPERATOR_MANAGER=rhdh-operator, and its
operator job flow calls prepare_operator, which downloads and executes this script—meaning the
hardcoded rhdh namespace assumption can break CI/e2e runs.

.rhdh/scripts/install-rhdh-catalog-source.sh[191-261]
.rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]
.rhdh/scripts/install-rhdh-catalog-source.sh[1187-1193]
External repo: redhat-developer/rhdh, .ci/pipelines/env_variables.sh [78-87]
External repo: redhat-developer/rhdh, .ci/pipelines/jobs/ocp-operator.sh [87-111]
External repo: redhat-developer/rhdh, .ci/pipelines/install-methods/operator.sh [24-43]

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 installer’s OLM v1 flow hardcodes the image-puller grant target namespace to `rhdh` (both for catalogd/operator-controller SAs and the installer SA). In RHDH CI (redhat-developer/rhdh), the operator is installed in `rhdh-operator` and deployments default to `showcase` namespaces, so `rhdh` may not exist and the script now exits 1.

### Issue Context
RHDH CI downloads and runs this script as part of the operator install. The script should not assume the existence of a `rhdh` namespace; it should either (a) detect the correct image namespace, (b) default to a safe existing namespace (e.g., `${NAMESPACE_SUBSCRIPTION}`), or (c) make the target namespace configurable and only enforce the RBAC when actually needed.

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[191-262]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1187-1193]

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

---

2. Unvalidated v1 wait timeout ✓ Resolved 🐞 Bug ☼ Reliability

Description

OLM_V1_WAIT_TIMEOUT is read from the environment but never validated as a positive integer before
being used in wait --timeout=... and in arithmetic (deadline=$((...))), so invalid values can
cause bash arithmetic errors (terminating the script under set -euo pipefail) or malformed CLI
timeout arguments.

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R19-20]

+# Timeout (seconds) for OLM v1 readiness waits (ClusterCatalog Serving, ClusterExtension Installed, CRD).
+OLM_V1_WAIT_TIMEOUT="${OLM_V1_WAIT_TIMEOUT:-300}"

Relevance

⭐⭐⭐ High

Team previously accepted validating numeric env vars under set -euo pipefail; same pattern as
MAX_PARALLEL.

PR-#2870
PR-#3047

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR introduces OLM_V1_WAIT_TIMEOUT but does not validate it, while the script does validate a
similar numeric env var (MAX_PARALLEL). The timeout is later used both in CLI timeouts and in
arithmetic for the CRD polling deadline, which requires a numeric value.

.rhdh/scripts/install-rhdh-catalog-source.sh[13-25]
.rhdh/scripts/install-rhdh-catalog-source.sh[1140-1142]
.rhdh/scripts/install-rhdh-catalog-source.sh[1228-1232]

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_V1_WAIT_TIMEOUT` is used as both a duration string (`--timeout="${OLM_V1_WAIT_TIMEOUT}s"`) and as a number in arithmetic (`deadline=$((SECONDS + OLM_V1_WAIT_TIMEOUT))`), but it is not validated. If it is set to a non-integer (or 0/negative), the script can fail immediately or behave unexpectedly.

## Issue Context
The script already validates `MAX_PARALLEL` as a positive integer but does not apply similar validation to `OLM_V1_WAIT_TIMEOUT`.

## Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[19-25]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1140-1142]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1230-1232]

## Suggested fix
Add a validation block right after `OLM_V1_WAIT_TIMEOUT` is initialized, similar to `MAX_PARALLEL`, e.g.:

```bash
if ! [[ "${OLM_V1_WAIT_TIMEOUT}" =~ ^[0-9]+$ ]] || [[ "${OLM_V1_WAIT_TIMEOUT}" -lt 1 ]]; then
 echo "[ERROR] OLM_V1_WAIT_TIMEOUT must be a positive integer, got: '${OLM_V1_WAIT_TIMEOUT}'" >&2
 exit 1
fi
```

This prevents arithmetic evaluation errors and ensures wait timeouts are well-formed.

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

---

3. Registry default-route required ✓ Resolved 🔗 Cross-repo conflict ☼ Reliability

Description

The script’s OLM v1 OpenShift flow now requires openshift-image-registry/default-route to exist
and fails otherwise, even though redhat-developer/rhdh CI installs the operator from quay.io and
doesn’t provision this route as part of its operator job setup. This introduces a new cluster
prerequisite that can break RHDH CI/e2e on OpenShift/OSD environments where the registry route is
not exposed.

Code

.rhdh/scripts/install-rhdh-catalog-source.sh[R204-208]

+  external_registry_url=$(oc get route default-route -n openshift-image-registry --template='{{ .spec.host }}' 2>/dev/null || true)
+  if [[ -z "${external_registry_url}" ]]; then
+    errorf "Could not resolve OpenShift internal registry default route; is the registry exposed?"
+    return 1
+  fi

Relevance

⭐⭐ Medium

No close precedent; may be intentional OpenShift prereq, but could break CI if route absent.

PR-#1844

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds a hard dependency on querying openshift-image-registry/default-route and returns an
error if it’s absent. The rhdh repo’s operator CI flow downloads and runs this script during
prepare_operator without any preceding registry-route provisioning, so this new prerequisite can
cause cross-repo failures depending on cluster defaults.

.rhdh/scripts/install-rhdh-catalog-source.sh[202-208]
.rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]
External repo: redhat-developer/rhdh, .ci/pipelines/jobs/ocp-operator.sh [87-111]
External repo: redhat-developer/rhdh, .ci/pipelines/install-methods/operator.sh [24-43]

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()` fails if it can’t resolve the OpenShift internal registry `default-route`. This is an implicit new prerequisite for OLM v1 installs that can break consumers (like redhat-developer/rhdh CI) on clusters where the registry route isn’t exposed.

### Issue Context
RHDH CI’s operator install path downloads and runs this script and doesn’t show any setup to expose the internal registry route prior to running it. The script should avoid failing hard on missing `default-route` unless it is strictly required for the selected image source.

### Fix Focus Areas
- .rhdh/scripts/install-rhdh-catalog-source.sh[202-208]
- .rhdh/scripts/install-rhdh-catalog-source.sh[1119-1122]

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

---

[zdrapela]

/agentic_review

[rhdh-qodo-merge]

Code review by qodo was updated up to the latest commit 7e5b22a

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
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

[rhdh-qodo-merge]

PR Summary by Qodo

Fix OLM v1 install script to wait for Serving/Installed/CRD readiness

🐞 Bug fix 📝 Documentation 🕐 20-40 Minutes

AI Description

• Prevent OLM v1 installs from exiting 0 before the operator is actually ready.
• Add OLM v1 readiness gates (Serving/Installed/CRD Established) with timeout diagnostics.
• Ensure OpenShift registry auth and image-puller grants are correctly applied for catalogd pulls.

Diagram

graph TD
  A["CI/User runs install script"] --> B{"OLM v1 detected?"}
  B -->|"No"| C["Run OLM v0 flow"] --> G["Done (exit 0)"]
  B -->|"Yes"| D["Prepare OpenShift auth + image-puller"] --> E["Apply ClusterCatalog/ClusterExtension"] --> F["Wait: Serving/Installed/Established"] --> G
  F --> H["On timeout: dump diagnostics + exit 1"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Make readiness timeouts configurable (flag/env)

• ➕ Allows slower clusters/CI runs to avoid false negatives
• ➕ Keeps the same readiness semantics while improving operability
• ➖ Adds surface area and support burden for another tuning knob
• ➖ Can mask real install issues if set excessively high

2. Wait on operator deployment/pod readiness instead of CRD Established

• ➕ More directly reflects the operator actually running
• ➕ Could detect cases where CRDs exist but controller never starts
• ➖ Harder to implement generically across different operators/images
• ➖ RHDH-specific wiring needed; CRD readiness is a stable, low-cost proxy

3. Avoid mutating openshift-config/pull-secret (document prerequisite only)

• ➕ Less invasive cluster-wide change
• ➕ Reduces risk of unintended pull-secret merge issues
• ➖ CI/users will continue to hit catalogd 'authentication required' failures
• ➖ Increases manual setup and decreases script reliability for OLM v1 installs

Recommendation: Keep the PR’s approach: enforcing explicit OLM v1 readiness gates plus global pull-secret merge is the most reliable way to prevent silent success and unblock catalogd pulls on OpenShift. If follow-up work is desired, the most valuable improvement would be an optional timeout override for exceptionally slow environments while keeping the 300s default.

Files changed (2) +153 / -14

Bug fix (1) +152 / -13

install-rhdh-catalog-source.shAdd OLM v1 readiness waits, diagnostics, and OpenShift registry auth handling +152/-13

Add OLM v1 readiness waits, diagnostics, and OpenShift registry auth handling

• Prevents OLM v1 installs from exiting successfully before resources are actually ready by waiting for ClusterCatalog Serving, ClusterExtension Installed, and (for RHDH) the Backstage CRD Established condition. Adds failure diagnostics dumping ClusterCatalog/ClusterExtension status on timeouts and hard-fails when critical OpenShift RBAC grants fail. On OpenShift, merges internal (and optional external route) registry credentials into the global pull-secret for catalogd authentication, derives the internal registry image namespace from the rebuilt IIB image ref, ensures the namespace exists, and grants image-puller to the resolved OLM controller service accounts.

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

Documentation (1) +1 / -1

installing-ci-builds.adocDocument OLM auto-detect and OLM v1 readiness gates +1/-1

Document OLM auto-detect and OLM v1 readiness gates

• Updates the install procedure to clarify that the script auto-detects OLM v0/v1. Documents that '--install-operator' now waits for ClusterCatalog Serving, ClusterExtension Installed, and Backstage CRD Established before exiting successfully.

.rhdh/docs/installing-ci-builds.adoc

[rhdh-qodo-merge]

Code Review by Qodo

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again manually by commenting /agentic_review on this PR.

[Fortune-Ndlovu]

lgtm