diff --git a/api/v1alpha3/virtualmachine_types.go b/api/v1alpha3/virtualmachine_types.go index f3cb45254..e6cd59ed9 100644 --- a/api/v1alpha3/virtualmachine_types.go +++ b/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 ( diff --git a/pkg/util/annotations/helpers.go b/pkg/util/annotations/helpers.go index 1eb21160a..4079170dd 100644 --- a/pkg/util/annotations/helpers.go +++ b/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) } diff --git a/pkg/util/annotations/helpers_test.go b/pkg/util/annotations/helpers_test.go index 702825244..7ffe2ba8d 100644 --- a/pkg/util/annotations/helpers_test.go +++ b/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), +) diff --git a/pkg/util/vmopv1/vm.go b/pkg/util/vmopv1/vm.go index b979e15f0..8a8d79bc3 100644 --- a/pkg/util/vmopv1/vm.go +++ b/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. diff --git a/pkg/util/vmopv1/vm_test.go b/pkg/util/vmopv1/vm_test.go index 8593aeecd..84c16335a 100644 --- a/pkg/util/vmopv1/vm_test.go +++ b/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 diff --git a/webhooks/virtualmachine/validation/virtualmachine_validator.go b/webhooks/virtualmachine/validation/virtualmachine_validator.go index 149f84bb2..7a6543d56 100644 --- a/webhooks/virtualmachine/validation/virtualmachine_validator.go +++ b/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)) } } diff --git a/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go b/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go index 3270eb59d..4c8f9996f 100644 --- a/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go +++ b/webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go @@ -48,6 +48,8 @@ const ( dummyFirstBootDoneVal = "dummy-first-boot-done" dummyCreatedAtBuildVersionVal = "dummy-created-at-build-version" dummyCreatedAtSchemaVersionVal = "dummy-created-at-schema-version" + dummyRegisteredAnnVal = "dummy-registered-annotation" + dummyImportedAnnVal = "dummy-imported-annotation" dummyPausedVMLabelVal = "dummy-devops" dummyVmiName = "vmi-dummy" dummyNamespaceName = "dummy-vm-namespace-for-webhook-validation" @@ -65,12 +67,10 @@ type testParams struct { func doValidateWithMsg(msgs ...string) func(admission.Response) { return func(response admission.Response) { - reasons := strings.Split(string(response.Result.Reason), ", ") + reasons := string(response.Result.Reason) for _, m := range msgs { - ExpectWithOffset(1, reasons).To(ContainElement(m)) + ExpectWithOffset(1, reasons).To(ContainSubstring(m)) } - // This may be overly strict in some cases but catches missed assertions. - ExpectWithOffset(1, reasons).To(HaveLen(len(msgs))) } } @@ -580,6 +580,23 @@ func unitTestsValidateCreate() { ), // FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled + + Entry("allow empty spec.image for privileged user when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled and VM contains restored annotation", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + ctx.IsPrivilegedAccount = true + ctx.vm.Annotations = map[string]string{ + vmopv1.RegisteredVMAnnotation: "", + } + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + }, + expectAllowed: true, + }, + ), Entry("allow empty spec.image for privileged user when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled", testParams{ setup: func(ctx *unitValidatingWebhookContext) { @@ -623,6 +640,24 @@ func unitTestsValidateCreate() { ), }, ), + Entry("forbid empty spec.image for unprivileged user when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled and annotation is present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + ctx.IsPrivilegedAccount = false + ctx.vm.Annotations = map[string]string{ + vmopv1.RegisteredVMAnnotation: "", + } + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + }, + validate: doValidateWithMsg( + field.Forbidden(field.NewPath("spec", "image"), "restricted to privileged users").Error(), + ), + }, + ), Entry("require spec.image.kind for unprivileged user when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled", testParams{ setup: func(ctx *unitValidatingWebhookContext) { @@ -676,7 +711,23 @@ func unitTestsValidateCreate() { // // FSS_WCP_MOBILITY_VM_IMPORT_NEW_NET is enabled // - Entry("allow empty spec.image for privileged user when FSS_WCP_MOBILITY_VM_IMPORT_NEW_NET is enabled", + Entry("allow empty spec.image for privileged user when FSS_WCP_MOBILITY_VM_IMPORT_NEW_NET is enabled and annotation is present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + ctx.IsPrivilegedAccount = true + ctx.vm.Annotations = map[string]string{ + vmopv1.ImportedVMAnnotation: "", + } + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMImportNewNet = true + }) + }, + expectAllowed: true, + }, + ), + Entry("(To be removed) allow empty spec.image for privileged user when FSS_WCP_MOBILITY_VM_IMPORT_NEW_NET is enabled, but the annotation is not present", testParams{ setup: func(ctx *unitValidatingWebhookContext) { ctx.vm.Spec.Image = nil @@ -719,6 +770,24 @@ func unitTestsValidateCreate() { ), }, ), + Entry("forbid empty spec.image for unprivileged user when FSS_WCP_MOBILITY_VM_IMPORT_NEW_NET is enabled and annotation is present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + ctx.IsPrivilegedAccount = false + ctx.vm.Annotations = map[string]string{ + vmopv1.ImportedVMAnnotation: "", + } + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMImportNewNet = true + }) + }, + validate: doValidateWithMsg( + field.Forbidden(field.NewPath("spec", "image"), "restricted to privileged users").Error(), + ), + }, + ), Entry("require spec.image.kind for unprivileged user when FSS_WCP_MOBILITY_VM_IMPORT_NEW_NET is enabled", testParams{ setup: func(ctx *unitValidatingWebhookContext) { @@ -918,10 +987,14 @@ func unitTestsValidateCreate() { setup: func(ctx *unitValidatingWebhookContext) { ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = dummyInstanceIDVal ctx.vm.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal + ctx.vm.Annotations[vmopv1.RegisteredVMAnnotation] = dummyFirstBootDoneVal + ctx.vm.Annotations[vmopv1.ImportedVMAnnotation] = dummyFirstBootDoneVal }, validate: doValidateWithMsg( - field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), - field.Forbidden(annotationPath.Child(vmopv1.FirstBootDoneAnnotation), "modifying this annotation is not allowed for non-admin users").Error()), + field.Forbidden(annotationPath.Key(vmopv1.RegisteredVMAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.ImportedVMAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.InstanceIDAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.FirstBootDoneAnnotation), "modifying this annotation is not allowed for non-admin users").Error()), }, ), Entry("should allow creating VM with admin-only annotations set by service user", @@ -931,6 +1004,8 @@ func unitTestsValidateCreate() { ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = dummyInstanceIDVal ctx.vm.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal + ctx.vm.Annotations[vmopv1.RegisteredVMAnnotation] = dummyFirstBootDoneVal + ctx.vm.Annotations[vmopv1.ImportedVMAnnotation] = dummyFirstBootDoneVal }, expectAllowed: true, }, @@ -948,6 +1023,8 @@ func unitTestsValidateCreate() { ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = dummyInstanceIDVal ctx.vm.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal + ctx.vm.Annotations[vmopv1.RegisteredVMAnnotation] = dummyFirstBootDoneVal + ctx.vm.Annotations[vmopv1.ImportedVMAnnotation] = dummyFirstBootDoneVal }, expectAllowed: true, }, @@ -2710,16 +2787,24 @@ func unitTestsValidateUpdate() { ctx.oldVM.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal ctx.oldVM.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal ctx.oldVM.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + ctx.oldVM.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + ctx.oldVM.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal + ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = dummyInstanceIDVal + updateSuffix ctx.vm.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal + updateSuffix ctx.vm.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal + updateSuffix ctx.vm.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + updateSuffix + ctx.vm.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + updateSuffix + ctx.vm.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal + updateSuffix }, validate: doValidateWithMsg( - field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), - field.Forbidden(annotationPath.Child(vmopv1.FirstBootDoneAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), - field.Forbidden(annotationPath.Child(constants.CreatedAtBuildVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error(), - field.Forbidden(annotationPath.Child(constants.CreatedAtSchemaVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error()), + field.Forbidden(annotationPath.Key(vmopv1.InstanceIDAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.FirstBootDoneAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(constants.CreatedAtBuildVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(constants.CreatedAtSchemaVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.RegisteredVMAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.ImportedVMAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + ), }, ), Entry("should disallow removing admin-only annotations by SSO user", @@ -2729,12 +2814,17 @@ func unitTestsValidateUpdate() { ctx.oldVM.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal ctx.oldVM.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal ctx.oldVM.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + ctx.oldVM.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + ctx.oldVM.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal }, validate: doValidateWithMsg( - field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), - field.Forbidden(annotationPath.Child(vmopv1.FirstBootDoneAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), - field.Forbidden(annotationPath.Child(constants.CreatedAtBuildVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error(), - field.Forbidden(annotationPath.Child(constants.CreatedAtSchemaVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error()), + field.Forbidden(annotationPath.Key(vmopv1.InstanceIDAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.FirstBootDoneAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(constants.CreatedAtBuildVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(constants.CreatedAtSchemaVersionAnnotationKey), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.RegisteredVMAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + field.Forbidden(annotationPath.Key(vmopv1.ImportedVMAnnotation), "modifying this annotation is not allowed for non-admin users").Error(), + ), }, ), Entry("should allow updating admin-only annotations by service user", @@ -2746,10 +2836,15 @@ func unitTestsValidateUpdate() { ctx.oldVM.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal ctx.oldVM.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal ctx.oldVM.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + ctx.oldVM.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + ctx.oldVM.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal + ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = dummyInstanceIDVal + updateSuffix ctx.vm.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal + updateSuffix ctx.vm.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal + updateSuffix ctx.vm.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + updateSuffix + ctx.vm.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + updateSuffix + ctx.vm.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal + updateSuffix }, expectAllowed: true, }, @@ -2763,6 +2858,8 @@ func unitTestsValidateUpdate() { ctx.oldVM.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal ctx.oldVM.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal ctx.oldVM.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + ctx.oldVM.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + ctx.oldVM.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal }, expectAllowed: true, }, @@ -2784,10 +2881,15 @@ func unitTestsValidateUpdate() { ctx.oldVM.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal ctx.oldVM.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal ctx.oldVM.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + ctx.oldVM.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + ctx.oldVM.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal + ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = dummyInstanceIDVal + updateSuffix ctx.vm.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal + updateSuffix ctx.vm.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal + updateSuffix ctx.vm.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + updateSuffix + ctx.vm.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + updateSuffix + ctx.vm.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal + updateSuffix }, expectAllowed: true, }, @@ -2809,6 +2911,8 @@ func unitTestsValidateUpdate() { ctx.oldVM.Annotations[vmopv1.FirstBootDoneAnnotation] = dummyFirstBootDoneVal ctx.oldVM.Annotations[constants.CreatedAtBuildVersionAnnotationKey] = dummyCreatedAtBuildVersionVal ctx.oldVM.Annotations[constants.CreatedAtSchemaVersionAnnotationKey] = dummyCreatedAtSchemaVersionVal + ctx.oldVM.Annotations[vmopv1.RegisteredVMAnnotation] = dummyRegisteredAnnVal + ctx.oldVM.Annotations[vmopv1.ImportedVMAnnotation] = dummyImportedAnnVal }, expectAllowed: true, }, @@ -2861,6 +2965,214 @@ func unitTestsValidateUpdate() { ) }) + Context("Image and ImageName", func() { + DescribeTable("imageName", doTest, + Entry("forbid changing imageName to non empty value", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.oldVM.Spec.ImageName = dummyVmiName + + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.ImageName = dummyVmiName + updateSuffix + }, + validate: doValidateWithMsg( + fmt.Sprintf(`spec.imageName: Invalid value: "%v": field is immutable`, dummyVmiName+updateSuffix)), + }, + ), + + Entry("forbid unset of imageName if FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is disabled", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.oldVM.Spec.ImageName = dummyVmiName + + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.ImageName = "" + ctx.vm.Spec.Image = nil + }, + validate: doValidateWithMsg( + `spec.imageName: Invalid value: "": field is immutable`), + }, + ), + + // FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled + Entry("forbid unset of imageName if FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled, but annotation is not present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + + ctx.oldVM.Spec.ImageName = dummyVmiName + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.ImageName = "" + ctx.vm.Spec.Image = nil + }, + validate: doValidateWithMsg( + fmt.Sprintf(`spec.imageName: Invalid value: "%v": field is immutable`, "")), + }, + ), + + Entry("forbid unset of imageName by unprivileged users if FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled and registered annotation is present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + ctx.IsPrivilegedAccount = false + ctx.oldVM.Annotations = map[string]string{ + vmopv1.RegisteredVMAnnotation: "foo", + } + + ctx.oldVM.Spec.ImageName = dummyVmiName + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.ImageName = "" + ctx.vm.Spec.Image = nil + }, + validate: doValidateWithMsg( + field.Forbidden(field.NewPath("spec", "imageName"), "restricted to privileged users").Error()), + }, + ), + + Entry("allow unset of imageName for privileged users when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled, and registered annotation is present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + + ctx.IsPrivilegedAccount = true + + ctx.oldVM.Spec.ImageName = dummyVmiName + ctx.oldVM.Annotations = map[string]string{ + vmopv1.RegisteredVMAnnotation: "foo", + } + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.ImageName = "" + ctx.vm.Spec.Image = nil + }, + expectAllowed: true, + }, + ), + ) + + DescribeTable("image", doTest, + Entry("forbid changing image from nil to a non-nil value", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.oldVM.Spec.Image = nil + + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.Image = &vmopv1.VirtualMachineImageRef{ + Name: dummyVmiName + updateSuffix, + } + }, + validate: doValidateWithMsg( + field.Invalid(field.NewPath("spec", "image"), &vmopv1.VirtualMachineImageRef{Name: dummyVmiName + updateSuffix}, apivalidation.FieldImmutableErrorMsg).Error()), + }, + ), + Entry("forbid changing image from non-nil to non-nil value", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.oldVM.Spec.Image = &vmopv1.VirtualMachineImageRef{ + Name: dummyVmiName, + } + + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.Image = &vmopv1.VirtualMachineImageRef{ + Name: dummyVmiName + updateSuffix, + } + }, + validate: doValidateWithMsg( + field.Invalid(field.NewPath("spec", "image"), &vmopv1.VirtualMachineImageRef{Name: dummyVmiName + updateSuffix}, apivalidation.FieldImmutableErrorMsg).Error()), + }, + ), + + Entry("forbid unset of image when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is disabled", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + ctx.oldVM.Spec.Image = &vmopv1.VirtualMachineImageRef{ + Name: dummyVmiName, + } + + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + }, + validate: doValidateWithMsg( + field.Invalid(field.NewPath("spec", "image"), nil, apivalidation.FieldImmutableErrorMsg).Error()), + }, + ), + + Entry("forbid unset of image for privileged users when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled, but Registered annotation is not present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + + ctx.IsPrivilegedAccount = true + + ctx.oldVM.Spec.Image = &vmopv1.VirtualMachineImageRef{ + Name: dummyVmiName, + } + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + }, + validate: doValidateWithMsg( + field.Invalid(field.NewPath("spec", "image"), nil, apivalidation.FieldImmutableErrorMsg).Error()), + }, + ), + + Entry("forbid unset of image by unprivileged users when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled and the Registered annotation is present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + ctx.IsPrivilegedAccount = false + + ctx.oldVM.Annotations = map[string]string{ + vmopv1.RegisteredVMAnnotation: "bar", + } + + ctx.oldVM.Spec.Image = &vmopv1.VirtualMachineImageRef{ + Name: dummyVmiName, + } + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + }, + validate: doValidateWithMsg( + field.Forbidden(field.NewPath("spec", "image"), "restricted to privileged users").Error()), + }, + ), + + Entry("alow changing image for privileged users when FSS_WCP_VMSERVICE_INCREMENTAL_RESTORE is enabled, and Registered annotation is present", + testParams{ + setup: func(ctx *unitValidatingWebhookContext) { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.Features.VMIncrementalRestore = true + }) + + ctx.IsPrivilegedAccount = true + ctx.oldVM.Annotations = map[string]string{ + vmopv1.RegisteredVMAnnotation: "bar", + } + + ctx.oldVM.Spec.Image = &vmopv1.VirtualMachineImageRef{ + Name: dummyVmiName, + } + ctx.vm = ctx.oldVM.DeepCopy() + ctx.vm.Spec.Image = nil + ctx.vm.Spec.ImageName = "" + }, + expectAllowed: true, + }, + ), + ) + }) + Context("ClassName", func() { DescribeTable("class name", doTest,