Repository navigation
fix(dashboard): grant tenant dashboard read on cozy-public PVCs - #2843
Conversation
The vm-disk source image dropdown in the console lists PersistentVolumeClaims in cozy-public (the vm-default-images-* golden images). The browser issues that list as the tenant identity (e.g. system:serviceaccount:tenant-root:tenant-root), but the cozy:tenant:dashboard Role only granted read on Flux helmrepositories and helmcharts. The list returned 403 and the dropdown stayed empty even when the images existed. Add get/list/watch on persistentvolumeclaims to the Role. cozy-public is a shared public namespace, so read on its PVCs is safe to expose to every tenant dashboard subject. A helm-unittest regression pins the grant. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
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 an issue where tenant dashboards were unable to populate the VM disk source image dropdown due to insufficient RBAC permissions. By granting read access to PersistentVolumeClaims in the shared Highlights
New Features🧠 You can now enable Memory (public preview) 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 the 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 counterproductive. 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
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe dashboard Role template ( ChangesDashboard Role PVC Access
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the tenant dashboard Role in cozystack-basics to grant read access (get, list, watch) to persistentvolumeclaims in the cozy-public namespace, enabling the console to list golden images for VM disk creation. It also adds a unit test to verify these permissions. The review feedback suggests adhering to the principle of least privilege by removing the watch verb if real-time updates are not required, and updating the unit tests accordingly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| - apiGroups: [""] | ||
| resources: ["persistentvolumeclaims"] | ||
| verbs: ["get", "list", "watch"] |
There was a problem hiding this comment.
To adhere to the principle of least privilege and minimize API server load, consider removing the watch verb if the tenant dashboard only needs to list the PVCs once to populate the dropdown. If the UI does not require real-time updates via a watch stream, get and list are sufficient.
- apiGroups: [""]
resources: ["persistentvolumeclaims"]
verbs: ["get", "list"]| apiGroups: [""] | ||
| resources: ["persistentvolumeclaims"] | ||
| verbs: ["get", "list", "watch"] |
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — minimal, correctly-scoped RBAC fix: adds get/list/watch on persistentvolumeclaims to the namespaced cozy:tenant:dashboard Role in cozy-public, with a helm-unittest regression pinning the grant. No console code changes.
Business context: the VM-disk source-image dropdown lists the vm-default-images-* golden-image PVCs in cozy-public as the tenant identity, but the tenant dashboard Role only granted read on Flux helm resources, so the list returned 403 and the dropdown was empty for every real tenant (cluster-admin bypasses tenant RBAC, which made it look intermittent).
Verified: the grant is a namespaced Role in cozy-public (not a ClusterRole), so the read is confined to the shared public images namespace; the added rule matches the resource the console lists (persistentvolumeclaims); the new test asserts the rule on the cozy:tenant:dashboard Role in cozy-public.
Non-blocking follow-up
watchmay exceed what the dropdown needs — a one-shotlistpopulates it unless the console runs an informer over PVCs. If it does not watch, narrowing the grant toget/list(and updating the test in step) would be closer to least privilege; harmless either way on a shared public namespace.
|
Successfully created backport PR for |
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The VM image picker no longer offers golden images whose import failed. A golden keeps its PVC in `cozy-public` after the import fails, so a picker built on the PVC list offered an image whose clone never completes. The `image` option source in cozystack-api now reads the `vm-default-images-*` DataVolumes next to the PVCs and drops names whose DataVolume is in phase `Failed`. I filter rather than mark them, because such an option can never be used and `OptionItem` has no disabled state. A golden that is still importing stays in the list, since CDI holds a clone until the source is populated, and so does a PVC with no DataVolume. When the DataVolumes cannot be read, the PVCs go out unfiltered. The Option list drops a source whose provider errors, and an empty picker would be worse than the old behaviour. cozystack-api gets a read grant on `cdi.kubevirt.io` DataVolumes for this. The console did not use that source at all. `SourceField` listed the PVCs itself, as the tenant, so the fix above would not reach the picker users see. It now reads the same Option list the other dynamic dropdowns use. With that, nothing in the console lists `cozy-public` PVCs, so I removed the read grant on them from the `cozy:tenant:dashboard` Role. That is a visible change: a tenant loses `kubectl get pvc --namespace cozy-public`, which was only ever granted for this picker in #2843. One limitation: in CDI v1.66.1, which cozystack ships, a golden import reaches `Failed` only on a checksum mismatch, and a broken URL keeps it in `ImportInProgress`. So this filter covers less than the issue assumed. I filed #4564 for that. Closes #4553 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> I have no screenshot. The picker looks the same; only the list behind it changes. `apps/console/src/components/SourceField.test.tsx` covers it: the picker shows the images the option source serves and not the raw PVC names. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(api): the VM image picker no longer offers golden images whose import failed. The console now reads images from the image option source, and tenants no longer get read access to PersistentVolumeClaims in cozy-public. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * The image picker now displays available images from the tenant’s image options. * **Bug Fixes** * Images whose imports have failed are excluded from the picker. Images remain available when import status is still in progress, has succeeded, or cannot be retrieved. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
The vm-disk source image dropdown in the console lists
PersistentVolumeClaimsincozy-public(thevm-default-images-*golden images) to offer them when creating a VM disk. That list is issued by the browser as the tenant identity (e.g.system:serviceaccount:tenant-root:tenant-root), but thecozy:tenant:dashboardRole incozy-publiconly granted read on Fluxhelmrepositories/helmcharts. The list returned403 Forbiddenand the dropdown stayed empty even when images had been enabled (e.g. viabundles.enabledPackages).This is identity-dependent, which is why it can look intermittent: a cluster-admin login bypasses tenant RBAC and sees the images, while any real tenant — including
tenant-root-super-admin— hits the 403.This PR adds
get/list/watchonpersistentvolumeclaimsto the Role.cozy-publicis a shared public namespace, so read on its PVCs is safe to expose to every tenant dashboard subject (the Role is already bound to the tenant groups and the tenant ServiceAccount). A helm-unittest regression pins the grant.Verified with
kubectl auth can-i list pvc -n cozy-public:--as=system:serviceaccount:tenant-root:tenant-root→ no (before this PR)--as-group=tenant-root-super-admin→ no (before this PR)No console code changed — this is an RBAC-only fix.
Release note
Summary by CodeRabbit
New Features
Tests