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