feat: add validation result so we can pass reason more clearly

Signed-off-by: Dustin Scott <[email protected]>

resources/resources.go

@@ -4,12 +4,17 @@ import (
 	admissionv1 "k8s.io/api/admission/v1"
 )
 
+type WindowsValidationResult struct {
+	NeedsValidation bool
+	Reason          string
+}
+
 // WindowsInstanceValidator is an interface that represents an object containing all methods required to
 // validate a windows instance.
 type WindowsInstanceValidator interface {
 	Extract(*admissionv1.AdmissionRequest) (*virtualMachineInstance, error)
 	SumCPU() int
-	NeedsValidation() bool
+	NeedsValidation() *WindowsValidationResult
 
 	GetName() string
 	GetNamespace() string

resources/virtualmachine.go

@@ -33,7 +33,7 @@ func (vm virtualMachine) Extract(admissionRequest *admissionv1.AdmissionRequest)
 }
 
 // NeedsValidation returns if a virtual machine object needs validation or not.
-func (vm virtualMachine) NeedsValidation() bool {
+func (vm virtualMachine) NeedsValidation() *WindowsValidationResult {
 	return vm.isWindows()
 }
 
@@ -58,9 +58,11 @@ func (vm virtualMachine) VirtualMachineInstance() *virtualMachineInstance {
 }
 
 // isWindows determines if a virtual machine object is a windows instance or not.
-func (vm virtualMachine) isWindows() bool {
-	if vm.hasWindowsPreference() {
-		return true
+func (vm virtualMachine) isWindows() *WindowsValidationResult {
+	result := vm.hasWindowsPreference()
+
+	if result.NeedsValidation {
+		return result
 	}
 
 	return vm.VirtualMachineInstance().isWindows()
@@ -71,14 +73,14 @@ func (vm virtualMachine) isWindows() bool {
 // way to determine windows but it does not stop a user from mislabeling the instance type.
 // WARN: it should be noted that users who deploy their instances via YAML may have a copy/paste error that includes
 // sysprep volumes for linux machines.  This is guaranteed to work when using out of the box OpenShift templates.
-func (vm virtualMachine) hasWindowsPreference() bool {
+func (vm virtualMachine) hasWindowsPreference() *WindowsValidationResult {
 	if vm.Spec.Preference == nil {
-		return false
+		return &WindowsValidationResult{Reason: "nil preference"}
 	}
 
 	if strings.HasPrefix(vm.Spec.Preference.Name, "windows") {
-		return true
+		return &WindowsValidationResult{Reason: "has windows name preference"}
 	}
 
-	return false
+	return &WindowsValidationResult{Reason: "has no windows preference"}
 }

resources/virtualmachineinstance.go

@@ -32,7 +32,7 @@ func (vmi virtualMachineInstance) Extract(admissionRequest *admissionv1.Admissio
 }
 
 // NeedsValidation returns if a virtual machine instance object needs validation or not.
-func (vmi virtualMachineInstance) NeedsValidation() bool {
+func (vmi virtualMachineInstance) NeedsValidation() *WindowsValidationResult {
 	// if we have owner references, see if we are owned by a virtual machine
 	if len(vmi.GetOwnerReferences()) > 0 {
 		for _, ref := range vmi.GetOwnerReferences() {
@@ -41,7 +41,7 @@ func (vmi virtualMachineInstance) NeedsValidation() bool {
 			// to determine for windows.
 			// TODO: this logic is likely to need adjusted.
 			if ref.Name == VirtualMachineType {
-				return false
+				return &WindowsValidationResult{Reason: "virtual machine instance owned by virtual machine object"}
 			}
 		}
 	}
@@ -80,34 +80,36 @@ func (vmi virtualMachineInstance) SumCPU() int {
 }
 
 // isWindows determines if a virtual machine instance object is a windows instance or not.
-func (vmi virtualMachineInstance) isWindows() bool {
-	for _, hasWindowsIdentifier := range []func() bool{
+func (vmi virtualMachineInstance) isWindows() *WindowsValidationResult {
+	for _, hasWindowsIdentifier := range []func() *WindowsValidationResult{
 		vmi.hasSysprepVolume,
 		vmi.hasWindowsDriverDiskVolume,
 		vmi.hasHyperV,
 		vmi.hasWindowsPreference,
 	} {
-		if hasWindowsIdentifier() {
-			return true
+		result := hasWindowsIdentifier()
+
+		if result.NeedsValidation {
+			return result
 		}
 	}
 
-	return false
+	return &WindowsValidationResult{Reason: "no validation required"}
 }
 
 // hasSysprepVolume returns if the virtualmachineinstance has a sysprep volume or not.  Sysprep volumes are exclusive
 // to windows machines.
 // WARN: it should be noted that users who deploy their instances via YAML may have a copy/paste error that includes
 // sysprep volumes for linux machines. This is guaranteed to work when using out of the box OpenShift templates, but
 // may not work with use created templates.
-func (vmi virtualMachineInstance) hasSysprepVolume() bool {
+func (vmi virtualMachineInstance) hasSysprepVolume() *WindowsValidationResult {
 	for _, volume := range vmi.Spec.Volumes {
 		if volume.Sysprep != nil {
-			return true
+			return &WindowsValidationResult{NeedsValidation: true, Reason: "has sysprep volume"}
 		}
 	}
 
-	return false
+	return &WindowsValidationResult{Reason: "has no sysprep volume"}
 }
 
 // hasWindowsDriverDiskVolume returns if the virtualmachineinstance has a windows driver volume or not.  Windows driver
@@ -116,49 +118,53 @@ func (vmi virtualMachineInstance) hasSysprepVolume() bool {
 // WARN: it should be noted that users who deploy their instances via YAML may have a copy/paste error that includes
 // sysprep volumes for linux machines. This is guaranteed to work when using out of the box OpenShift templates, but
 // may not work with use created templates.
-func (vmi virtualMachineInstance) hasWindowsDriverDiskVolume() bool {
+func (vmi virtualMachineInstance) hasWindowsDriverDiskVolume() *WindowsValidationResult {
 	for _, volume := range vmi.Spec.Volumes {
 		if volume.DataVolume == nil {
 			continue
 		}
 
 		if volume.DataVolume.Name == "windows-drivers-disk" {
-			return true
+			return &WindowsValidationResult{NeedsValidation: true, Reason: "has windows-driver-disk-volume"}
 		}
 	}
 
-	return false
+	return &WindowsValidationResult{Reason: "has no windows-driver-disk-volume"}
 }
 
 // hasHyperV returns if the virtualmachineinstance has Hyper-V settings set.
 // WARN: it should be noted that users who deploy their instances via YAML may have a copy/paste error.
 // This is guaranteed to work when using out of the box OpenShift templates, but may not work with use created
 // templates.
-func (vmi virtualMachineInstance) hasHyperV() bool {
+func (vmi virtualMachineInstance) hasHyperV() *WindowsValidationResult {
 	if vmi.Spec.Domain.Features == nil {
-		return false
+		return &WindowsValidationResult{Reason: "has nil features"}
 	}
 
 	if vmi.Spec.Domain.Features.Hyperv != nil {
-		return true
+		return &WindowsValidationResult{NeedsValidation: true, Reason: "has hyper-v features"}
 	}
 
-	return false
+	return &WindowsValidationResult{Reason: "has no hyper-v features"}
 }
 
 // hasWindowsPreference returns if the virtualmachineinstance has a windows preference annotation.
 // WARN: it should be noted that this annotation is created when provisioning from instance type.  It is entirely
 // possible that users can select their own instance type and bypass this check.
-func (vmi virtualMachineInstance) hasWindowsPreference() bool {
+func (vmi virtualMachineInstance) hasWindowsPreference() *WindowsValidationResult {
 	annotations := vmi.GetAnnotations()
 
 	if len(annotations) == 0 {
-		return false
+		return &WindowsValidationResult{Reason: "has no annotations"}
 	}
 
 	if annotations["vm.kubevirt.io/os"] == "windows" {
-		return true
+		return &WindowsValidationResult{NeedsValidation: true, Reason: "has 'vm.kubevirt.io/os' windows annotation"}
+	}
+
+	if strings.HasPrefix(annotations["kubevirt.io/cluster-preference-name"], "windows") {
+		return &WindowsValidationResult{NeedsValidation: true, Reason: "has 'kubevirt.io/cluster-preference-name' windows annotation"}
 	}
 
-	return strings.HasPrefix(annotations["kubevirt.io/cluster-preference-name"], "windows")
+	return &WindowsValidationResult{Reason: "has no windows preference"}
 }

resources/virtualmachineinstance_test.go

@@ -7,60 +7,6 @@ import (
 	corev1 "kubevirt.io/api/core/v1"
 )
 
-func Test_virtualMachineInstance_hasWindowsPreference(t *testing.T) {
-	tests := []struct {
-		name string
-		vmi  virtualMachineInstance
-		want bool
-	}{
-		{
-			name: "ensure resource with windows prefix annotation returns true",
-			vmi: virtualMachineInstance{
-				ObjectMeta: v1.ObjectMeta{
-					Annotations: map[string]string{
-						"kubevirt.io/cluster-preference-name": "windows.2k19",
-					},
-				},
-			},
-			want: true,
-		},
-		{
-			name: "ensure resource without windows prefix annotation returns true",
-			vmi: virtualMachineInstance{
-				ObjectMeta: v1.ObjectMeta{
-					Annotations: map[string]string{
-						"kubevirt.io/cluster-preference-name": "rhel.9",
-					},
-				},
-			},
-			want: false,
-		},
-		{
-			name: "ensure resource without annotation returns false",
-			vmi: virtualMachineInstance{
-				ObjectMeta: v1.ObjectMeta{
-					Annotations: map[string]string{
-						"fake": "annotation",
-					},
-				},
-			},
-			want: false,
-		},
-		{
-			name: "ensure resource with no annotations returns false",
-			vmi:  virtualMachineInstance{},
-			want: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := tt.vmi.hasWindowsPreference(); got != tt.want {
-				t.Errorf("virtualMachineInstance.hasWindowsPreference() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func Test_virtualMachineInstance_NeedsValidation(t *testing.T) {
 	t.Parallel()
 
@@ -195,7 +141,8 @@ func Test_virtualMachineInstance_NeedsValidation(t *testing.T) {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			if got := tt.vmi.NeedsValidation(); got != tt.want {
+			result := tt.vmi.NeedsValidation()
+			if got := result.NeedsValidation; got != tt.want {
 				t.Errorf("virtualMachineInstance.NeedsValidation() = %v, want %v", got, tt.want)
 			}
 		})

resources/virtualmachineinstances.go

@@ -20,11 +20,11 @@ func (instances VirtualMachineInstances) Filter(filter *VirtualMachineInstancesF
 	for i := 0; i < len(instances); i++ {
 		var instance virtualMachineInstance = virtualMachineInstance(instances[i])
 
-		if instance.hasSysprepVolume() {
+		if instance.hasSysprepVolume().NeedsValidation {
 			filtered = append(filtered, corev1.VirtualMachineInstance(instance))
 		}
 
-		if instance.hasWindowsDriverDiskVolume() {
+		if instance.hasWindowsDriverDiskVolume().NeedsValidation {
 			filtered = append(filtered, corev1.VirtualMachineInstance(instance))
 		}
 	}

resources/virtualmachines.go

@@ -20,7 +20,7 @@ func (vms VirtualMachines) Filter(filter *VirtualMachinesFilter) VirtualMachineI
 	for i := 0; i < len(vms); i++ {
 		var vm virtualMachine = virtualMachine(vms[i])
 
-		if vm.isWindows() {
+		if vm.isWindows().NeedsValidation {
 			filtered = append(filtered, corev1.VirtualMachineInstance(*vm.VirtualMachineInstance()))
 		}
 	}

webhook/webhook.go

@@ -68,11 +68,14 @@ func (wh *webhook) Validate(w http.ResponseWriter, r *http.Request) {
 	wh.Log.Debug().Msgf("OBJECT: %+v", op.object)
 
 	// return immediately if we do not need validation
-	if !op.object.NeedsValidation() {
-		wh.respond(op, "skipping validation", true)
+	validationResult := op.object.NeedsValidation()
+	if !validationResult.NeedsValidation {
+		wh.respond(op, fmt.Sprintf("skipping validation, reason [%s]", validationResult.Reason), true)
 		return
 	}
 
+	wh.Log.Info().Msgf("validating request for reason [%s]", validationResult.Reason)
+
 	// get the requested capacity from the request
 	requested := op.object.SumCPU()