Validate bootorder to be > 0, if exists

yuvalif · May 29, 2018 · 9101b78 · 9101b78
1 parent 6a240fe
commit 9101b78
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 7 deletions.
diff --git a/pkg/api/v1/schema.go b/pkg/api/v1/schema.go
@@ -131,7 +131,7 @@ type Disk struct {
 	DiskDevice `json:",inline"`
 	// BootOrder is an integer value used to determine ordering of boot devices
 	// +optional
-	BootOrder uint `json:"bootOrder,omitempty"`
+	BootOrder *uint `json:"bootOrder,omitempty"`
 }
 
 // Represents the target of a volume to mount.

diff --git a/pkg/virt-api/validating-webhook/validating-webhook.go b/pkg/virt-api/validating-webhook/validating-webhook.go
@@ -172,6 +172,15 @@ func validateDisks(field *k8sfield.Path, disks []v1.Disk) []metav1.StatusCause {
 				Field:   field.Index(idx).String(),
 			})
 		}
+
+		// Verify boot order is greater than 0, if provided
+		if disk.BootOrder != nil && *disk.BootOrder < 1 {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: fmt.Sprintf("%s must have a boot order > 0, if supplied", field.Index(idx).String()),
+				Field:   field.Index(idx).Child("bootorder").String(),
+			})
+		}
 	}
 
 	return causes

diff --git a/pkg/virt-api/validating-webhook/validating-webhook_test.go b/pkg/virt-api/validating-webhook/validating-webhook_test.go
@@ -646,5 +646,40 @@ var _ = Describe("Validating Webhook", func() {
 			Expect(len(causes)).To(Equal(1))
 			Expect(causes[0].Field).To(Equal("fake[0]"))
 		})
+
+		It("should accept a boot order greater than '0'", func() {
+			vm := v1.NewMinimalVM("testvm")
+			order := uint(1)
+
+			vm.Spec.Domain.Devices.Disks = append(vm.Spec.Domain.Devices.Disks, v1.Disk{
+				Name:       "testdisk",
+				VolumeName: "testvolume1",
+				BootOrder:  &order,
+				DiskDevice: v1.DiskDevice{
+					Disk: &v1.DiskTarget{},
+				},
+			})
+
+			causes := validateDisks(k8sfield.NewPath("fake"), vm.Spec.Domain.Devices.Disks)
+			Expect(len(causes)).To(Equal(0))
+		})
+
+		It("should reject a disk with a boot order of '0'", func() {
+			vm := v1.NewMinimalVM("testvm")
+			order := uint(0)
+
+			vm.Spec.Domain.Devices.Disks = append(vm.Spec.Domain.Devices.Disks, v1.Disk{
+				Name:       "testdisk",
+				VolumeName: "testvolume1",
+				BootOrder:  &order,
+				DiskDevice: v1.DiskDevice{
+					Disk: &v1.DiskTarget{},
+				},
+			})
+
+			causes := validateDisks(k8sfield.NewPath("fake"), vm.Spec.Domain.Devices.Disks)
+			Expect(len(causes)).To(Equal(1))
+			Expect(causes[0].Field).To(Equal("fake[0].bootorder"))
+		})
 	})
 })
diff --git a/pkg/virt-launcher/virtwrap/api/converter.go b/pkg/virt-launcher/virtwrap/api/converter.go
@@ -73,8 +73,8 @@ func Convert_v1_Disk_To_api_Disk(diskDevice *v1.Disk, disk *Disk, devicePerBus m
 		Name: "qemu",
 	}
 	disk.Alias = &Alias{Name: diskDevice.Name}
-	if diskDevice.BootOrder != 0 {
-		disk.BootOrder = &BootOrder{Order: diskDevice.BootOrder}
+	if diskDevice.BootOrder != nil {
+		disk.BootOrder = &BootOrder{Order: *diskDevice.BootOrder}
 	}
 
 	return nil

diff --git a/pkg/virt-launcher/virtwrap/api/converter_test.go b/pkg/virt-launcher/virtwrap/api/converter_test.go
@@ -38,9 +38,10 @@ var _ = Describe("Converter", func() {
 
 	Context("with v1.Disk", func() {
 		It("Should add boot order when provided", func() {
+			order := uint(1)
 			kubevirtDisk := &v1.Disk{
 				Name:       "mydisk",
-				BootOrder:  1,
+				BootOrder:  &order,
 				VolumeName: "myvolume",
 				DiskDevice: v1.DiskDevice{
 					Disk: &v1.DiskTarget{
@@ -80,6 +81,7 @@ var _ = Describe("Converter", func() {
 			fmt.Println(xml)
 			Expect(xml).To(Equal(convertedDisk))
 		})
+
 	})
 
 	Context("with v1.VirtualMachine", func() {

diff --git a/tests/utils.go b/tests/utils.go
@@ -785,7 +785,7 @@ func AddEphemeralDisk(vm *v1.VirtualMachine, name string, bus string, image stri
 	return vm
 }
 
-func AddBootOrderToDisk(vm *v1.VirtualMachine, diskName string, bootorder uint) *v1.VirtualMachine {
+func AddBootOrderToDisk(vm *v1.VirtualMachine, diskName string, bootorder *uint) *v1.VirtualMachine {
 	for i, d := range vm.Spec.Domain.Devices.Disks {
 		if d.Name == diskName {
 			vm.Spec.Domain.Devices.Disks[i].BootOrder = bootorder

diff --git a/tests/vmlifecycle_test.go b/tests/vmlifecycle_test.go
@@ -167,8 +167,8 @@ var _ = Describe("Vmlifecycle", func() {
 				tests.AddEphemeralDisk(vm, "disk2", "virtio", tests.RegistryDiskFor(tests.RegistryDiskCirros))
 
 				By("setting boot order")
-				vm = tests.AddBootOrderToDisk(vm, "disk0", alpineBootOrder)
-				vm = tests.AddBootOrderToDisk(vm, "disk2", cirrosBootOrder)
+				vm = tests.AddBootOrderToDisk(vm, "disk0", &alpineBootOrder)
+				vm = tests.AddBootOrderToDisk(vm, "disk2", &cirrosBootOrder)
 
 				By("starting VM")
 				vm, err = virtClient.VM(tests.NamespaceTestDefault).Create(vm)