🌱 Allow imported and registered VMs to skip image validation checks on Create/Update

api/v1alpha3/virtualmachine_types.go

@@ -197,6 +197,18 @@ const (
 	// ManagerID on a VirtualMachine contains the UUID of the
 	// VMware vCenter (VC) that is managing this virtual machine.
 	ManagerID = GroupName + "/manager-id"
+
+	// RegisteredVMAnnotation on a VirtualMachine represents that a virtual machine has
+	// been registered using the RegisterVM API after a restore, or a fail-over operation by
+	// a vendor. The presence of this annotation is used to bypass some validation checks
+	// that are otherwise applicable to all VirtualMachine create/update requests.
+	RegisteredVMAnnotation = GroupName + "/registered-vm"
+
+	// ImportedVMAnnotation on a VirtualMachine represents that a traditional virtual
+	// machine has been imported into Supervisor using the ImportVM API. The presence of this
+	// annotation is used to bypass some validation checks that are otherwise applicable
+	// to all VirtualMachine create/update requests.
+	ImportedVMAnnotation = GroupName + "/imported-vm"
 )
 
 const (

pkg/util/annotations/helpers.go

@@ -9,6 +9,14 @@ import (
 	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3"
 )
 
+func HasRegisterVM(o metav1.Object) bool {
+	return hasAnnotation(o, vmopv1.RegisteredVMAnnotation)
+}
+
+func HasImportVM(o metav1.Object) bool {
+	return hasAnnotation(o, vmopv1.ImportedVMAnnotation)
+}
+
 func HasForceEnableBackup(o metav1.Object) bool {
 	return hasAnnotation(o, vmopv1.ForceEnableBackupAnnotation)
 }

pkg/util/annotations/helpers_test.go

@@ -46,3 +46,37 @@ var _ = DescribeTable(
 	Entry("present but empty ", map[string]string{vmopv1.ForceEnableBackupAnnotation: ""}, true),
 	Entry("present and not empty ", map[string]string{vmopv1.ForceEnableBackupAnnotation: "true"}, true),
 )
+
+var _ = DescribeTable(
+	"HasRegisterVM",
+	func(in map[string]string, out bool) {
+		vm := &vmopv1.VirtualMachine{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: in,
+			},
+		}
+		actual := annotations.HasRegisterVM(vm)
+		Expect(actual).To(Equal(out))
+	},
+	Entry("nil", nil, false),
+	Entry("not present", map[string]string{"foo": "bar"}, false),
+	Entry("present but empty ", map[string]string{vmopv1.RegisteredVMAnnotation: ""}, true),
+	Entry("present and not empty ", map[string]string{vmopv1.RegisteredVMAnnotation: "true"}, true),
+)
+
+var _ = DescribeTable(
+	"HasImportVM",
+	func(in map[string]string, out bool) {
+		vm := &vmopv1.VirtualMachine{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: in,
+			},
+		}
+		actual := annotations.HasImportVM(vm)
+		Expect(actual).To(Equal(out))
+	},
+	Entry("nil", nil, false),
+	Entry("not present", map[string]string{"foo": "bar"}, false),
+	Entry("present but empty ", map[string]string{vmopv1.ImportedVMAnnotation: ""}, true),
+	Entry("present and not empty ", map[string]string{vmopv1.ImportedVMAnnotation: "true"}, true),
+)

pkg/util/vmopv1/vm.go

@@ -233,6 +233,19 @@ func IsImagelessVM(vm vmopv1.VirtualMachine) bool {
 	return vm.Spec.Image == nil && vm.Spec.ImageName == ""
 }
 
+// ImageRefsEqual returns true if the two image refs match.
+func ImageRefsEqual(ref1, ref2 *vmopv1.VirtualMachineImageRef) bool {
+	if ref1 == nil && ref2 == nil {
+		return true
+	}
+
+	if ref1 != nil && ref2 != nil {
+		return *ref1 == *ref2
+	}
+
+	return false
+}
+
 // 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.

pkg/util/vmopv1/vm_test.go

@@ -522,6 +522,59 @@ var _ = DescribeTable("IsImageLessVM",
 	),
 )
 
+var _ = DescribeTable("ImageRefsEqual",
+	func(
+		ref1 *vmopv1.VirtualMachineImageRef,
+		ref2 *vmopv1.VirtualMachineImageRef,
+		expected bool,
+	) {
+		Ω(vmopv1util.ImageRefsEqual(ref1, ref2)).Should(Equal(expected))
+	},
+	Entry(
+		"both refs nil",
+		nil,
+		nil,
+		true,
+	),
+	Entry(
+		"ref1 is nil, ref2 is not",
+		nil,
+		&vmopv1.VirtualMachineImageRef{
+			Name: "dummy",
+		},
+		false,
+	),
+	Entry(
+		"ref1 is not nil, ref2 is nil",
+		&vmopv1.VirtualMachineImageRef{
+			Name: "dummy",
+		},
+		nil,
+		false,
+	),
+	Entry(
+		"both not nil, one containing an extra field",
+		&vmopv1.VirtualMachineImageRef{
+			Name: "dummy",
+		},
+		&vmopv1.VirtualMachineImageRef{
+			Name: "dummy",
+			Kind: "ClusterVirtualMachineImage",
+		},
+		false,
+	),
+	Entry(
+		"both not nil, same values",
+		&vmopv1.VirtualMachineImageRef{
+			Name: "dummy",
+		},
+		&vmopv1.VirtualMachineImageRef{
+			Name: "dummy",
+		},
+		true,
+	),
+)
+
 var _ = Describe("SyncStorageUsageForNamespace", func() {
 	var (
 		ctx          context.Context

webhooks/virtualmachine/validation/virtualmachine_validator.go

@@ -41,6 +41,7 @@ import (
 	"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/config"
 	"github.com/vmware-tanzu/vm-operator/pkg/topology"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
+	"github.com/vmware-tanzu/vm-operator/pkg/util/annotations"
 	cloudinitvalidate "github.com/vmware-tanzu/vm-operator/pkg/util/cloudinit/validate"
 	kubeutil "github.com/vmware-tanzu/vm-operator/pkg/util/kube"
 	vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1"
@@ -153,6 +154,37 @@ 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 vmopv1util.IsImagelessVM(*vm) && pkgcfg.FromContext(ctx).Features.VMIncrementalRestore {
+		if annotations.HasRegisterVM(vm) {
+			if !vmopv1util.ImageRefsEqual(vm.Spec.Image, oldVM.Spec.Image) {
+				if !ctx.IsPrivilegedAccount {
+					allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "image"), restrictedToPrivUsers))
+				}
+			}
+
+			if 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
@@ -369,8 +401,19 @@ func (v validator) validateImageOnCreate(ctx *pkgctx.WebhookRequestContext, vm *
 	case vmopv1util.IsImagelessVM(*vm) &&
 		(pkgcfg.FromContext(ctx).Features.VMImportNewNet ||
 			pkgcfg.FromContext(ctx).Features.VMIncrementalRestore):
+		// TODO: Simplify this once mobility operator starts creating VMs with correct annotation.
+		// Skip validations on images if it is a VM created using ImportVM, or RegisterVM.
+		if annotations.HasImportVM(vm) || annotations.HasRegisterVM(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))
 		}
@@ -1143,8 +1186,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.
@@ -1288,21 +1330,29 @@ func (v validator) validateAnnotation(ctx *pkgctx.WebhookRequestContext, vm, old
 	annotationPath := field.NewPath("metadata", "annotations")
 
 	if vm.Annotations[vmopv1.InstanceIDAnnotation] != oldVM.Annotations[vmopv1.InstanceIDAnnotation] {
-		allErrs = append(allErrs, field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), modifyAnnotationNotAllowedForNonAdmin))
+		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.InstanceIDAnnotation), modifyAnnotationNotAllowedForNonAdmin))
 	}
 
 	if vm.Annotations[vmopv1.FirstBootDoneAnnotation] != oldVM.Annotations[vmopv1.FirstBootDoneAnnotation] {
-		allErrs = append(allErrs, field.Forbidden(annotationPath.Child(vmopv1.FirstBootDoneAnnotation), modifyAnnotationNotAllowedForNonAdmin))
+		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.FirstBootDoneAnnotation), modifyAnnotationNotAllowedForNonAdmin))
+	}
+
+	if vm.Annotations[vmopv1.RegisteredVMAnnotation] != oldVM.Annotations[vmopv1.RegisteredVMAnnotation] {
+		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.RegisteredVMAnnotation), modifyAnnotationNotAllowedForNonAdmin))
+	}
+
+	if vm.Annotations[vmopv1.ImportedVMAnnotation] != oldVM.Annotations[vmopv1.ImportedVMAnnotation] {
+		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.ImportedVMAnnotation), modifyAnnotationNotAllowedForNonAdmin))
 	}
 
 	// The following annotations will be added by the mutation webhook upon VM creation.
 	if !reflect.DeepEqual(oldVM, &vmopv1.VirtualMachine{}) {
 		if vm.Annotations[constants.CreatedAtBuildVersionAnnotationKey] != oldVM.Annotations[constants.CreatedAtBuildVersionAnnotationKey] {
-			allErrs = append(allErrs, field.Forbidden(annotationPath.Child(constants.CreatedAtBuildVersionAnnotationKey), modifyAnnotationNotAllowedForNonAdmin))
+			allErrs = append(allErrs, field.Forbidden(annotationPath.Key(constants.CreatedAtBuildVersionAnnotationKey), modifyAnnotationNotAllowedForNonAdmin))
 		}
 
 		if vm.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] != oldVM.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] {
-			allErrs = append(allErrs, field.Forbidden(annotationPath.Child(constants.CreatedAtSchemaVersionAnnotationKey), modifyAnnotationNotAllowedForNonAdmin))
+			allErrs = append(allErrs, field.Forbidden(annotationPath.Key(constants.CreatedAtSchemaVersionAnnotationKey), modifyAnnotationNotAllowedForNonAdmin))
 		}
 	}