Don't crash when parsing vm yaml from virt-v2v

Make sure to test pointer types for nil before using them. Because yaml unmarshalling tries to parse as much as it can and ignores extra fields in both the struct and the data stream, we can end up with a struct that successfully parses the input stream but is only partially initialized. Because we're parsing this data from another source, we need to be defensive and ensure that the data is initialized before using it. Fixes: https://issues.redhat.com/browse/MTV-1577 Signed-off-by: Jonathon Jongsma <[email protected]>
mnecas · Oct 17, 2024 · d8eee49 · d8eee49
1 parent 31037e9
commit d8eee49
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 5 deletions.
diff --git a/pkg/controller/plan/util/kubevirtvmparser.go b/pkg/controller/plan/util/kubevirtvmparser.go
@@ -66,11 +66,17 @@ func GetFirmwareFromYaml(yamlData []byte) (string, error) {
 	if err := yaml.Unmarshal(yamlData, &cnvVm); err != nil {
 		return "", err
 	}
-	if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.BIOS != nil {
-		return "bios", nil
-	}
-	if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.EFI != nil {
-		return "uefi", nil
+
+	if cnvVm.Spec.Template != nil &&
+		cnvVm.Spec.Template.Spec.Domain.Firmware != nil &&
+		cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader != nil {
+
+		if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.BIOS != nil {
+			return "bios", nil
+		}
+		if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.EFI != nil {
+			return "uefi", nil
+		}
 	}
 
 	log.Info("Firmware type was not detected")

diff --git a/pkg/controller/plan/util/kubevirtvmparser_test.go b/pkg/controller/plan/util/kubevirtvmparser_test.go
@@ -13,6 +13,8 @@ func TestKubevirtVmParser(t *testing.T) {
 	testFile(t, "new_format_efi.yml", "uefi")
 	testFile(t, "old_format_bios.yml", "bios")
 	testFile(t, "old_format_efi.yml", "uefi")
+	testFile(t, "old_format_none.yml", "")
+	testFile(t, "new_format_none.yml", "")
 }
 
 func testFile(t *testing.T, filename, expectedFormat string) {

diff --git a/pkg/controller/plan/util/testdata/new_format_none.yml b/pkg/controller/plan/util/testdata/new_format_none.yml
@@ -0,0 +1,37 @@
+---
+# generated by virt-v2v 2.5.8fedora=40,release=1.fc40
+apiVersion: kubevirt.io/v1
+kind: VirtualMachine
+metadata:
+  name: mnecas-win-2022
+  labels:
+    libguestfs.org/virt-v2v-version: "2.5.8"
+    libguestfs.org/genid: f2fc75e8-4961-1425-4740-fcf2bc09685a
+    libguestfs.org/osinfo: win2k22
+    libguestfs.org/source: vmware
+spec:
+  template:
+    spec:
+      domain:
+        firmware:
+          bootloader:
+        resources:
+          clock:
+            timer:
+              hpet:
+                present: false
+              hyperv: {}
+              pit:
+                tickPolicy: delay
+              rtc:
+                tickPolicy: catchup
+            utc: {}
+          requests:
+            memory: 8192Mi
+          features:
+        cpu:
+          sockets: 2
+          cores: 2
+          thread: 1
+      terminationGracePeriodSeconds: 0
+
diff --git a/pkg/controller/plan/util/testdata/old_format_none.yml b/pkg/controller/plan/util/testdata/old_format_none.yml
@@ -0,0 +1,37 @@
+---
+# generated by virt-v2v 2.5.8fedora=40,release=1.fc40
+apiVersion: kubevirt.io/v1
+kind: VirtualMachine
+metadata:
+  name: mnecas-win-2022
+  labels:
+    libguestfs.org/virt-v2v-version: "2.5.8"
+    libguestfs.org/genid: 4f3c8a7c-3464-c9b0-cf79-08325a6bf1e8
+    libguestfs.org/osinfo: win2k22
+    libguestfs.org/source: kvm
+spec:
+  template:
+    spec:
+      domain:
+        os:
+        resources:
+          clock:
+            timer:
+              hpet:
+                present: false
+              hyperv: {}
+              pit:
+                tickPolicy: delay
+              rtc:
+                tickPolicy: catchup
+            utc: {}
+          requests:
+            memory: 8192Mi
+          features:
+            acpi: {}
+            apic: {}
+        cpu:
+          sockets: 2
+          cores: 2
+          thread: 1
+      terminationGracePeriodSeconds: 0