You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(kubevirt): let tenant admins upload a VMDisk image with virtctl - #5114
A VMDisk created with source: upload waits in UploadReady for an image, but no tenant role can fill it. virtctl image-upload reads the disk PVC and CDI's PVC Prime and requests an upload token, and the tenant roles grant neither get on persistentvolumeclaims nor create on upload.cdi.kubevirt.io/uploadtokenrequests, so the command stops on a forbidden PVC read.
cozy:tenant:admin:base now grants both. Tenant admins and super-admins get them in their own namespaces through the existing RoleBindings, the use and view tiers do not. Reading the DataVolume is already granted for the disk's own name by the vm-disk chart's dashboard role, so no DataVolume rule is added. The vm-disk notes print the command with --no-create, because the chart has already created the DataVolume, and the security self-assessment and threat model list the new grants.
This is the RBAC part of #3847, split out so tenants can upload while the console upload panel and the upload proxy certificate stay in review there. The rules, their tests, the notes change and the docs lines are from myasnikovdaniil, who is co-author on the commit.
Checked live on a cluster with KubeVirt v1.8.4 and CDI v1.66.1, with virtctl v1.8.4 impersonating a user in the <tenant>-admin group.
With the stock roles virtctl image-upload dv vm-disk-<name> --no-create fails with persistentvolumeclaims "vm-disk-<name>" is forbidden.
With exactly these two rules added as a temporary Role, the same command reads prime-<uid>, uploads a 16 MiB image and the DataVolume reaches Succeeded. get on the DataVolume came from the vm-disk dashboard role.
Unit tests with helm unittest 1.1.2, the version CI pins. packages/system/cozystack-basics passes 72 of 72, against 66 on main, and packages/apps/vm-disk 117 of 117, against 116. Removing the upload token rule fails one case, and dropping --no-create from the notes fails one case.
Walked the trigger map in docs/agents/contributing.md against the diff. Two tenant RBAC rules, their tests, the vm-disk notes and two docs files. No values key, schema, default, README or package change, so no trigger map entry applies.
No downstream repository is affected by this change
fix(kubevirt): tenant admins can upload an image into a VMDisk created with source upload using virtctl image-upload. The tenant admin and super-admin roles get get on persistentvolumeclaims and create on upload.cdi.kubevirt.io/uploadtokenrequests, and the vm-disk notes print the command with --no-create.
Summary by CodeRabbit
New Features
Tenant administrators can now upload images into existing VM disks.
Improvements
The displayed upload command now explicitly targets the release namespace and avoids creating a new disk.
Documentation
Security guidance now clarifies the access needed for uploads and notes that persistent-volume permissions remain limited by namespace scope.
A VMDisk created with source upload waits in UploadReady for an image,
but no tenant role can fill it. virtctl image-upload reads the disk PVC
and CDI's PVC Prime and requests an upload token, and the tenant roles
grant neither get on persistentvolumeclaims nor create on
uploadtokenrequests, so the command stops on a forbidden PVC read.
cozy:tenant:admin:base now grants both, so tenant admins and
super-admins get them in their own namespaces through the existing
RoleBindings, while the use and view tiers do not. Reading the
DataVolume is already granted for the disk's own name by the vm-disk
chart's dashboard role. The vm-disk notes print the command with
--no-create, because the chart has already created the DataVolume, and
the security self-assessment and threat model list the new grants.
The rules, their tests, the notes change and the docs lines come from
cozystack#3847, which also carries a console upload panel and an upload proxy
certificate that are still in review there.
Fixescozystack#3759
Co-authored-by: Myasnikov Daniil <[email protected]>
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <[email protected]>
Reviewing files that changed from the base of the PR and between bcc5f32 and 95364b8.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Walkthrough
The tenant admin base role gains permissions for VM disk image uploads. The upload notes use --no-create and --namespace. Security documentation and Helm tests describe and check these changes.
The tenant admin base role gains PVC get and CDI uploadtokenrequestscreate permissions. Tests check role aggregation and confirm these permissions are absent from tenant-use and tenant-view roles. Security descriptions document the grants and the namespace-scoped persistent-volume limitation.
VM disk upload command packages/apps/vm-disk/templates/NOTES.txt, packages/apps/vm-disk/tests/upload_notes_test.yaml
The upload command uses --no-create and --namespace. A Helm test checks the command, namespace, and image-path placeholder.
No actionable merge blocker is identified for tenant-admin uploads into existing VM disks; normal validation remains appropriate.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly summarizes the main change: enabling tenant administrators to upload VMDisk images with virtctl.
Linked Issues check
For #3759, cozy:tenant:admin:base adds get on persistentvolumeclaims and create on upload.cdi.kubevirt.io/uploadtokenrequests. The RBAC tests verify admin and super-admin aggregation and exc…
Out of Scope Changes check
The changes stay within #3759. The RBAC tests validate permission boundaries, the vm-disk notes and test support the upload workflow, and the security documents record the new grants. No unrelated pro…
Docstring Coverage
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Full details: Linked Issues check
Explanation
For #3759, cozy:tenant:admin:base adds get on persistentvolumeclaims and create on upload.cdi.kubevirt.io/uploadtokenrequests. The RBAC tests verify admin and super-admin aggregation and exclude the use and view tiers. The existing vm-disk dashboard role supplies DataVolume access. The notes use --no-create for the existing DataVolume. The reported live test completed a 16 MiB upload through CDI and reached Succeeded.
Full details: Out of Scope Changes check
Explanation
The changes stay within #3759. The RBAC tests validate permission boundaries, the vm-disk notes and test support the upload workflow, and the security documents record the new grants. No unrelated product behavior or downstream package change is introduced.
Full details: Docstring Coverage
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (6 skipped: 6 unsupported.)
Finishing Touches
🧪 Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
area/virtualizationIssues or PRs related to virtualization (kubevirt, cdi, vmi, vm-import)kind/bugCategorizes issue or PR as related to a bugsize/LThis PR changes 100-499 lines, ignoring generated files
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
area/virtualization
Aleksei Sviridkin (lexfrei)
left a comment
What this PR does
A VMDisk created with
source: uploadwaits inUploadReadyfor an image, but no tenant role can fill it.virtctl image-uploadreads the disk PVC and CDI's PVC Prime and requests an upload token, and the tenant roles grant neithergeton persistentvolumeclaims norcreateonupload.cdi.kubevirt.io/uploadtokenrequests, so the command stops on a forbidden PVC read.cozy:tenant:admin:basenow grants both. Tenant admins and super-admins get them in their own namespaces through the existing RoleBindings, the use and view tiers do not. Reading the DataVolume is already granted for the disk's own name by the vm-disk chart's dashboard role, so no DataVolume rule is added. The vm-disk notes print the command with--no-create, because the chart has already created the DataVolume, and the security self-assessment and threat model list the new grants.This is the RBAC part of #3847, split out so tenants can upload while the console upload panel and the upload proxy certificate stay in review there. The rules, their tests, the notes change and the docs lines are from myasnikovdaniil, who is co-author on the commit.
Checked live on a cluster with KubeVirt v1.8.4 and CDI v1.66.1, with virtctl v1.8.4 impersonating a user in the
<tenant>-admingroup.virtctl image-upload dv vm-disk-<name> --no-createfails withpersistentvolumeclaims "vm-disk-<name>" is forbidden.prime-<uid>, uploads a 16 MiB image and the DataVolume reachesSucceeded.geton the DataVolume came from the vm-disk dashboard role.cdi-uploadproxywith--insecure. A trusted certificate on the published proxy is the other half of fix(dashboard,kubevirt): let tenant admins upload VM disk images with virtctl #3847 and is not changed here.Unit tests with
helm unittest1.1.2, the version CI pins.packages/system/cozystack-basicspasses 72 of 72, against 66 on main, andpackages/apps/vm-disk117 of 117, against 116. Removing the upload token rule fails one case, and dropping--no-createfrom the notes fails one case.Fixes #3759
Screenshots
Not a UI change.
Downstream repositories
Walked the trigger map in
docs/agents/contributing.mdagainst the diff. Two tenant RBAC rules, their tests, the vm-disk notes and two docs files. No values key, schema, default, README or package change, so no trigger map entry applies.Release note
Summary by CodeRabbit