Allow imported and registered VMs to skip image validation checks on …

…create/update

VMs that are imported, or registered already have the virtual machine
created on the infrastructure.  As such, we don't need to apply the
same strict validations for them such as making sure the image is not
null / valid.  To that end, this change introduces an annotation that
will be applied on the Imported, or Registered VMs.  This will allow
our webhooks to skip these checks if the annotation is present.

For now, maintain the existing mechanism which allows _all_ VM
creations to be imageless if the import or the incremental restore
feature is enabled.  Once this change is merged, we will yank out that
code to make that check stricter (only annotation based).

api/v1alpha3/virtualmachine_types.go

@@ -197,6 +197,19 @@ const (
 	// ManagerID on a VirtualMachine contains the UUID of the
 	// VMware vCenter (VC) that is managing this virtual machine.
 	ManagerID = GroupName + "/manager-id"
+
+	// +kubebuilder:validation:Enum=ImportVM;RegisterVM
+	//
+	// SourceAPI on a VirtualMachine contains the API responsible for creating / updating
+	// 	the VirtualMachine custom resource for a virtual machine that already exists on the
+	// infrastructure. The ImportVM API is used to import a traditional virtual machine in
+	// Supervisor. The RegisterVM API is used to sync the state of a virtual machine back
+	// to Supervisor after a restore/fail-over operation. This annotation helps bypass some
+	// validation checks that are otherwise applicable to all VirtualMachine create/update
+	// requests.
+	//
+	// Valid values are "ImportVM" and "RegisterVM".
+	SourceAPI = GroupName + "/source-api"
 )
 
 const (

pkg/constants/constants.go

@@ -37,4 +37,8 @@ const (
 	// MinSupportedHWVersionForPCIPassthruDevices is the supported virtual
 	// hardware version for NVidia PCI devices.
 	MinSupportedHWVersionForPCIPassthruDevices = vimtypes.VMX17
+
+	// APIs that are exempt from some validation webhook checks because they operate on pre-existing VMs.
+	ImportVMAPI   = "ImportVM"
+	RegisterVMAPI = "RegisterVM"
 )

pkg/util/vmopv1/vm.go

@@ -233,6 +233,17 @@ func IsImagelessVM(vm vmopv1.VirtualMachine) bool {
 	return vm.Spec.Image == nil && vm.Spec.ImageName == ""
 }
 
+// IsImportedVM returns true if the provided VM was imported using ImportVM API.
+func IsImportedVM(vm vmopv1.VirtualMachine) bool {
+	return vm.Annotations != nil && vm.Annotations[vmopv1.SourceAPI] == constants.ImportVMAPI
+}
+
+// IsRegisteredVM returns true if the provided VM was registered with Supervisor
+// using RegisterVM API.
+func IsRegisteredVM(vm vmopv1.VirtualMachine) bool {
+	return vm.Annotations != nil && vm.Annotations[vmopv1.SourceAPI] == constants.RegisterVMAPI
+}
+
 // SyncStorageUsageForNamespace updates the StoragePolicyUsage resource for
 // the given namespace and storage class with the reported usage information
 // for VMs in that namespace that use the specified storage class.

webhooks/virtualmachine/validation/virtualmachine_validator.go

@@ -153,6 +153,35 @@ func (v validator) ValidateDelete(*pkgctx.WebhookRequestContext) admission.Respo
 	return admission.Allowed("")
 }
 
+// Updates to VM's image are only allowed if it is a Registered VM (used for failover
+// in disaster recovery).
+func (v validator) validateImageOnUpdate(ctx *pkgctx.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList {
+	var allErrs field.ErrorList
+
+	if f := pkgcfg.FromContext(ctx).Features; f.VMIncrementalRestore && vmopv1util.IsRegisteredVM(*vm) {
+		if !equality.Semantic.DeepEqual(oldVM.Spec.Image, vm.Spec.Image) {
+			if !ctx.IsPrivilegedAccount {
+				allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "image"), restrictedToPrivUsers))
+			}
+		}
+
+		if !equality.Semantic.DeepEqual(oldVM.Spec.ImageName, vm.Spec.ImageName) {
+			if !ctx.IsPrivilegedAccount {
+				allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "imageName"), restrictedToPrivUsers))
+			}
+		}
+
+		return allErrs
+	}
+
+	allErrs = append(allErrs,
+		validation.ValidateImmutableField(vm.Spec.ImageName, oldVM.Spec.ImageName, field.NewPath("spec", "imageName"))...)
+	allErrs = append(allErrs,
+		validation.ValidateImmutableField(vm.Spec.Image, oldVM.Spec.Image, field.NewPath("spec", "image"))...)
+
+	return allErrs
+}
+
 // ValidateUpdate validates if the given VirtualMachineSpec update is valid.
 // Changes to following fields are not allowed:
 //   - Image
@@ -370,11 +399,21 @@ func (v validator) validateImageOnCreate(ctx *pkgctx.WebhookRequestContext, vm *
 		(pkgcfg.FromContext(ctx).Features.VMImportNewNet ||
 			pkgcfg.FromContext(ctx).Features.VMIncrementalRestore):
 
+		// Skip validations on images if it is a VM created using ImportVM, or RegisterVM.
+		if vmopv1util.IsImportedVM(*vm) || vmopv1util.IsRegisteredVM(*vm) {
+			// Restrict creating imageless VM resources to privileged users.
+			if !ctx.IsPrivilegedAccount {
+				allErrs = append(allErrs, field.Forbidden(f, restrictedToPrivUsers))
+			}
+
+			return allErrs
+		}
+
 		// Restrict creating imageless VM resources to privileged users.
+		// TODO: This should be removed once all consumers have migrated to using annotations.
 		if !ctx.IsPrivilegedAccount {
 			allErrs = append(allErrs, field.Forbidden(f, restrictedToPrivUsers))
 		}
-
 	case vm.Spec.Image == nil:
 		allErrs = append(allErrs, field.Required(f, ""))
 	case vm.Spec.Image.Kind == "":
@@ -1143,8 +1182,7 @@ func (v validator) validateImmutableFields(ctx *pkgctx.WebhookRequestContext, vm
 	var allErrs field.ErrorList
 	specPath := field.NewPath("spec")
 
-	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.Image, oldVM.Spec.Image, specPath.Child("image"))...)
-	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ImageName, oldVM.Spec.ImageName, specPath.Child("imageName"))...)
+	allErrs = append(allErrs, v.validateImageOnUpdate(ctx, vm, oldVM)...)
 	allErrs = append(allErrs, v.validateClassOnUpdate(ctx, vm, oldVM)...)
 	allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.StorageClass, oldVM.Spec.StorageClass, specPath.Child("storageClass"))...)
 	// New VMs always have non-empty biosUUID. Existing VMs being upgraded may have an empty biosUUID.