Skip to content

Conversation

@RoiGlinik
Copy link
Contributor

No description provided.

Signed-off-by: Roi Glinik <groi.tech@gmail.com>
Signed-off-by: Roi Glinik <groi.tech@gmail.com>
Signed-off-by: Roi Glinik <groi.tech@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Walkthrough

Adds configurable per-tool CRD read-only permissions to the runner via Helm values (including a new externalSecrets toggle), updates the ClusterRole template to conditionally include those RBAC rules, introduces a top-level scaleAlertsProcessing value, and documents the default CRD permissions.

Changes

Cohort / File(s) Summary
Helm values
helm/robusta/values.yaml
Added top-level boolean scaleAlertsProcessing (default: false) and new runner.crdPermissions mapping with booleans for argo, flux, kafka, keda, crossplane, istio, gatewayApi, velero, and externalSecrets (all true by default).
RBAC template
helm/robusta/templates/runner-service-account.yaml
Added conditional ClusterRole rule blocks for per-tool CRD permissions guarded by .Values.runner.crdPermissions.*, including a new block for externalSecrets granting read-only verbs on externalsecrets/secretstores/clustersecretstores/clusterexternalsecrets.
Documentation
docs/setup-robusta/crds.rst
Added "Default CRD Permissions" section documenting the default read-only CRD permissions and showing the YAML snippet for enabling/disabling each tool's CRD access.

Sequence Diagram(s)

(omitted — changes are configuration/docs and conditional RBAC additions without multi-component control-flow that warrants a sequence diagram)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • arikalon1
  • moshemorad

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a description explaining the purpose of the CRD permissions changes, which CRDs are affected, and why these permissions are being enabled by default.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'ROB-2714 more crds default access' directly refers to the main change in the PR: adding more default CRD permissions (externalSecrets) to the runner service account and documenting these permissions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ROB-2714-more-crds-default-access

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5403f2 and 77f9934.

📒 Files selected for processing (3)
  • docs/setup-robusta/crds.rst
  • helm/robusta/templates/runner-service-account.yaml
  • helm/robusta/values.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
  • docs/setup-robusta/crds.rst
  • helm/robusta/values.yaml
  • helm/robusta/templates/runner-service-account.yaml
⏰ 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: run_tests
  • GitHub Check: run_tests
  • GitHub Check: Deploy docs

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

@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: 0

🧹 Nitpick comments (3)
helm/robusta/values.yaml (1)

115-117: Consider using lowercase YAML boolean convention.

The value False uses Python-style capitalization. YAML convention typically uses lowercase false for boolean values, though both are valid.

🔎 Proposed fix
-scaleAlertsProcessing: False
+scaleAlertsProcessing: false
docs/setup-robusta/crds.rst (1)

86-103: LGTM! Clear documentation of the new feature.

The new "Default CRD Permissions" section clearly explains the opt-out model and provides a helpful code example with inline comments describing each tool.

Consider adding a brief note about the relationship between runner.crdPermissions.* and the existing runner.customCRD / argoRollouts configurations, since they provide overlapping functionality for some resources. This would help users understand when to use each approach.

Example addition
.. note::
    The ``crdPermissions`` flags provide read-only access to common operators. 
    For write access or custom CRDs not covered by these defaults, use 
    ``customClusterRoleRules`` as shown in the Basic Configuration section above.
helm/robusta/templates/runner-service-account.yaml (1)

402-428: Document the configuration strategy for overlapping StrimziPodSet permissions.

Lines 298-308 grant write-level permissions to strimzipodsets (under core.strimzi.io) when "StrimziPodSet" is in .Values.runner.customCRD (with patch and update verbs). Lines 402-428 grant read-only access to the same resource via .Values.runner.crdPermissions.kafka. Both configuration methods target the same resource but with different permission levels, and both can be independently enabled. This creates potential confusion about which configuration method to use and what the expected permission scope should be. Consider documenting the intended relationship and migration path between these two configuration approaches, particularly for overlapping Strimzi resources.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a91ecd5 and f5403f2.

📒 Files selected for processing (3)
  • docs/setup-robusta/crds.rst
  • helm/robusta/templates/runner-service-account.yaml
  • helm/robusta/values.yaml
⏰ 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). (1)
  • GitHub Check: run_tests
🔇 Additional comments (8)
helm/robusta/values.yaml (1)

742-751: LGTM! Well-structured CRD permissions configuration.

The new crdPermissions section provides a clean, granular way to control read-only access to common operators. Defaulting to true offers good out-of-the-box visibility while allowing users to disable specific tools as needed.

helm/robusta/templates/runner-service-account.yaml (7)

351-401: LGTM! Comprehensive Flux CD permissions.

The Flux CD permissions cover all major Flux toolkit API groups with appropriate read-only access. This provides full visibility into GitOps resources without granting modification rights.


429-442: LGTM! KEDA permissions are appropriate.

The KEDA autoscaler permissions cover all essential resources with read-only access, providing visibility into scaling configurations without modification rights.


443-465: LGTM! Crossplane permissions are well-structured.

The Crossplane permissions appropriately cover both package management and composition resources with read-only access, enabling visibility into infrastructure provisioning without granting modification rights.


466-493: LGTM! Comprehensive Istio service mesh permissions.

The Istio permissions cover both networking and security resources with appropriate read-only access, providing full visibility into service mesh configuration.


494-511: LGTM! Gateway API permissions are complete.

The Gateway API permissions cover all standard gateway and route resources with read-only access, providing visibility into ingress configuration using the Gateway API standard.


512-531: LGTM! Velero backup permissions are comprehensive.

The Velero permissions cover all essential backup, restore, and schedule resources with read-only access, enabling visibility into disaster recovery operations without granting modification rights.


331-350: Review the overlap between .Values.argoRollouts and .Values.runner.crdPermissions.argo.

The argoRollouts flag (line 287) grants write permissions (patch, update) to rollouts and also sets an environment variable to enable Argo Rollouts-specific logic in the runner. The new crdPermissions.argo block (line 331) grants read-only access to a broader set of Argo resources, including rollouts.

If both flags are enabled, the service account gains both read and write permissions for rollouts, which is redundant. More importantly, users may be confused about which flag controls what: argoRollouts appears to be a legacy flag with no documentation in values.yaml, while crdPermissions.argo is part of a modern, organized structure. Consider clarifying the intended relationship between these two flags or documenting when each should be used.

Signed-off-by: Roi Glinik <groi.tech@gmail.com>
@arikalon1 arikalon1 merged commit a0b3cfc into master Dec 25, 2025
8 checks passed
@arikalon1 arikalon1 deleted the ROB-2714-more-crds-default-access branch December 25, 2025 16:30
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.

3 participants