Repository navigation
fix(backups): move velero-configmap Role to velero chart - #2459
Conversation
The backupstrategy-controller chart declared a Role/RoleBinding in the cozy-velero namespace for ResourceModifier ConfigMap management. Because velero is an optional package, that namespace does not exist in bundles without velero, so Helm install aborted with "namespaces \"cozy-velero\" not found" and blocked the default install of backupstrategy-controller. Move the Role and RoleBinding into the velero chart so they are created only when velero is actually installed. The RoleBinding subject points to the backupstrategy-controller ServiceAccount in its fixed namespace (cozy-backup-controller). Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
myasnikovdaniil
added
the
kind/backport
dosubot
Bot
added
the
size/XS
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a dependency issue in the Helm chart deployment process. By relocating namespace-specific RBAC resources to the optional Velero package, the backupstrategy-controller can now be successfully installed in environments where Velero is not present, preventing premature namespace validation failures. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughThe changes relocate RBAC configuration for the backupstrategy-controller's configmaps access from its own chart templates to the velero chart. A Role and RoleBinding granting configmaps management permissions are removed from the backupstrategy-controller chart and introduced as a new template in the velero chart, consolidating related RBAC definitions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request moves the RBAC configuration for the backupstrategy-controller from its own chart to the velero chart. This ensures that the Role and RoleBinding required for managing ConfigMaps in the Velero namespace are only created when Velero is actually installed, preventing errors related to non-existent namespaces. Feedback was provided regarding the hardcoded namespace cozy-backup-controller in the new template, suggesting the use of a global value for better flexibility.
| subjects: | ||
| - kind: ServiceAccount | ||
| name: backupstrategy-controller | ||
| namespace: cozy-backup-controller |
There was a problem hiding this comment.
The namespace cozy-backup-controller is hardcoded here. While this aligns with the current project structure described in the PR, it assumes the backupstrategy-controller is always installed in this specific namespace. If the project allows for custom namespace naming or if the namespace is derived from a different release name, this binding will break. Consider using a global value if one exists for the backup controller's namespace.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/system/velero/templates/backupstrategy-controller-rbac.yaml (1)
6-26: LGTM — RBAC correctly scoped to the velero install namespace.The
RoleandRoleBindingomitmetadata.namespace, so Helm will namespace them to the velero chart's release namespace (cozy-velero), which is wherecreateResourceModifiersConfigMap/cleanup paths create and delete ConfigMaps (seeinternal/backupcontroller/velerostrategy_controller.go:902-934andrestorejob_controller.go:156-193). TheRoleBindingsubject correctly points at the controller's ServiceAccount in its own namespace (cozy-backup-controller) — cross-namespace subjects are valid for aRoleBinding. Moving this into the velero chart is the right fix for the "namespaces "cozy-velero" not found" install failure when velero is disabled.Optional: consider setting
metadata.namespace: cozy-veleroexplicitly on both objects to make the intent obvious to readers and decouple from the chart's install-time release namespace.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/velero/templates/backupstrategy-controller-rbac.yaml` around lines 6 - 26, The Role and RoleBinding currently lack metadata.namespace so they rely on the Helm release namespace and caused "namespaces \"cozy-velero\" not found"; update both the Role (metadata.name: backups.cozystack.io:strategy-controller:velero-configmaps) and the RoleBinding (same name, subject ServiceAccount name: backupstrategy-controller, subject namespace: cozy-backup-controller) to include metadata.namespace: cozy-velero so the RBAC objects are explicitly created in the velero namespace where createResourceModifiersConfigMap / restorejob cleanup operate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/system/velero/templates/backupstrategy-controller-rbac.yaml`:
- Around line 6-26: The Role and RoleBinding currently lack metadata.namespace
so they rely on the Helm release namespace and caused "namespaces
\"cozy-velero\" not found"; update both the Role (metadata.name:
backups.cozystack.io:strategy-controller:velero-configmaps) and the RoleBinding
(same name, subject ServiceAccount name: backupstrategy-controller, subject
namespace: cozy-backup-controller) to include metadata.namespace: cozy-velero so
the RBAC objects are explicitly created in the velero namespace where
createResourceModifiersConfigMap / restorejob cleanup operate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 968e41d1-3366-4d27-94f4-b6758b055a37
📒 Files selected for processing (3)
packages/system/backupstrategy-controller/templates/rbac-bind.yamlpackages/system/backupstrategy-controller/templates/rbac.yamlpackages/system/velero/templates/backupstrategy-controller-rbac.yaml
💤 Files with no reviewable changes (1)
- packages/system/backupstrategy-controller/templates/rbac-bind.yaml
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-1.2
git worktree add -d .worktree/backport-2459-to-release-1.2 origin/release-1.2
cd .worktree/backport-2459-to-release-1.2
git switch --create backport-2459-to-release-1.2
git cherry-pick -x c4477259c7e914accaf45238a2b2b591451672a9 |
…lero chart (#2467) ## Summary Backport of #2459 to release-1.3. Fixes installation failure of `backupstrategy-controller` in bundles without velero. ## Changes - Move velero-configmap Role/RoleBinding from backupstrategy-controller chart to velero chart - Prevents "namespaces \"cozy-velero\" not found" error when velero is not installed ## Original Commit Cherry-pick of c447725 from main.
The AI-generated v1.3.1 changelog (#2480) was generated from git log v1.3.0..main rather than git log v1.3.0..v1.3.1, because the workflow checked out main while the v1.3.1 tag points to release-1.3. As a result the changelog included: - 8 PRs that were merged to main but never shipped in v1.3.1 - 6 backport PRs that were merged to release-1.3 *after* v1.3.1 was tagged - Both originals and their backports as separate duplicate entries - A 2024 PR (#435) that has nothing to do with this range - Generic "Documentation updates" placeholders for website entries - Title duplicated as both the bold label and the description (`* **fix(...): X**: fix(...): X (...)`) - The cozystack-ci bot listed as a human contributor The actual v1.3.1 release range (v1.3.0..v1.3.1) contains exactly one user-facing change: 41bcb0b [Backport release-1.3] fix(backups): move velero-configmap Role to velero chart (#2467) which is the backport of #2459 (myasnikovdaniil) shipped via #2467 (IvanHunters). This commit replaces the contents of docs/changelogs/v1.3.1.md with that one entry, the matching two-person contributors list, and the standard footer. The workflow + docs fixes that prevent this regression for future patch releases will land in a separate PR against main. Signed-off-by: Myasnikov Daniil <[email protected]>
The auto-generated changelog only listed #2459/#2467 (velero-configmap Role move). Five additional PRs were backported and merged into release-1.3 between then and the v1.3.1 tag (2026-04-28): - #2471/#2524 - fix(api): IDOR in TenantNamespace Get/Watch - #2496/#2505 - feat(linstor): linstor-csi v1.10.6 (Protocol-C dual-attach) - #2462/#2511 - fix(etcd): remove destructive post-upgrade hook - #2421/#2491 - fix(kamaji): memory limits + startup probe - #2498/#2518 - build(linstor): wire linstor-gui into root build target Update the release date to match the actual tag (2026-04-28), rewrite the intro paragraph, and add @kvaps to contributors. Signed-off-by: Myasnikov Daniil <[email protected]>
2 participants
What this PR does
The
backupstrategy-controllerchart declared aRoleandRoleBindingscoped to thecozy-veleronamespace (for managingResourceModifierConfigMaps consumed by Velero Restore). Becausecozystack.velerois an optional package, that namespace does not exist in bundles that do not enable velero — andbackupstrategy-controlleris a default package. Helm install aborted with:which blocked the entire
cozy-backup-controller/backupstrategy-controllerHelmRelease on any cluster where velero was not explicitly enabled (including the E2E environment).This PR moves that Role/RoleBinding into the velero chart (
packages/system/velero/templates/backupstrategy-controller-rbac.yaml), so the permission grant only exists when velero is actually installed — where it is useful. The RoleBinding subject points to the stablebackupstrategy-controllerServiceAccount incozy-backup-controller.Release note
Summary by CodeRabbit