remove DeepEqual from backfilled volume validation (vmware-tanzu#1418)

Remove duplicate validation since validateVolume already validates
immutable fields for all volumes.

Author: zhangheliu

webhooks/virtualmachine/validation/virtualmachine_validator.go

@@ -1640,8 +1640,8 @@ func (v validator) validateInstanceStorageVolumes(
 	return allErrs
 }
 
-// validateBackfilledVolumesNotRemoved checks if any volumes backfilled from classic
-// disks have been removed from the spec when the AllDisksArePVCs feature is disabled.
+// validateBackfilledVolumesNotRemoved checks if any volumes backfilled from
+// classic disks have been removed or modified.
 func (v validator) validateBackfilledVolumesNotRemoved(
 	_ *pkgctx.WebhookRequestContext,
 	vm, oldVM *vmopv1.VirtualMachine,
@@ -1670,18 +1670,12 @@ func (v validator) validateBackfilledVolumesNotRemoved(
 			continue
 		}
 
-		newVol, exists := newVolumesMap[oldVol.Name]
+		// No need to validate immutable fields since they are validated in validateVolume.
+		_, exists := newVolumesMap[oldVol.Name]
 		if !exists {
 			allErrs = append(allErrs, field.Forbidden(
 				volumesPath,
 				fmt.Sprintf("%s: %s", oldVol.Name, removingBackfilledVolumeNotAllowed)))
-			continue
-		}
-
-		if !equality.Semantic.DeepEqual(*newVol, oldVol) {
-			allErrs = append(allErrs, field.Forbidden(
-				volumesPath,
-				fmt.Sprintf("%s: %s", oldVol.Name, modifyingBackfilledVolumeNotAllowed)))
 		}
 	}
 

webhooks/virtualmachine/validation/virtualmachine_validator_unit_test.go

@@ -7707,28 +7707,6 @@ func unitTestsValidateUpdate() { //nolint:gocyclo
 					validate:      doValidateWithMsg("spec.volumes: Forbidden: backfilled-vol: removing volume backfilled from classic disk is not allowed"),
 				},
 			),
-			Entry("should deny modifying a backfilled volume",
-				testParams{
-					setup: func(ctx *unitValidatingWebhookContext) {
-						// Create a backfilled volume in old VM
-						backfilledVol := vmopv1.VirtualMachineVolume{
-							Name:                "backfilled-vol",
-							ControllerType:      vmopv1.VirtualControllerTypeSCSI,
-							ControllerBusNumber: ptr.To(int32(0)),
-							UnitNumber:          ptr.To(int32(1)),
-							// No PersistentVolumeClaim
-						}
-						ctx.oldVM.Spec.Volumes = append(ctx.oldVM.Spec.Volumes, backfilledVol)
-
-						// Modify it in new VM (change unit number)
-						modifiedVol := backfilledVol.DeepCopy()
-						modifiedVol.UnitNumber = ptr.To(int32(2))
-						ctx.vm.Spec.Volumes = append(ctx.vm.Spec.Volumes, *modifiedVol)
-					},
-					expectAllowed: false,
-					validate:      doValidateWithMsg("spec.volumes: Forbidden: backfilled-vol: modifying backfilled volume is not allowed"),
-				},
-			),
 			Entry("should allow when backfilled volume is unchanged",
 				testParams{
 					setup: func(ctx *unitValidatingWebhookContext) {
@@ -7792,29 +7770,6 @@ func unitTestsValidateUpdate() { //nolint:gocyclo
 					expectAllowed: true,
 				},
 			),
-			Entry("should deny modifying ApplicationType of backfilled volume",
-				testParams{
-					setup: func(ctx *unitValidatingWebhookContext) {
-						// Create a backfilled volume in old VM
-						backfilledVol := vmopv1.VirtualMachineVolume{
-							Name:                "backfilled-vol",
-							ControllerType:      vmopv1.VirtualControllerTypeSCSI,
-							ControllerBusNumber: ptr.To(int32(0)),
-							UnitNumber:          ptr.To(int32(1)),
-							ApplicationType:     vmopv1.VolumeApplicationTypeOracleRAC,
-							// No PersistentVolumeClaim
-						}
-						ctx.oldVM.Spec.Volumes = append(ctx.oldVM.Spec.Volumes, backfilledVol)
-
-						// Change ApplicationType (but keep placement fields same)
-						modifiedVol := backfilledVol.DeepCopy()
-						modifiedVol.ApplicationType = vmopv1.VolumeApplicationTypeMicrosoftWSFC
-						ctx.vm.Spec.Volumes = append(ctx.vm.Spec.Volumes, *modifiedVol)
-					},
-					expectAllowed: false,
-					validate:      doValidateWithMsg("spec.volumes: Forbidden: backfilled-vol: modifying backfilled volume is not allowed"),
-				},
-			),
 		)
 	})