Skip to content

OCPBUGS-85579: Move Console resources from bundle to runtime with capability detection#455

Open
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:fix-consoleless-runtime
Open

OCPBUGS-85579: Move Console resources from bundle to runtime with capability detection#455
sebrandon1 wants to merge 1 commit into
openshift:masterfrom
sebrandon1:fix-consoleless-runtime

Conversation

@sebrandon1

@sebrandon1 sebrandon1 commented Jul 8, 2026

Copy link
Copy Markdown
Member

Problem

cert-manager-operator fails on consoleless clusters (RAN/RDS/SNO) when OLM tries to install Console resources (ConsoleYAMLSample, ConsoleQuickStart):

GroupVersionKind console.openshift.io/v1, Kind=ConsoleYAMLSample not found

Why PR #424's approach didn't work:
The capability.openshift.io/name: Console annotation is only processed by CVO for core platform components — not by OLM for third-party operator bundles.

Solution

Move Console resource creation from the OLM bundle to operator runtime, following the pattern from cluster-monitoring-operator #2011.

How it works:

  1. Operator checks ClusterVersion.status.capabilities.enabledCapabilities for Console capability
  2. Console enabled → creates resources at runtime
  3. Console disabled → skips silently

Changes

  • Removed Console manifests from bundle/manifests/ (OLM no longer creates them)
  • Added pkg/controller/certmanager/console_resources.go — runtime Console capability detection
  • Added Console YAML files to bindata/console/ for runtime embedding
  • Added RBAC for Console resources and ClusterVersion access

Behavior

Cluster Type Result
Consoleless (RAN/RDS/SNO) ✅ InstallPlan succeeds, Console resources not created
Console-enabled (standard) ✅ InstallPlan succeeds, Console resources created at runtime

Testing

Verified on OCP 4.22:

  • Console-enabled: InstallPlan Complete, no regression
  • Simulated consoleless: InstallPlan Complete, no GVK errors
  • Unit tests: 13 new tests covering capability detection and sync behavior

References

Other operators that solved this same problem

Runtime capability/API detection (the pattern this PR follows):

Operator PR Console Resource Approach
cluster-monitoring-operator #2011 ConsolePlugin ClusterVersion.status.capabilities check (OCPBUGS-14922)
cluster-authentication-operator #589 Operator restart Watches ClusterVersion for Console capability (OCPBUGS-3440)
oadp-operator #2170 ConsoleCLIDownload REST mapper probe / meta.IsNoMatchError (OADP-7868)
gitops-operator #1172 ConsolePlugin backend config.openshift.io discovery check (GITOPS-9943)
tektoncd/operator (Pipelines) #220 ConsoleYAMLSample, ConsoleQuickStart Runtime CRD existence check
hyperconverged-cluster-operator controller-runtime #2456 ConsoleQuickStart, ConsolePlugin Fixed upstream — lazy cache type validation
stolostron/hypershift-addon-operator #202 ConsoleCLIDownload Skip on consoleless (ACM-3863)
assisted-installer #574 Installer hang Skip waiting for console operator (MGMT-12471)

CVO annotation approach (works for CVO-managed components only, NOT for OLM operators):

Operator PR Console Resource Notes
console-operator #665, #992, #1082 All Console CRDs Self-annotation (CONSOLE-3220)
cluster-monitoring-operator #2254 Dashboard ConfigMaps Required renaming manifests to avoid CVO force-enable (OCPBUGS-18326)
cluster-kube-apiserver-operator #1542, #1565 Dashboard ConfigMaps Same rename workaround needed (OCPBUGS-18247, OCPBUGS-20331)
external-secrets-operator #148 ConsoleYAMLSample CVO annotation (same bug as cert-manager — ineffective for OLM)
secrets-store-csi-driver-operator #95 ConsoleQuickStart, ConsoleYAMLSample CVO annotation (SSCSI-235)
cluster-version-operator #1388 ConsolePlugin CVO annotation (OTA-1956, TechPreview WIP)

Annotation removed / Console resources deleted entirely:

Operator PR Notes
lightspeed-agentic-operator #249 Removed ConsolePlugin deployment entirely (OLS-3350)

Test framework fixes:

Component PR Notes
openshift/origin #27439 Console disruption monitoring conditional (TRT-573)
openshift/origin #27681 Console RBAC expectations conditional
openshift/assisted-service #5022 Cluster deployment completion check (MGMT-13941)
openshift/hypershift #6183, #8839 Console capability in hosted clusters (OTA-1956)

⚠️ Key lesson from CMO and kube-apiserver-operator

Adding capability.openshift.io/name: Console to existing manifests during an upgrade causes CVO to force-enable the Console capability on clusters that had it disabled (OCPBUGS-20331). Both CMO (#2118 revert, #2254 re-fix) and kube-apiserver-operator (#1565) had to rename their manifests to work around this. The runtime detection approach used in this PR avoids this pitfall entirely.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2026
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jul 8, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@sebrandon1: This pull request references Jira Issue OCPBUGS-85579, which is invalid:

  • expected the bug to target either version "5.0." or "openshift-5.0.", but it targets "4.22" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Release Pending (Done) instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Problem

PR #424 added capability.openshift.io/name: Console annotations to Console resources (ConsoleYAMLSample, ConsoleQuickStart) to support consoleless RAN/RDS clusters. This annotation is only processed by CVO for core platform components — it is not processed by OLM for third-party operator bundles.

Additionally, the olm.manifests.optional property referenced in the OLM enhancement proposal was never implemented (closed as "not planned").

Result: cert-manager-operator still fails on consoleless clusters with:

GroupVersionKind console.openshift.io/v1, Kind=ConsoleYAMLSample not found

Solution

Move Console resource creation from the OLM bundle to the operator runtime, following the pattern established by cluster-monitoring-operator PR #2011 (OCPBUGS-14922).

The operator now:

  1. Checks ClusterVersion.status.capabilities.enabledCapabilities for the Console capability
  2. If Console is enabled: creates ConsoleYAMLSample and ConsoleQuickStart resources via dynamic client
  3. If Console is disabled: skips silently

This supersedes PR #424.

Changes

  • Removed 4 Console manifests from bundle/manifests/ (OLM no longer creates them)
  • Added pkg/controller/certmanager/console_resources.go — new controller with Console capability check
  • Added Console YAML files to bindata/console/ for runtime embedding
  • Added RBAC for console.openshift.io resources and config.openshift.io/clusterversions
  • Removed ineffective capability.openshift.io/name: Console annotations from config/console/ source templates

Behavior

Cluster Type Behavior
Consoleless (RAN/RDS/SNO) InstallPlan succeeds, Console resources not created
Console-enabled (standard OCP) InstallPlan succeeds, Console resources created by operator at runtime

Testing

Verified on OCP 4.22 (cnfdt16):

  • Console-enabled: InstallPlan phase Complete, no regression
  • Simulated consoleless (CRDs removed via CVO override): InstallPlan phase Complete, no GVK errors

References

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Walkthrough

This PR adds embedded console sample and quickstart assets, reconciles them conditionally through a new controller, and updates operator wiring and RBAC to support console resource management.

Changes

Console resources

Layer / File(s) Summary
Assets and manifests
bindata/console/*.yaml, pkg/operator/assets/bindata.go, config/manifests/kustomization.yaml
Adds the quickstart, adjusts the console sample YAMLs, updates generated embedded assets, and removes the console manifest input from the bundle.
Console controller
pkg/controller/certmanager/console_resources.go
Adds a controller that preloads console assets, checks ClusterVersion capability, maps supported kinds to GVRs, and applies resources only when Console is enabled.
Controller wiring
pkg/controller/certmanager/cert_manager_controller_set.go, pkg/operator/starter.go
Creates a dynamic client at startup, passes it into the controller set, and registers the console resources controller.
RBAC permissions
bundle/manifests/cert-manager-operator.clusterserviceversion.yaml, config/rbac/role.yaml
Expands permissions for clusterversions, consolequickstarts, and consoleyamlsamples.
Tests
pkg/controller/certmanager/console_resources_test.go
Adds coverage for capability checks, sync behavior, GVR mapping, and asset parsing.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed No Ginkgo titles were added; the new tests use static t.Run names like "console enabled" and "Unknown", with no dynamic suffixes or volatile values.
Test Structure And Quality ✅ Passed PASS: The new tests are plain table-driven unit tests using fake clients only; no Ginkgo, cluster resources, Eventually calls, or cleanup issues.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the new test file uses plain testing unit tests and no MicroShift-sensitive cluster assumptions.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the only new tests are unit tests in pkg/controller/certmanager using testing.T, so there are no SNO-specific assumptions to flag.
Topology-Aware Scheduling Compatibility ✅ Passed No new pod placement, replica, affinity, PDB, or nodeSelector constraints were introduced; changes only add runtime Console resource reconciliation and RBAC.
Ote Binary Stdout Contract ✅ Passed PR adds no OTE binary stdout writes; changed code is controller/runtime logic only, and suite logging is routed to GinkgoWriter/stderr.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the only new test file is a Go unit test using package testing, with no IPv4 or external-network assumptions.
No-Weak-Crypto ✅ Passed No weak-crypto primitives, custom crypto, or secret/token comparisons appear in the PR changes.
Container-Privileges ✅ Passed No touched manifest adds privileged/root/host* or CAP_SYS_ADMIN settings; diff search found no such additions.
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data is logged; new logs are generic status/errors and don't include credentials, PII, or customer data.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: moving Console resources from the bundle to runtime with capability detection.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot requested review from TrilokGeer and swghosh July 8, 2026 01:25
@openshift-ci

openshift-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sebrandon1
Once this PR has been reviewed and has the lgtm label, please assign mytreya-rh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (4)
bindata/console/cert-manager-example-quickstart.yaml (1)

5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stale/inconsistent include.release.openshift.io/* annotations.

These CVO release-payload inclusion annotations are inert for a resource applied via dynamic client at runtime, and this PR removed the same annotations from the sibling ConsoleYAMLSample manifests (acme-issuer, certificate, issuer) and reportedly from config/console/cert-manager-example-quickstart.yaml as well. Leaving them here looks like a leftover from before this PR's annotation cleanup rather than an intentional keep.

🧹 Proposed cleanup
 metadata:
   name: cert-manager-example
-  annotations:
-    include.release.openshift.io/ibm-cloud-managed: "true"
-    include.release.openshift.io/self-managed-high-availability: "true"
-    include.release.openshift.io/single-node-developer: "true"
 spec:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindata/console/cert-manager-example-quickstart.yaml` around lines 5 - 8,
Remove the stale include.release.openshift.io/* annotations from the
cert-manager example quickstart manifest so it stays consistent with the
cleaned-up sibling ConsoleYAMLSample manifests. Update the
cert-manager-example-quickstart YAML by deleting the annotations block under the
sample metadata, keeping the resource aligned with the annotation cleanup
already applied elsewhere.
bundle/manifests/cert-manager-operator.clusterserviceversion.yaml (1)

413-431: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

RBAC grants broader verbs than the controller actually uses.

consoleResourcesController (pkg/controller/certmanager/console_resources.go) only calls ClusterVersions().Get(...) for a single named object and Get/Create/Update on consoleyamlsamples/consolequickstarts. The new rules additionally grant list/watch on clusterversions and delete/patch/watch on the console resources, none of which are exercised by this controller.

🔒 Proposed trim to match actual usage
         - apiGroups:
           - config.openshift.io
           resources:
           - clusterversions
           - featuregates
           verbs:
           - get
-          - list
-          - watch
         - apiGroups:
           - console.openshift.io
           resources:
           - consolequickstarts
           - consoleyamlsamples
           verbs:
-          - create
-          - delete
-          - get
-          - list
-          - patch
-          - update
-          - watch
+          - create
+          - get
+          - update

As per path instructions, **/*.{yaml,yml} Kubernetes manifests should follow "RBAC: least privilege; no cluster-admin for workloads."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bundle/manifests/cert-manager-operator.clusterserviceversion.yaml` around
lines 413 - 431, Trim the RBAC in the CSV manifest to match
consoleResourcesController’s actual usage: in
pkg/controller/certmanager/console_resources.go it only needs
ClusterVersions().Get for a single object and Get/Create/Update on
consoleyamlsamples and consolequickstarts. Remove the extra list/watch verbs for
clusterversions and the delete/patch/watch verbs on the console resources so the
permissions follow least privilege.

Source: Path instructions

pkg/controller/certmanager/console_resources.go (2)

80-147: 🎯 Functional Correctness | 🔵 Trivial

No test coverage for the new controller.

sync, hasConsoleCapability, and applyConsoleResource implement the core capability-gating and apply logic for this PR but ship with no unit tests (e.g. using fake dynamic/config clientsets) to verify skip-on-disabled-capability, create-on-not-found, and update-on-existing paths.

Do you want me to draft unit tests using k8s.io/client-go/dynamic/fake and the fake config clientset for these three code paths?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/controller/certmanager/console_resources.go` around lines 80 - 147, Add
unit tests for consoleResourcesController covering the new sync,
hasConsoleCapability, and applyConsoleResource paths using fake config/dynamic
clients. Verify sync skips when Console capability is disabled, proceeds when
enabled, and returns wrapped errors from capability lookup or apply failures.
Also test applyConsoleResource for create-on-not-found and update-on-existing
behavior, referencing consoleResourcesController, hasConsoleCapability, and
applyConsoleResource so the tests stay resilient if lines move.

149-156: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Silent GVR fallback for unrecognized kinds.

gvrForKind maps any kind other than "ConsoleQuickStart" to consoleYAMLSampleGVR, including kinds that aren't ConsoleYAMLSample. If a future asset file has an unexpected/typo'd kind, this will silently route the apply to the wrong GVR instead of failing with a clear error at decode time.

♻️ Proposed fix: fail explicitly on unknown kinds
-func gvrForKind(kind string) schema.GroupVersionResource {
-	switch kind {
-	case "ConsoleQuickStart":
-		return consoleQuickStartGVR
-	default:
-		return consoleYAMLSampleGVR
-	}
-}
+func gvrForKind(kind string) (schema.GroupVersionResource, error) {
+	switch kind {
+	case "ConsoleQuickStart":
+		return consoleQuickStartGVR, nil
+	case "ConsoleYAMLSample":
+		return consoleYAMLSampleGVR, nil
+	default:
+		return schema.GroupVersionResource{}, fmt.Errorf("unsupported console resource kind %q", kind)
+	}
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/controller/certmanager/console_resources.go` around lines 149 - 156, The
gvrForKind helper currently falls back to consoleYAMLSampleGVR for every unknown
kind, which can silently apply the wrong resource. Update gvrForKind in
console_resources.go to explicitly recognize only the supported kinds (such as
ConsoleQuickStart and ConsoleYAMLSample) and return an error or fail fast for
anything else, then propagate that failure through the caller that decodes the
asset so unexpected or misspelled kind values are rejected instead of routed
incorrectly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@bindata/console/cert-manager-example-quickstart.yaml`:
- Around line 5-8: Remove the stale include.release.openshift.io/* annotations
from the cert-manager example quickstart manifest so it stays consistent with
the cleaned-up sibling ConsoleYAMLSample manifests. Update the
cert-manager-example-quickstart YAML by deleting the annotations block under the
sample metadata, keeping the resource aligned with the annotation cleanup
already applied elsewhere.

In `@bundle/manifests/cert-manager-operator.clusterserviceversion.yaml`:
- Around line 413-431: Trim the RBAC in the CSV manifest to match
consoleResourcesController’s actual usage: in
pkg/controller/certmanager/console_resources.go it only needs
ClusterVersions().Get for a single object and Get/Create/Update on
consoleyamlsamples and consolequickstarts. Remove the extra list/watch verbs for
clusterversions and the delete/patch/watch verbs on the console resources so the
permissions follow least privilege.

In `@pkg/controller/certmanager/console_resources.go`:
- Around line 80-147: Add unit tests for consoleResourcesController covering the
new sync, hasConsoleCapability, and applyConsoleResource paths using fake
config/dynamic clients. Verify sync skips when Console capability is disabled,
proceeds when enabled, and returns wrapped errors from capability lookup or
apply failures. Also test applyConsoleResource for create-on-not-found and
update-on-existing behavior, referencing consoleResourcesController,
hasConsoleCapability, and applyConsoleResource so the tests stay resilient if
lines move.
- Around line 149-156: The gvrForKind helper currently falls back to
consoleYAMLSampleGVR for every unknown kind, which can silently apply the wrong
resource. Update gvrForKind in console_resources.go to explicitly recognize only
the supported kinds (such as ConsoleQuickStart and ConsoleYAMLSample) and return
an error or fail fast for anything else, then propagate that failure through the
caller that decodes the asset so unexpected or misspelled kind values are
rejected instead of routed incorrectly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8f662201-1b0b-4379-96b5-0476cecc4279

📥 Commits

Reviewing files that changed from the base of the PR and between 454cb88 and 07d3553.

📒 Files selected for processing (14)
  • bindata/console/cert-manager-acme-issuer-sample.yaml
  • bindata/console/cert-manager-certificate-sample.yaml
  • bindata/console/cert-manager-example-quickstart.yaml
  • bindata/console/cert-manager-issuer-sample.yaml
  • bundle/manifests/cert-manager-example_console.openshift.io_v1_consolequickstart.yaml
  • bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
  • config/console/cert-manager-acme-issuer-sample.yaml
  • config/console/cert-manager-certificate-sample.yaml
  • config/console/cert-manager-example-quickstart.yaml
  • config/console/cert-manager-issuer-sample.yaml
  • pkg/controller/certmanager/cert_manager_controller_set.go
  • pkg/controller/certmanager/console_resources.go
  • pkg/operator/assets/bindata.go
  • pkg/operator/starter.go
💤 Files with no reviewable changes (5)
  • config/console/cert-manager-example-quickstart.yaml
  • config/console/cert-manager-acme-issuer-sample.yaml
  • config/console/cert-manager-certificate-sample.yaml
  • bundle/manifests/cert-manager-example_console.openshift.io_v1_consolequickstart.yaml
  • config/console/cert-manager-issuer-sample.yaml

@sebrandon1 sebrandon1 force-pushed the fix-consoleless-runtime branch from ff46829 to ef15415 Compare July 8, 2026 02:55

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
pkg/controller/certmanager/console_resources.go (2)

80-91: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Panic on embedded asset load/parse failure.

NewConsoleResourcesController panics if any embedded console asset fails to load or has an unrecognized kind. Since these are compiled-in assets this is unlikely to fail at runtime, but a panic here crashes the whole operator process at startup rather than degrading just this controller.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/controller/certmanager/console_resources.go` around lines 80 - 91,
NewConsoleResourcesController currently panics inside the consoleAssetFiles load
loop when assets.Asset, resourceread.ReadUnstructuredOrDie, or gvrForKind fails,
which can crash the operator at startup. Replace those panic paths with error
propagation from NewConsoleResourcesController and have the caller handle the
failure so only the console resources setup is affected; use the existing
consoleAssetFiles iteration and consoleAsset initialization points to wire the
error handling through cleanly.

50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

No cleanup path if Console capability is later disabled/unset.

RBAC includes delete for consoleyamlsamples/consolequickstarts, but sync() only ever applies resources when enabled and does nothing (not even cleanup) when disabled. If this is intentional (capabilities are generally not revocable post-enable), consider dropping the unused delete verb or adding a comment clarifying the decision.

Also applies to: 110-118

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/controller/certmanager/console_resources.go` around lines 50 - 51, The
RBAC for console resources grants delete access, but the controller path in
sync() only creates/updates when the Console capability is enabled and never
performs cleanup when it is disabled or unset. Either remove the unused delete
verb from the kubebuilder RBAC markers on console_resources.go (including the
related markers around the console YAML samples/quickstarts permissions) or add
a clear comment in sync() explaining that capability disablement is
intentionally not reconciled and no cleanup is expected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/controller/certmanager/console_resources.go`:
- Around line 93-97: The controller setup in consoleResourcesController only
watches operatorClient.Informer(), so changes in ClusterVersion and transient
GET failures in hasConsoleCapability won’t retrigger reconciliation. Update the
controller wiring to add either a periodic resync or a ClusterVersion
informer/watch alongside the existing operatorClient.Informer() in the
factory.New() chain. Keep the existing sync logic intact, and make sure
reconcile can be triggered when ClusterVersion changes even if operator config
does not.

---

Nitpick comments:
In `@pkg/controller/certmanager/console_resources.go`:
- Around line 80-91: NewConsoleResourcesController currently panics inside the
consoleAssetFiles load loop when assets.Asset,
resourceread.ReadUnstructuredOrDie, or gvrForKind fails, which can crash the
operator at startup. Replace those panic paths with error propagation from
NewConsoleResourcesController and have the caller handle the failure so only the
console resources setup is affected; use the existing consoleAssetFiles
iteration and consoleAsset initialization points to wire the error handling
through cleanly.
- Around line 50-51: The RBAC for console resources grants delete access, but
the controller path in sync() only creates/updates when the Console capability
is enabled and never performs cleanup when it is disabled or unset. Either
remove the unused delete verb from the kubebuilder RBAC markers on
console_resources.go (including the related markers around the console YAML
samples/quickstarts permissions) or add a clear comment in sync() explaining
that capability disablement is intentionally not reconciled and no cleanup is
expected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6e8aa412-343b-4c09-a1a4-81abd03722f7

📥 Commits

Reviewing files that changed from the base of the PR and between ff46829 and ef15415.

📒 Files selected for processing (16)
  • bindata/console/cert-manager-acme-issuer-sample.yaml
  • bindata/console/cert-manager-certificate-sample.yaml
  • bindata/console/cert-manager-example-quickstart.yaml
  • bindata/console/cert-manager-issuer-sample.yaml
  • bundle/manifests/cert-manager-example_console.openshift.io_v1_consolequickstart.yaml
  • bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
  • config/console/cert-manager-acme-issuer-sample.yaml
  • config/console/cert-manager-certificate-sample.yaml
  • config/console/cert-manager-example-quickstart.yaml
  • config/console/cert-manager-issuer-sample.yaml
  • config/manifests/kustomization.yaml
  • config/rbac/role.yaml
  • pkg/controller/certmanager/cert_manager_controller_set.go
  • pkg/controller/certmanager/console_resources.go
  • pkg/operator/assets/bindata.go
  • pkg/operator/starter.go
💤 Files with no reviewable changes (6)
  • bundle/manifests/cert-manager-example_console.openshift.io_v1_consolequickstart.yaml
  • config/console/cert-manager-issuer-sample.yaml
  • config/manifests/kustomization.yaml
  • config/console/cert-manager-acme-issuer-sample.yaml
  • config/console/cert-manager-certificate-sample.yaml
  • config/console/cert-manager-example-quickstart.yaml
✅ Files skipped from review due to trivial changes (1)
  • pkg/operator/assets/bindata.go
🚧 Files skipped from review as they are similar to previous changes (7)
  • bindata/console/cert-manager-issuer-sample.yaml
  • bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
  • bindata/console/cert-manager-certificate-sample.yaml
  • bindata/console/cert-manager-example-quickstart.yaml
  • config/rbac/role.yaml
  • pkg/operator/starter.go
  • pkg/controller/certmanager/cert_manager_controller_set.go

Comment on lines +93 to +97
return factory.New().
WithInformers(operatorClient.Informer()).
WithSync(c.sync).
ToController(consoleResourcesControllerName, c.eventRecorder)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== target file ==\n'
git ls-files 'pkg/controller/certmanager/console_resources.go' && cat -n pkg/controller/certmanager/console_resources.go | sed -n '1,220p'

printf '\n== search for resync usage ==\n'
rg -n 'ResyncEvery|ResyncSchedule|WithInformers\(|ClusterVersion|hasConsoleCapability' pkg/controller -S

printf '\n== similar controller patterns ==\n'
rg -n 'ResyncEvery\(|ResyncSchedule\(' pkg/controller -S

Repository: openshift/cert-manager-operator

Length of output: 6818


Add a periodic resync or watch ClusterVersion here
WithInformers(operatorClient.Informer()) is the only trigger, but hasConsoleCapability does a live ClusterVersion GET. Without a resync or a ClusterVersion watch, capability changes and transient API failures won’t retrigger reconciliation until some operator-config change happens.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/controller/certmanager/console_resources.go` around lines 93 - 97, The
controller setup in consoleResourcesController only watches
operatorClient.Informer(), so changes in ClusterVersion and transient GET
failures in hasConsoleCapability won’t retrigger reconciliation. Update the
controller wiring to add either a periodic resync or a ClusterVersion
informer/watch alongside the existing operatorClient.Informer() in the
factory.New() chain. Keep the existing sync logic intact, and make sure
reconcile can be triggered when ClusterVersion changes even if operator config
does not.

…ability detection

Remove ConsoleYAMLSample and ConsoleQuickStart manifests from the OLM
bundle and create them at runtime only when the Console capability is
enabled on the cluster.

On consoleless RAN/RDS clusters, the operator now skips Console resource
creation instead of failing during InstallPlan execution.

This follows the pattern established by cluster-monitoring-operator
(PR #2011, OCPBUGS-14922) for handling optional Console resources in
OLM-managed operators.

Changes:
- Remove 4 Console manifests from bundle/manifests/
- Remove config/console from kustomize manifests generation
- Add console_resources.go controller that checks ClusterVersion
  capabilities before creating Console resources via dynamic client
- Use library-go ApplyUnstructuredResourceImproved for idempotent
  create-or-update with change detection
- Add Console YAML files to bindata/ for runtime embedding
- Add RBAC for console.openshift.io and clusterversions via
  kubebuilder markers
- Remove ineffective capability.openshift.io/name annotations
@sebrandon1 sebrandon1 force-pushed the fix-consoleless-runtime branch from ef15415 to cfbca6e Compare July 8, 2026 03:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
pkg/controller/certmanager/console_resources_test.go (1)

118-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicate asset-loading/parsing logic.

The load-parse-resolve-GVR loop is repeated verbatim in TestSyncConsoleEnabled (Lines 118-129) and TestConsoleAssetsPreParsed (Lines 184-199). Extracting a small helper (e.g., loadConsoleAssets(t) []consoleAsset) would reduce duplication and keep both tests in sync if the asset format changes.

Also applies to: 183-201

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/controller/certmanager/console_resources_test.go` around lines 118 - 129,
The asset-loading/parsing loop in TestSyncConsoleEnabled and
TestConsoleAssetsPreParsed is duplicated, so extract the shared
load-parse-resolve-GVR logic into a small helper like loadConsoleAssets(t)
[]consoleAsset and have both tests call it. Keep the helper centered around
assets.Asset, resourceread.ReadUnstructuredOrDie, and gvrForKind so any future
asset-format change only needs one update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/controller/certmanager/console_resources_test.go`:
- Around line 118-129: The asset-loading/parsing loop in TestSyncConsoleEnabled
and TestConsoleAssetsPreParsed is duplicated, so extract the shared
load-parse-resolve-GVR logic into a small helper like loadConsoleAssets(t)
[]consoleAsset and have both tests call it. Keep the helper centered around
assets.Asset, resourceread.ReadUnstructuredOrDie, and gvrForKind so any future
asset-format change only needs one update.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5743956a-24cb-423e-a650-4a6545f60885

📥 Commits

Reviewing files that changed from the base of the PR and between ef15415 and cfbca6e.

📒 Files selected for processing (17)
  • bindata/console/cert-manager-acme-issuer-sample.yaml
  • bindata/console/cert-manager-certificate-sample.yaml
  • bindata/console/cert-manager-example-quickstart.yaml
  • bindata/console/cert-manager-issuer-sample.yaml
  • bundle/manifests/cert-manager-example_console.openshift.io_v1_consolequickstart.yaml
  • bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
  • config/console/cert-manager-acme-issuer-sample.yaml
  • config/console/cert-manager-certificate-sample.yaml
  • config/console/cert-manager-example-quickstart.yaml
  • config/console/cert-manager-issuer-sample.yaml
  • config/manifests/kustomization.yaml
  • config/rbac/role.yaml
  • pkg/controller/certmanager/cert_manager_controller_set.go
  • pkg/controller/certmanager/console_resources.go
  • pkg/controller/certmanager/console_resources_test.go
  • pkg/operator/assets/bindata.go
  • pkg/operator/starter.go
💤 Files with no reviewable changes (6)
  • config/console/cert-manager-acme-issuer-sample.yaml
  • bundle/manifests/cert-manager-example_console.openshift.io_v1_consolequickstart.yaml
  • config/console/cert-manager-certificate-sample.yaml
  • config/console/cert-manager-example-quickstart.yaml
  • config/console/cert-manager-issuer-sample.yaml
  • config/manifests/kustomization.yaml
✅ Files skipped from review due to trivial changes (2)
  • bindata/console/cert-manager-example-quickstart.yaml
  • pkg/operator/assets/bindata.go
🚧 Files skipped from review as they are similar to previous changes (8)
  • config/rbac/role.yaml
  • bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
  • pkg/operator/starter.go
  • pkg/controller/certmanager/cert_manager_controller_set.go
  • bindata/console/cert-manager-certificate-sample.yaml
  • bindata/console/cert-manager-issuer-sample.yaml
  • bindata/console/cert-manager-acme-issuer-sample.yaml
  • pkg/controller/certmanager/console_resources.go

@openshift-ci

openshift-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@sebrandon1: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@sebrandon1 sebrandon1 changed the title WIP: OCPBUGS-85579: Move Console resources from bundle to runtime with capability detection OCPBUGS-85579: Move Console resources from bundle to runtime with capability detection Jul 8, 2026
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants