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 annotations that
will be applied on the Imported, and 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,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),
+)

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 f := pkgcfg.FromContext(ctx).Features; f.VMIncrementalRestore {
+		if annotations.HasRegisterVM(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
@@ -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.