|
| 1 | +From 65d9983cddb566e34b5f910c47cf7b230f7be412 Mon Sep 17 00:00:00 2001 |
| 2 | +From: AllSpark < [email protected]> |
| 3 | +Date: Mon, 24 Nov 2025 06:07:26 +0000 |
| 4 | +Subject: [PATCH] host-disk: only chown when file is created; make Create use |
| 5 | + pointer receiver; add failure ownership manager for tests; adjust storage |
| 6 | + tests for non-qemu ownership |
| 7 | + |
| 8 | +Signed-off-by: Azure Linux Security Servicing Account < [email protected]> |
| 9 | +Upstream-reference: AI Backport of https://github.com/kubevirt/kubevirt/pull/15037.diff |
| 10 | +--- |
| 11 | + pkg/ephemeral-disk-utils/utils.go | 18 ++++++++++++++++-- |
| 12 | + pkg/host-disk/host-disk.go | 14 +++++++------- |
| 13 | + pkg/host-disk/host-disk_test.go | 11 ++++++++++- |
| 14 | + tests/storage/storage.go | 19 +++++++++++++++---- |
| 15 | + 4 files changed, 48 insertions(+), 14 deletions(-) |
| 16 | + |
| 17 | +diff --git a/pkg/ephemeral-disk-utils/utils.go b/pkg/ephemeral-disk-utils/utils.go |
| 18 | +index fc1a07b..3a2f493 100644 |
| 19 | +--- a/pkg/ephemeral-disk-utils/utils.go |
| 20 | ++++ b/pkg/ephemeral-disk-utils/utils.go |
| 21 | +@@ -44,14 +44,28 @@ func MockDefaultOwnershipManager() { |
| 22 | + type nonOpManager struct { |
| 23 | + } |
| 24 | + |
| 25 | +-func (no *nonOpManager) UnsafeSetFileOwnership(file string) error { |
| 26 | ++func (no *nonOpManager) UnsafeSetFileOwnership(_ string) error { |
| 27 | + return nil |
| 28 | + } |
| 29 | + |
| 30 | +-func (no *nonOpManager) SetFileOwnership(file *safepath.Path) error { |
| 31 | ++func (no *nonOpManager) SetFileOwnership(_ *safepath.Path) error { |
| 32 | + return nil |
| 33 | + } |
| 34 | + |
| 35 | ++func MockDefaultOwnershipManagerWithFailure() { |
| 36 | ++ DefaultOwnershipManager = &failureManager{} |
| 37 | ++} |
| 38 | ++ |
| 39 | ++type failureManager struct{} |
| 40 | ++ |
| 41 | ++func (no *failureManager) UnsafeSetFileOwnership(_ string) error { |
| 42 | ++ panic("unexpected call to UnsafeSetFileOwnership") |
| 43 | ++} |
| 44 | ++ |
| 45 | ++func (no *failureManager) SetFileOwnership(_ *safepath.Path) error { |
| 46 | ++ panic("unexpected call to SetFileOwnership") |
| 47 | ++} |
| 48 | ++ |
| 49 | + type OwnershipManager struct { |
| 50 | + user string |
| 51 | + } |
| 52 | +diff --git a/pkg/host-disk/host-disk.go b/pkg/host-disk/host-disk.go |
| 53 | +index 0d13301..02daaa1 100644 |
| 54 | +--- a/pkg/host-disk/host-disk.go |
| 55 | ++++ b/pkg/host-disk/host-disk.go |
| 56 | +@@ -226,7 +226,7 @@ func (hdc *DiskImgCreator) setlessPVCSpaceToleration(toleration int) { |
| 57 | + hdc.lessPVCSpaceToleration = toleration |
| 58 | + } |
| 59 | + |
| 60 | +-func (hdc DiskImgCreator) Create(vmi *v1.VirtualMachineInstance) error { |
| 61 | ++func (hdc *DiskImgCreator) Create(vmi *v1.VirtualMachineInstance) error { |
| 62 | + for _, volume := range vmi.Spec.Volumes { |
| 63 | + if hostDisk := volume.VolumeSource.HostDisk; shouldMountHostDisk(hostDisk) { |
| 64 | + if err := hdc.mountHostDiskAndSetOwnership(vmi, volume.Name, hostDisk); err != nil { |
| 65 | +@@ -249,14 +249,14 @@ func (hdc *DiskImgCreator) mountHostDiskAndSetOwnership(vmi *v1.VirtualMachineIn |
| 66 | + return err |
| 67 | + } |
| 68 | + if !fileExists { |
| 69 | +- if err := hdc.handleRequestedSizeAndCreateSparseRaw(vmi, diskDir, diskPath, hostDisk); err != nil { |
| 70 | ++ if err = hdc.handleRequestedSizeAndCreateSparseRaw(vmi, diskDir, diskPath, hostDisk); err != nil { |
| 71 | ++ return err |
| 72 | ++ } |
| 73 | ++ // Change file ownership to the qemu user. |
| 74 | ++ if err = ephemeraldiskutils.DefaultOwnershipManager.UnsafeSetFileOwnership(diskPath); err != nil { |
| 75 | ++ log.Log.Reason(err).Errorf("Couldn't set Ownership on %s: %v", diskPath, err) |
| 76 | + return err |
| 77 | + } |
| 78 | +- } |
| 79 | +- // Change file ownership to the qemu user. |
| 80 | +- if err := ephemeraldiskutils.DefaultOwnershipManager.UnsafeSetFileOwnership(diskPath); err != nil { |
| 81 | +- log.Log.Reason(err).Errorf("Couldn't set Ownership on %s: %v", diskPath, err) |
| 82 | +- return err |
| 83 | + } |
| 84 | + return nil |
| 85 | + } |
| 86 | +diff --git a/pkg/host-disk/host-disk_test.go b/pkg/host-disk/host-disk_test.go |
| 87 | +index 2b04fa5..fb44104 100644 |
| 88 | +--- a/pkg/host-disk/host-disk_test.go |
| 89 | ++++ b/pkg/host-disk/host-disk_test.go |
| 90 | +@@ -40,6 +40,8 @@ import ( |
| 91 | + v1 "kubevirt.io/api/core/v1" |
| 92 | + "kubevirt.io/client-go/kubecli" |
| 93 | + |
| 94 | ++ ephemeraldiskutils "kubevirt.io/kubevirt/pkg/ephemeral-disk-utils" |
| 95 | ++ |
| 96 | + libvmistatus "kubevirt.io/kubevirt/pkg/libvmi/status" |
| 97 | + |
| 98 | + "kubevirt.io/kubevirt/pkg/testutils" |
| 99 | +@@ -289,7 +291,14 @@ var _ = Describe("HostDisk", func() { |
| 100 | + }) |
| 101 | + }) |
| 102 | + Context("With existing disk.img", func() { |
| 103 | +- It("Should not re-create disk.img", func() { |
| 104 | ++ AfterEach(func() { |
| 105 | ++ By("Switching back to the regular mock ownership manager") |
| 106 | ++ ephemeraldiskutils.MockDefaultOwnershipManager() |
| 107 | ++ }) |
| 108 | ++ |
| 109 | ++ It("Should not re-create or chown disk.img", func() { |
| 110 | ++ By("Switching to an ownership manager that panics when called") |
| 111 | ++ ephemeraldiskutils.MockDefaultOwnershipManagerWithFailure() |
| 112 | + By("Creating a disk.img before adding a HostDisk volume") |
| 113 | + tmpDiskImg := createTempDiskImg("volume1") |
| 114 | + By("Creating a new VMI with a HostDisk volumes") |
| 115 | +diff --git a/tests/storage/storage.go b/tests/storage/storage.go |
| 116 | +index b28efdd..1646dde 100644 |
| 117 | +--- a/tests/storage/storage.go |
| 118 | ++++ b/tests/storage/storage.go |
| 119 | +@@ -254,14 +254,25 @@ var _ = SIGDescribe("Storage", func() { |
| 120 | + // Start the VirtualMachineInstance with the PVC attached |
| 121 | + vmi = newVMI(pvcName) |
| 122 | + |
| 123 | +- vmi = libvmops.RunVMIAndExpectLaunch(vmi, 180) |
| 124 | ++ if imageOwnedByQEMU { |
| 125 | ++ vmi = libvmops.RunVMIAndExpectLaunch(vmi, 180) |
| 126 | + |
| 127 | +- By(checkingVMInstanceConsoleOut) |
| 128 | +- Expect(console.LoginToAlpine(vmi)).To(Succeed()) |
| 129 | ++ By(checkingVMInstanceConsoleOut) |
| 130 | ++ Expect(console.LoginToAlpine(vmi)).To(Succeed()) |
| 131 | ++ } else { |
| 132 | ++ By("Starting a VirtualMachineInstance") |
| 133 | ++ createdVMI := libvmops.RunVMIAndExpectScheduling(vmi, 60) |
| 134 | ++ |
| 135 | ++ By(fmt.Sprintf("Checking that VirtualMachineInstance start failed: starting at %v", time.Now())) |
| 136 | ++ ctx, cancel := context.WithCancel(context.Background()) |
| 137 | ++ defer cancel() |
| 138 | ++ event := watcher.New(createdVMI).Timeout(60*time.Second).SinceWatchedObjectResourceVersion().WaitFor(ctx, watcher.WarningEvent, "SyncFailed") |
| 139 | ++ Expect(event.Message).To(ContainSubstring("Could not open '/var/run/kubevirt-private/vmi-disks/disk0/disk.img': Permission denied"), "VMI should not be started") |
| 140 | ++ } |
| 141 | + }, |
| 142 | + Entry("[test_id:3130]with Disk PVC", newRandomVMIWithPVC, true), |
| 143 | + Entry("[test_id:3131]with CDRom PVC", newRandomVMIWithCDRom, true), |
| 144 | +- Entry("hostpath disk image file not owned by qemu", newRandomVMIWithPVC, false), |
| 145 | ++ Entry("unless hostpath disk image file not owned by qemu", newRandomVMIWithPVC, false), |
| 146 | + ) |
| 147 | + }) |
| 148 | + |
| 149 | +-- |
| 150 | +2.45.4 |
| 151 | + |
0 commit comments