Merge pull request kubevirt#2164 from slintes/reject-floppy

Reject floppy disks
plwhite · Apr 1, 2019 · dad1392 · dad1392
2 parents 231ef60 + 96a3f48
commit dad1392
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 34 deletions.
diff --git a/pkg/virt-api/webhooks/validating-webhook/validating-webhook.go b/pkg/virt-api/webhooks/validating-webhook/validating-webhook.go
@@ -125,6 +125,16 @@ func validateDisks(field *k8sfield.Path, disks []v1.Disk) []metav1.StatusCause {
 				Field:   field.Index(idx).Child("name").String(),
 			})
 		}
+
+		// Reject Floppy disks
+		if disk.Floppy != nil {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueNotSupported,
+				Message: fmt.Sprintf("Floppy disks are deprecated and will be removed from the API soon."),
+				Field:   field.Index(idx).Child("name").String(),
+			})
+		}
+
 		// Verify only a single device type is set.
 		deviceTargetSetCount := 0
 		var diskType, bus string

diff --git a/pkg/virt-api/webhooks/validating-webhook/validating-webhook_test.go b/pkg/virt-api/webhooks/validating-webhook/validating-webhook_test.go
@@ -615,8 +615,8 @@ var _ = Describe("Validating Webhook", func() {
 			vmiPDomain.Devices.Disks = append(vmiPDomain.Devices.Disks, v1.Disk{
 				Name: "testdisk",
 				DiskDevice: v1.DiskDevice{
-					Disk:   &v1.DiskTarget{},
-					Floppy: &v1.FloppyTarget{},
+					Disk:  &v1.DiskTarget{},
+					CDRom: &v1.CDRomTarget{},
 				},
 			})
 			vmiPreset := &v1.VirtualMachineInstancePreset{
@@ -1134,8 +1134,8 @@ var _ = Describe("Validating Webhook", func() {
 			vmi.Spec.Domain.Devices.Disks = append(vmi.Spec.Domain.Devices.Disks, v1.Disk{
 				Name: "testdisk",
 				DiskDevice: v1.DiskDevice{
-					Disk:   &v1.DiskTarget{},
-					Floppy: &v1.FloppyTarget{},
+					Disk:  &v1.DiskTarget{},
+					CDRom: &v1.CDRomTarget{},
 				},
 			})
 
@@ -2888,13 +2888,25 @@ var _ = Describe("Validating Webhook", func() {
 			table.Entry("with LUN target",
 				v1.Disk{Name: "testdisk", DiskDevice: v1.DiskDevice{LUN: &v1.LunTarget{}}},
 			),
-			table.Entry("with Floppy target",
-				v1.Disk{Name: "testdisk", DiskDevice: v1.DiskDevice{Floppy: &v1.FloppyTarget{}}},
-			),
 			table.Entry("with CDRom target",
 				v1.Disk{Name: "testdisk", DiskDevice: v1.DiskDevice{CDRom: &v1.CDRomTarget{}}},
 			),
 		)
+
+		It("should reject floppy disks", func() {
+			vmi := v1.NewMinimalVMI("testvmi")
+
+			vmi.Spec.Domain.Devices.Disks = append(vmi.Spec.Domain.Devices.Disks, v1.Disk{
+				Name: "floppydisk",
+				DiskDevice: v1.DiskDevice{
+					Floppy: &v1.FloppyTarget{},
+				},
+			})
+			causes := validateDisks(k8sfield.NewPath("fake"), vmi.Spec.Domain.Devices.Disks)
+			Expect(len(causes)).To(Equal(1))
+			Expect(causes[0].Field).To(Equal("fake[0].name"))
+		})
+
 		It("should allow disk without a target", func() {
 			vmi := v1.NewMinimalVMI("testvmi")
 
@@ -2912,6 +2924,7 @@ var _ = Describe("Validating Webhook", func() {
 			causes := validateDisks(k8sfield.NewPath("fake"), vmi.Spec.Domain.Devices.Disks)
 			Expect(len(causes)).To(Equal(0))
 		})
+
 		It("should reject disks with duplicate names ", func() {
 			vmi := v1.NewMinimalVMI("testvmi")
 
@@ -2970,8 +2983,8 @@ var _ = Describe("Validating Webhook", func() {
 			vmi.Spec.Domain.Devices.Disks = append(vmi.Spec.Domain.Devices.Disks, v1.Disk{
 				Name: "testdisk",
 				DiskDevice: v1.DiskDevice{
-					Disk:   &v1.DiskTarget{},
-					Floppy: &v1.FloppyTarget{},
+					Disk:  &v1.DiskTarget{},
+					CDRom: &v1.CDRomTarget{},
 				},
 			})
 			vmi.Spec.Volumes = append(vmi.Spec.Volumes, v1.Volume{

diff --git a/tests/utils.go b/tests/utils.go
@@ -1409,25 +1409,6 @@ func AddPVCDisk(vmi *v1.VirtualMachineInstance, name string, bus string, claimNa
 	return vmi
 }
 
-func AddEphemeralFloppy(vmi *v1.VirtualMachineInstance, name string, image string) *v1.VirtualMachineInstance {
-	vmi.Spec.Domain.Devices.Disks = append(vmi.Spec.Domain.Devices.Disks, v1.Disk{
-		Name: name,
-		DiskDevice: v1.DiskDevice{
-			Floppy: &v1.FloppyTarget{},
-		},
-	})
-	vmi.Spec.Volumes = append(vmi.Spec.Volumes, v1.Volume{
-		Name: name,
-		VolumeSource: v1.VolumeSource{
-			ContainerDisk: &v1.ContainerDiskSource{
-				Image: image,
-			},
-		},
-	})
-
-	return vmi
-}
-
 func AddEphemeralCdrom(vmi *v1.VirtualMachineInstance, name string, bus string, image string) *v1.VirtualMachineInstance {
 	vmi.Spec.Domain.Devices.Disks = append(vmi.Spec.Domain.Devices.Disks, v1.Disk{
 		Name: name,

diff --git a/tests/vmi_configuration_test.go b/tests/vmi_configuration_test.go
@@ -1074,9 +1074,7 @@ var _ = Describe("Configurations", func() {
 			vmi = tests.NewRandomVMIWithEphemeralDiskAndUserdata(containerImage, "echo hi!\n")
 			// sata
 			tests.AddEphemeralDisk(vmi, "disk2", "sata", containerImage)
-			// floppy
-			tests.AddEphemeralFloppy(vmi, "disk3", containerImage)
-			// NOTE: we have one disk per bus, so we expect vda, sda, fda
+			// NOTE: we have one disk per bus, so we expect vda, sda
 		})
 		checkPciAddress := func(vmi *v1.VirtualMachineInstance, expectedPciAddress string, prompt string) {
 			err := tests.CheckForTextExpecter(vmi, []expect.Batcher{
@@ -1088,7 +1086,6 @@ var _ = Describe("Configurations", func() {
 			Expect(err).ToNot(HaveOccurred())
 		}
 
-		// FIXME floppy is not recognized by the used image right now
 		It("[test_id:1682]should have all the device nodes", func() {
 			vmi, err = virtClient.VirtualMachineInstance(tests.NamespaceTestDefault).Create(vmi)
 			Expect(err).ToNot(HaveOccurred())

diff --git a/tests/vmipreset_test.go b/tests/vmipreset_test.go
@@ -111,8 +111,8 @@ var _ = Describe("[rfe_id:609][crit:medium][vendor:[email protected]][level:comp
 			preset.Spec.Domain.Devices.Disks = append(vmi.Spec.Domain.Devices.Disks, v1.Disk{
 				Name: "testdisk",
 				DiskDevice: v1.DiskDevice{
-					Disk:   &v1.DiskTarget{},
-					Floppy: &v1.FloppyTarget{},
+					Disk:  &v1.DiskTarget{},
+					CDRom: &v1.CDRomTarget{},
 				},
 			})
 			result := virtClient.RestClient().Post().Resource("virtualmachineinstancepresets").Namespace(tests.NamespaceTestDefault).Body(preset).Do()