NETOBSERV-2767: remove Console CR config edition#2791
Conversation
- Deprecating field `spec.consolePlugin.advanced.register` (it's not used anymore) - Remove "auto-patch" code that edits the Console config for adding our plugins - Remove all permissions related to Console CR - Add the console patch command in our "make deploy" script
|
@jotak: This pull request references NETOBSERV-2767 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.19.z" instead. DetailsIn response to this:
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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedToo many files! This PR contains 175 files, which is 25 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (175)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR replaces the operator's automatic console plugin registration with manual deployment. RBAC permissions shift from managing OpenShift consoles (with update capability) to reading networks (read-only). The auto-patch reconciliation logic is removed, and console plugin registration is now a manual kubectl patch operation in the deployment target. The register field is marked deprecated. ChangesConsole Plugin Registration Migration
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@helm/crds/flows.netobserv.io_flowcollectors.yaml`:
- Around line 1633-1637: Remove the default value for the deprecated boolean
field so the API server no longer materializes it: edit the FlowCollector CRD
schema entry for spec.consolePlugin.advanced.register and delete the "default:
true" line (keep the field present for compatibility), then regenerate the CRD
output so the published flows.netobserv.io_flowcollectors.yaml no longer
contains that default.
In `@Makefile`:
- Line 421: The current kubectl JSON-patch that uses two "add" ops to append
plugins is not idempotent and fails when spec.plugins is absent; replace that
JSON-patch command with a single merge-style patch that sets spec.plugins to the
exact array of plugins (["netobserv-plugin","netobserv-plugin-static"]) on the
console.operator.openshift.io cluster resource so the operation creates the
field if missing, overwrites any previous values (preventing duplicates), and is
idempotent; update the Makefile line that runs the kubectl patch for
console.operator.openshift.io cluster accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c1326c49-442f-4fcc-a3a5-ec490ff368b8
📒 Files selected for processing (14)
Makefileapi/flowcollector/v1beta2/flowcollector_types.gobundle/manifests/flows.netobserv.io_flowcollectors.yamlbundle/manifests/netobserv-operator.clusterserviceversion.yamlconfig/crd/bases/flows.netobserv.io_flowcollectors.yamlconfig/rbac/role.yamldocs/FlowCollector.mdhelm/crds/flows.netobserv.io_flowcollectors.yamlhelm/templates/clusterrole.yamlinternal/controller/consoleplugin/consoleplugin_reconciler.gointernal/controller/consoleplugin/consoleplugin_static_reconciler.gointernal/controller/flowcollector_controller_console_test.gointernal/pkg/helper/flowcollector.gointernal/pkg/manager/manager.go
💤 Files with no reviewable changes (6)
- bundle/manifests/netobserv-operator.clusterserviceversion.yaml
- internal/controller/consoleplugin/consoleplugin_static_reconciler.go
- internal/pkg/manager/manager.go
- internal/controller/consoleplugin/consoleplugin_reconciler.go
- config/rbac/role.yaml
- helm/templates/clusterrole.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2791 +/- ##
============================
============================
🚀 New features to boost your workflow:
|
|
/ok-to-test |
|
New images: quay.io/netobserv/network-observability-operator:e267e1c
quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-e267e1c
quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-e267e1cThey will expire in two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:e267e1c make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-sha-e267e1cOr as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-sha-e267e1c
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
|
@jotak - I tried the image from this PR, looks like now the users (installing from CLI) will have to patch the console operator to enable and disable (when uninstalling operator) our plugins manually: to enable: to disable: the most immediate impact would be on e2e tests. |
Yes that's right, I forgot to mention that. It's just for dev/test environment, typical users won't need that. So we need to update our scripts wherever it's relevant. I'll update |
Description
spec.consolePlugin.advanced.register(it's not used anymore)Dependencies
n/a
Checklist
Summary by CodeRabbit
Deprecations
registerfield in FlowCollector.spec.consolePlugin is deprecated and no longer used; OpenShift now uses the Console plugins interface instead.Improvements
Documentation