Merge pull request kubevirt#12933 from ShellyKa13/enforce-dvtemplate

VM admitter: improve validation of datavolumetemplate
praveenkumr · Oct 7, 2024 · e2d6419 · e2d6419
2 parents 64bd594 + dd639b3
commit e2d6419
Show file tree

Hide file tree

Showing 2 changed files with 327 additions and 139 deletions.
diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/vms-admitter.go b/pkg/virt-api/webhooks/validating-webhook/admitters/vms-admitter.go
@@ -23,6 +23,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"reflect"
 
 	"kubevirt.io/kubevirt/pkg/virt-config/deprecation"
 
@@ -33,6 +34,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	k8sfield "k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/client-go/tools/cache"
 
@@ -434,40 +436,116 @@ func ValidateVirtualMachineSpec(field *k8sfield.Path, spec *v1.VirtualMachineSpe
 }
 
 func validateDataVolumeTemplate(field *k8sfield.Path, spec *v1.VirtualMachineSpec) (causes []metav1.StatusCause) {
-	if len(spec.DataVolumeTemplates) > 0 {
-
-		for idx, dataVolume := range spec.DataVolumeTemplates {
-			if dataVolume.Name == "" {
-				causes = append(causes, metav1.StatusCause{
-					Type:    metav1.CauseTypeFieldValueRequired,
-					Message: fmt.Sprintf("'name' field must not be empty for DataVolumeTemplate entry %s.", field.Child("dataVolumeTemplate").Index(idx).String()),
-					Field:   field.Child("dataVolumeTemplate").Index(idx).Child("name").String(),
-				})
-			}
+	for idx, dataVolume := range spec.DataVolumeTemplates {
+		cause := validateDataVolume(field.Child("dataVolumeTemplate").Index(idx), dataVolume)
+		if cause != nil {
+			causes = append(causes, cause...)
+			continue
+		}
 
-			dataVolumeRefFound := false
-			for _, volume := range spec.Template.Spec.Volumes {
-				if volume.VolumeSource.PersistentVolumeClaim != nil && volume.VolumeSource.PersistentVolumeClaim.ClaimName == dataVolume.Name {
-					dataVolumeRefFound = true
-					break
-				} else if volume.VolumeSource.DataVolume != nil && volume.VolumeSource.DataVolume.Name == dataVolume.Name {
-					dataVolumeRefFound = true
-					break
-				}
+		dataVolumeRefFound := false
+		for _, volume := range spec.Template.Spec.Volumes {
+			if volume.VolumeSource.PersistentVolumeClaim != nil && volume.VolumeSource.PersistentVolumeClaim.ClaimName == dataVolume.Name ||
+				volume.VolumeSource.DataVolume != nil && volume.VolumeSource.DataVolume.Name == dataVolume.Name {
+				dataVolumeRefFound = true
+				break
 			}
+		}
 
-			if !dataVolumeRefFound {
-				causes = append(causes, metav1.StatusCause{
-					Type:    metav1.CauseTypeFieldValueRequired,
-					Message: fmt.Sprintf("DataVolumeTemplate entry %s must be referenced in the VMI template's 'volumes' list", field.Child("dataVolumeTemplate").Index(idx).String()),
-					Field:   field.Child("dataVolumeTemplate").Index(idx).String(),
-				})
-			}
+		if !dataVolumeRefFound {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueRequired,
+				Message: fmt.Sprintf("DataVolumeTemplate entry %s must be referenced in the VMI template's 'volumes' list", field.Child("dataVolumeTemplate").Index(idx).String()),
+				Field:   field.Child("dataVolumeTemplate").Index(idx).String(),
+			})
 		}
 	}
 	return causes
 }
 
+func validateDataVolume(field *k8sfield.Path, dataVolume v1.DataVolumeTemplateSpec) []metav1.StatusCause {
+	if dataVolume.Name == "" {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueRequired,
+			Message: fmt.Sprintf("'name' field must not be empty for DataVolumeTemplate entry %s.", field.Child("name").String()),
+			Field:   field.Child("name").String(),
+		}}
+	}
+	if dataVolume.Spec.PVC == nil && dataVolume.Spec.Storage == nil {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: "Missing Data volume PVC or Storage",
+			Field:   field.Child("PVC", "Storage").String(),
+		}}
+	}
+	if dataVolume.Spec.PVC != nil && dataVolume.Spec.Storage != nil {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: "Duplicate storage definition, both target storage and target pvc defined",
+			Field:   field.Child("PVC", "Storage").String(),
+		}}
+	}
+
+	var dataSourceRef *corev1.TypedObjectReference
+	var dataSource *corev1.TypedLocalObjectReference
+	if dataVolume.Spec.PVC != nil {
+		dataSourceRef = dataVolume.Spec.PVC.DataSourceRef
+		dataSource = dataVolume.Spec.PVC.DataSource
+	} else if dataVolume.Spec.Storage != nil {
+		dataSourceRef = dataVolume.Spec.Storage.DataSourceRef
+		dataSource = dataVolume.Spec.Storage.DataSource
+	}
+
+	// dataVolume is externally populated
+	if (dataSourceRef != nil || dataSource != nil) &&
+		(dataVolume.Spec.Source != nil || dataVolume.Spec.SourceRef != nil) {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: "External population is incompatible with Source and SourceRef",
+			Field:   field.Child("source").String(),
+		}}
+	}
+
+	if dataVolume.Spec.Source == nil && dataVolume.Spec.SourceRef == nil {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: "Data volume should have either Source, SourceRef, or be externally populated",
+			Field:   field.Child("source", "sourceRef").String(),
+		}}
+	}
+
+	if dataVolume.Spec.Source != nil {
+		return validateNumberOfSources(field, dataVolume.Spec.Source)
+	}
+
+	return nil
+}
+
+func validateNumberOfSources(field *field.Path, source *cdiv1.DataVolumeSource) []metav1.StatusCause {
+	numberOfSources := 0
+	s := reflect.ValueOf(source).Elem()
+	for i := 0; i < s.NumField(); i++ {
+		if !reflect.ValueOf(s.Field(i).Interface()).IsNil() {
+			numberOfSources++
+		}
+	}
+	if numberOfSources == 0 {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: "Missing dataVolume valid source",
+			Field:   field.Child("source").String(),
+		}}
+	}
+	if numberOfSources > 1 {
+		return []metav1.StatusCause{{
+			Type:    metav1.CauseTypeFieldValueInvalid,
+			Message: "Multiple dataVolume sources",
+			Field:   field.Child("source").String(),
+		}}
+	}
+	return nil
+}
+
 func validateRunStrategy(field *k8sfield.Path, spec *v1.VirtualMachineSpec) (causes []metav1.StatusCause) {
 	if spec.Running != nil && spec.RunStrategy != nil {
 		causes = append(causes, metav1.StatusCause{