feat: add agent-navigation feature flag and uiplugin field#1107
feat: add agent-navigation feature flag and uiplugin field#1107alanconway wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 55 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds support for an Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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: 3
🧹 Nitpick comments (1)
pkg/controllers/uiplugin/troubleshooting_panel.go (1)
37-39: ⚡ Quick winClone and dedupe
featuresbefore appending.This mutates the caller-owned slice and can also emit
agent-navigationtwice when the CLI already enabled it. Build a local copy and only append when the feature is not already present.Proposed fix
- if plugin.Spec.TroubleshootingPanel != nil && plugin.Spec.TroubleshootingPanel.EnableAgentNavigation { - features = append(features, "agent-navigation") - } + mergedFeatures := append([]string(nil), features...) + if plugin.Spec.TroubleshootingPanel != nil && plugin.Spec.TroubleshootingPanel.EnableAgentNavigation { + alreadyEnabled := false + for _, feature := range mergedFeatures { + if feature == "agent-navigation" { + alreadyEnabled = true + break + } + } + if !alreadyEnabled { + mergedFeatures = append(mergedFeatures, "agent-navigation") + } + } - if len(features) > 0 { - extraArgs = append(extraArgs, fmt.Sprintf("-features=%s", strings.Join(features, ","))) + if len(mergedFeatures) > 0 { + extraArgs = append(extraArgs, fmt.Sprintf("-features=%s", strings.Join(mergedFeatures, ","))) }🤖 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/controllers/uiplugin/troubleshooting_panel.go` around lines 37 - 39, The code currently mutates the caller-owned features slice and can duplicate "agent-navigation"; instead, create a local copy of features (e.g., newFeatures := append([]string{}, features...>) and then check for presence of "agent-navigation" before appending when plugin.Spec.TroubleshootingPanel != nil && plugin.Spec.TroubleshootingPanel.EnableAgentNavigation; finally, assign or return the deduped newFeatures so the original caller slice is not mutated and duplicates are avoided (refer to the features variable and plugin.Spec.TroubleshootingPanel.EnableAgentNavigation).
🤖 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 `@bundle/manifests/observability-operator.clusterserviceversion.yaml`:
- Line 62: The CSV currently sets its identity to
"observability-operator.v0.0.0-dev" which regresses the bundle version and can
break OLM upgrades; update the CSV identity/version string to a forward semver
suitable for a published channel (e.g., bump to a release like
"observability-operator.v1.4.0" or higher) and apply the same change to the
other occurrence of the dev version in the manifest so both entries match;
retain "0.0.0-dev" only in local/dev-only artifacts and ensure the manifest's
CSV version fields are monotonic with prior published releases.
In `@deploy/olm/kustomization.yaml`:
- Around line 15-16: The kustomization entries newName and newTag currently
point to a personal dev image; update them to reference the project-owned
release image or a CI-templated variable instead (replace newName/newTag values
in deploy/olm/kustomization.yaml so newName uses the organization/release
repository and newTag uses a real release tag or a CI-injected variable like
RELEASE_IMAGE or IMAGE_TAG); ensure the replacement uses the canonical
registry/repo used by releases so distributable OLM overlays are reproducible
and not tied to a personal dev image.
In `@deploy/package-operator/operator/kustomization.yaml`:
- Around line 6-7: The overlay currently pins newName and newTag to a dev image
(newName: quay.io/alanconway/observability-operator and newTag: 0.0.0-dev);
remove this unsafe dev override and align with the project release-image
strategy by either deleting the newTag/newName overrides or replacing newTag
with the release variable/token used by your CI/CD (e.g. ${RELEASE_TAG} or the
canonical production tag) and ensuring newName points to the project-owned image
repository; update references to newName/newTag in the kustomization so shared
packaging manifests do not contain hard-coded dev images.
---
Nitpick comments:
In `@pkg/controllers/uiplugin/troubleshooting_panel.go`:
- Around line 37-39: The code currently mutates the caller-owned features slice
and can duplicate "agent-navigation"; instead, create a local copy of features
(e.g., newFeatures := append([]string{}, features...>) and then check for
presence of "agent-navigation" before appending when
plugin.Spec.TroubleshootingPanel != nil &&
plugin.Spec.TroubleshootingPanel.EnableAgentNavigation; finally, assign or
return the deduped newFeatures so the original caller slice is not mutated and
duplicates are avoided (refer to the features variable and
plugin.Spec.TroubleshootingPanel.EnableAgentNavigation).
🪄 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 YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 40d13924-36a4-44b6-ae00-7ac909571f2b
📒 Files selected for processing (9)
bundle/manifests/observability-operator.clusterserviceversion.yamlbundle/manifests/observability.openshift.io_uiplugins.yamldeploy/crds/common/observability.openshift.io_uiplugins.yamldeploy/olm/kustomization.yamldeploy/package-operator/operator/kustomization.yamldocs/api.mdpkg/apis/uiplugin/v1alpha1/types.gopkg/controllers/uiplugin/troubleshooting_panel.gopkg/controllers/uiplugin/troubleshooting_panel_test.go
3abab8c to
1e6ace6
Compare
1e6ace6 to
0bcaa6b
Compare
uiplugin.enableFeatureNavigation bool - set via resource --features "agent-navigation" - set via command line
0bcaa6b to
7e871bf
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alanconway, PeterYurkovich The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
uiplugin.enableFeatureNavigation bool - set via resource
--features "agent-navigation" - set via command line