Skip to content

update helm for zxporter rbac#172

Merged
Parthiba-Hazra merged 1 commit into
mainfrom
ph/helm-update
Oct 14, 2025
Merged

update helm for zxporter rbac#172
Parthiba-Hazra merged 1 commit into
mainfrom
ph/helm-update

Conversation

@Parthiba-Hazra
Copy link
Copy Markdown
Collaborator

@Parthiba-Hazra Parthiba-Hazra commented Oct 14, 2025

Summary by CodeRabbit

  • New Features
    • Enabled metrics endpoint access for observability tools, including authentication and authorization checks for metrics scraping.
  • Chores
    • Expanded and reorganized RBAC permissions to cover additional Kubernetes resources and APIs, improving compatibility with common controllers and integrations.
    • Updated role bindings to align with the new permissions and metrics access.
    • Retained leadership election and manager role bindings with revised associations to support the updated roles.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 14, 2025

Walkthrough

Expanded ClusterRole for the manager with broader, adjusted permissions across many apiGroups. Introduced metrics-specific roles (auth and reader) and a corresponding ClusterRoleBinding to the controller ServiceAccount. Retained and aligned leader-election and manager role bindings within the same RBAC template.

Changes

Cohort / File(s) Summary of Changes
Manager ClusterRole expansion
helm-chart/zxporter/templates/zxporter-rbac.yaml
Reworked devzero-zxporter-manager-role with a revised, expanded ruleset across core and multiple apiGroups; updated resource/verb combinations.
Metrics RBAC additions
helm-chart/zxporter/templates/zxporter-rbac.yaml
Added devzero-zxporter-metrics-auth-role (create on authentication.k8s.io/tokenreviews, authorization.k8s.io/subjectaccessreviews) and devzero-zxporter-metrics-reader (get on nonResourceURL /metrics).
RoleBindings updates
helm-chart/zxporter/templates/zxporter-rbac.yaml
Added devzero-zxporter-metrics-auth-rolebinding binding metrics auth role to devzero-zxporter-controller-manager ServiceAccount; retained/updated manager and leader-election bindings accordingly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Prom as Prometheus
  participant KAPI as Kubernetes API Server
  participant AuthZ as authorization.k8s.io
  participant AuthN as authentication.k8s.io
  participant Ctrl as zxporter Controller

  Note over Prom,KAPI: Metrics scrape flow
  Prom->>KAPI: GET /apis/.../subjectaccessreviews (create)
  KAPI->>AuthZ: SubjectAccessReview for GET /metrics
  AuthZ-->>KAPI: Decision (allow/deny)
  KAPI-->>Prom: Allowed to access /metrics

  Prom->>Ctrl: GET /metrics
  Ctrl-->>Prom: Metrics payload

  Note over Ctrl,KAPI: Controller RBAC usage (read/watch)
  Ctrl->>KAPI: list/watch resources across apiGroups
  KAPI-->>Ctrl: Resource events/objects
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Poem

I thump my paws on YAML fields,
New rules bloom like clover shields.
Metrics doors now open wide,
Token tales and access guide.
I hop through clusters, ears alert—
Watch, list, patch—no grassy dirt.
Carrots logged, permissions pert.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly indicates that the Helm chart is being updated to modify ZXporter’s RBAC configuration, which aligns with the changes in the rbac.yaml template where roles and bindings were adjusted and new metrics roles were added.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ph/helm-update

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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f03a096 and d344c60.

📒 Files selected for processing (1)
  • helm-chart/zxporter/templates/zxporter-rbac.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Docker Image
  • GitHub Check: Build Docker Image
  • GitHub Check: Analyze (go)

Comment on lines +127 to +160
- apiGroups:
- ""
resources:
- configmaps
- endpoints
- events
- limitranges
- namespaces
- nodes
- persistentvolumeclaims
- persistentvolumes
- pods
- replicationcontrollers
- resourcequotas
- serviceaccounts
- services
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
- nodes/metrics
- nodes/status
- pods/status
verbs:
- get
- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- get
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Restore event create permissions for the manager.

Placing events in the read-only core rule strips the manager of create/patch on events, so the controller can no longer emit Kubernetes Events (you’ll get Forbidden errors from the event recorder). Please split out events and add back the write verbs:

-  - configmaps
-  - endpoints
-  - events
+  - configmaps
+  - endpoints
   - limitranges
   ...
-  - watch
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
+  - get
+  - list
+  - watch
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- apiGroups:
- ""
resources:
- configmaps
- endpoints
- events
- limitranges
- namespaces
- nodes
- persistentvolumeclaims
- persistentvolumes
- pods
- replicationcontrollers
- resourcequotas
- serviceaccounts
- services
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
- nodes/metrics
- nodes/status
- pods/status
verbs:
- get
- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- get
- apiGroups:
- ""
resources:
- configmaps
- endpoints
- limitranges
- namespaces
- nodes
- persistentvolumeclaims
- persistentvolumes
- pods
- replicationcontrollers
- resourcequotas
- serviceaccounts
- services
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
- events
verbs:
- create
- patch
- get
- list
- watch
- apiGroups:
- ""
resources:
- nodes/metrics
- nodes/status
- pods/status
verbs:
- get
- apiGroups:
- apiextensions.k8s.io
resources:
- customresourcedefinitions
verbs:
- get
🤖 Prompt for AI Agents
In helm-chart/zxporter/templates/zxporter-rbac.yaml around lines 127 to 160, the
current rule lists "events" under a read-only core rule which removes the
manager's ability to emit events; remove "events" from that read-only resources
list and add a separate RBAC rule for apiGroups: [""] with resources: ["events"]
and verbs: ["create","patch","update"] so the controller can create and update
Kubernetes Events (ensure the new rule is placed alongside the other core
rules).

@Parthiba-Hazra Parthiba-Hazra enabled auto-merge (squash) October 14, 2025 19:15
@Parthiba-Hazra Parthiba-Hazra merged commit 1d62b87 into main Oct 14, 2025
41 of 42 checks passed
@Parthiba-Hazra Parthiba-Hazra deleted the ph/helm-update branch October 14, 2025 19:23
@coderabbitai coderabbitai Bot mentioned this pull request Dec 1, 2025
Parthiba-Hazra added a commit that referenced this pull request May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants