fix: Add 'create tokenreview' to korrel8r RBAC#1073
fix: Add 'create tokenreview' to korrel8r RBAC#1073alanconway wants to merge 1 commit intorhobs:mainfrom
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR reorders a kubebuilder RBAC comment header in the UI plugin controller, adds a PolicyRule to korrel8rClusterRole granting create on authentication.k8s.io/tokenreviews, adds unit tests validating the ClusterRole and its rules, and introduces a phony Makefile target Estimated code review effort🎯 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
deploy/package-operator/operator/kustomization.yaml (1)
6-7:⚠️ Potential issue | 🔴 CriticalSame personal dev image override — revert before merge.
Mirrors the issue in
deploy/olm/kustomization.yaml. The package-operator package should not ship withquay.io/alanconway/observability-operator:0.0.10-devas the resolved image. Please restore the upstreamobservability-operatorimage reference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/package-operator/operator/kustomization.yaml` around lines 6 - 7, The package-operator kustomization currently hardcodes your personal dev image via the newName/newTag overrides (newName: quay.io/alanconway/observability-operator, newTag: 0.0.10-dev); revert these lines to remove the personal override and restore the upstream observability-operator image reference (i.e., remove or replace the newName/newTag entries with the canonical upstream image/tag used by the project), and mirror the same revert in deploy/olm/kustomization.yaml so both kustomizations reference the official observability-operator image.
🧹 Nitpick comments (1)
pkg/controllers/uiplugin/controller.go (1)
111-114: Duplicate kubebuilder RBAC marker fortokenreviews/create.The marker you added on line 111 is identical to the one already present on line 114 (under "RBAC for Health Analyzer").
controller-genwill dedupe so there's no generated-manifest impact, but having two copies invites drift if one is later changed. Consider keeping a single marker and adjusting the comment to mention both consumers (korrel8r and Health Analyzer).Suggested consolidation
//+kubebuilder:rbac:groups=monitoring.coreos.com,resources=alertmanagers/api,resourceNames=main,verbs=get;list -//+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create -// RBAC for Health Analyzer +// RBAC for Health Analyzer and korrel8r (TokenReview-based session auth) //+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/uiplugin/controller.go` around lines 111 - 114, Remove the duplicate kubebuilder RBAC marker so there is only a single "//+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create" in the file; consolidate the two adjacent comment blocks (the one labeled "RBAC for Health Analyzer" and the other for korrel8r) into a single comment that notes both consumers (korrel8r and Health Analyzer) and keep the single kubebuilder marker above that consolidated comment to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/olm/kustomization.yaml`:
- Around line 15-16: Revert the local/dev image override in kustomization.yaml
by removing or restoring the changed newName and newTag entries (the lines
setting newName: quay.io/alanconway/observability-operator and newTag:
0.0.10-dev) so the kustomization uses the canonical project image and stable
tag; locate the newName/newTag keys in the kustomization manifest and replace
them with the upstream image name and tag (or delete the override) before
merging.
---
Duplicate comments:
In `@deploy/package-operator/operator/kustomization.yaml`:
- Around line 6-7: The package-operator kustomization currently hardcodes your
personal dev image via the newName/newTag overrides (newName:
quay.io/alanconway/observability-operator, newTag: 0.0.10-dev); revert these
lines to remove the personal override and restore the upstream
observability-operator image reference (i.e., remove or replace the
newName/newTag entries with the canonical upstream image/tag used by the
project), and mirror the same revert in deploy/olm/kustomization.yaml so both
kustomizations reference the official observability-operator image.
---
Nitpick comments:
In `@pkg/controllers/uiplugin/controller.go`:
- Around line 111-114: Remove the duplicate kubebuilder RBAC marker so there is
only a single
"//+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create"
in the file; consolidate the two adjacent comment blocks (the one labeled "RBAC
for Health Analyzer" and the other for korrel8r) into a single comment that
notes both consumers (korrel8r and Health Analyzer) and keep the single
kubebuilder marker above that consolidated comment to avoid drift.
🪄 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: 80e53d24-ab3e-45c1-b513-21cbc8aaa84a
📒 Files selected for processing (5)
deploy/olm/kustomization.yamldeploy/package-operator/operator/kustomization.yamlpkg/controllers/uiplugin/controller.gopkg/controllers/uiplugin/troubleshooting_panel.gopkg/controllers/uiplugin/troubleshooting_panel_test.go
alanconway
force-pushed
the
korrel8r-tokenreview
branch
from
a0d2112 to
c6d9f8a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Makefile`:
- Around line 23-24: Add a .PHONY declaration for the Makefile "all" target so
Make won’t treat a file named "all" as an up-to-date target; update the existing
phony declarations block (where other phony targets are declared) to include
"all" (i.e., add .PHONY: all alongside the current .PHONY entries) to ensure the
all: lint test-unit operator-image bundle-image recipe always runs.
🪄 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: fedac21a-8e25-4a36-95b2-55d3375c7741
📒 Files selected for processing (4)
Makefilepkg/controllers/uiplugin/controller.gopkg/controllers/uiplugin/troubleshooting_panel.gopkg/controllers/uiplugin/troubleshooting_panel_test.go
✅ Files skipped from review due to trivial changes (1)
- pkg/controllers/uiplugin/controller.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/controllers/uiplugin/troubleshooting_panel_test.go
alanconway
force-pushed
the
korrel8r-tokenreview
branch
2 times, most recently
from
342d10b to
dcdd6b6
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alanconway, jan--f 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 |
alanconway
force-pushed
the
korrel8r-tokenreview
branch
from
dcdd6b6 to
ddd2e75
Compare
Korrel8r uses 'create tokenreview' to get user names for session tokens. Need usernames for reliable session keys. Same user can get different tokens per log-in.
alanconway
force-pushed
the
korrel8r-tokenreview
branch
from
ddd2e75 to
d5cab05
Compare
2 participants
Allow Korrel8r to create tokenreviews to associate user names with sessions.
In order to handle multiple users concurrently, korrel8r now has independent sessions per user.
To identify users (username, userID) from bearer tokens, it needs to use the tokenreview process.
The same user logged in multiple times can have different tokens, so comparing tokens is not sufficient.