From 9869e8120a698f391a18c12da1a88cedf965ead5 Mon Sep 17 00:00:00 2001 From: Bryan Venteicher Date: Wed, 5 Jul 2023 10:37:54 -0500 Subject: [PATCH] Add v1a2 webhooks This is the first round of v1a2 webhooks. At present, only unit tests are hooked up on main as integration tests require manifest generation changes that we're not ready for since the integration tests use the same artifacts that are shipped. Fix golangci-lint config to allow aliased imports --- .golangci.yml | 2 - test/builder/fake.go | 9 +- test/builder/utila2.go | 174 +++++ .../mutation/virtualmachine_mutator.go | 144 ++++ .../virtualmachine_mutator_intg_test.go | 151 ++++ .../virtualmachine_mutator_suite_test.go | 43 ++ .../virtualmachine_mutator_unit_test.go | 225 ++++++ .../v1alpha2/validation/helpers.go | 24 + .../validation/virtualmachine_validator.go | 680 ++++++++++++++++++ .../virtualmachine_validator_intg_test.go | 222 ++++++ .../virtualmachine_validator_suite_test.go | 28 + .../virtualmachine_validator_unit_test.go | 557 ++++++++++++++ webhooks/virtualmachine/v1alpha2/webhooks.go | 23 + webhooks/virtualmachine/webhooks.go | 10 +- .../mutation/virtualmachineclass_mutator.go | 116 +++ .../virtualmachineclass_mutator_intg_test.go | 58 ++ .../virtualmachineclass_mutator_suite_test.go | 28 + .../virtualmachineclass_mutator_unit_test.go | 89 +++ .../virtualmachineclass_validator.go | 131 ++++ ...virtualmachineclass_validator_intg_test.go | 102 +++ ...irtualmachineclass_validator_suite_test.go | 28 + ...virtualmachineclass_validator_unit_test.go | 223 ++++++ .../virtualmachineclass/v1alpha2/webhooks.go | 23 + .../virtualmachinepublishrequest_validator.go | 183 +++++ ...chinepublishrequest_validator_intg_test.go | 158 ++++ ...hinepublishrequest_validator_suite_test.go | 28 + ...chinepublishrequest_validator_unit_test.go | 245 +++++++ .../v1alpha2/webhooks.go | 19 + .../mutation/virtualmachineservice_mutator.go | 89 +++ ...virtualmachineservice_mutator_intg_test.go | 60 ++ ...irtualmachineservice_mutator_suite_test.go | 28 + ...virtualmachineservice_mutator_unit_test.go | 58 ++ .../virtualmachineservice_validator.go | 298 ++++++++ ...rtualmachineservice_validator_intg_test.go | 126 ++++ ...tualmachineservice_validator_suite_test.go | 28 + ...rtualmachineservice_validator_unit_test.go | 306 ++++++++ .../v1alpha2/webhooks.go | 23 + ...rtualmachinesetresourcepolicy_validator.go | 180 +++++ ...nesetresourcepolicy_validator_intg_test.go | 135 ++++ ...esetresourcepolicy_validator_suite_test.go | 28 + ...nesetresourcepolicy_validator_unit_test.go | 205 ++++++ .../v1alpha2/webhooks.go | 20 + 42 files changed, 5299 insertions(+), 8 deletions(-) create mode 100644 test/builder/utila2.go create mode 100644 webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go create mode 100644 webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_intg_test.go create mode 100644 webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_suite_test.go create mode 100644 webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go create mode 100644 webhooks/virtualmachine/v1alpha2/validation/helpers.go create mode 100644 webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go create mode 100644 webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go create mode 100644 webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_suite_test.go create mode 100644 webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go create mode 100644 webhooks/virtualmachine/v1alpha2/webhooks.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_intg_test.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_suite_test.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_unit_test.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_intg_test.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_suite_test.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_unit_test.go create mode 100644 webhooks/virtualmachineclass/v1alpha2/webhooks.go create mode 100644 webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator.go create mode 100644 webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_intg_test.go create mode 100644 webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_suite_test.go create mode 100644 webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_unit_test.go create mode 100644 webhooks/virtualmachinepublishrequest/v1alpha2/webhooks.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_intg_test.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_suite_test.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_unit_test.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_intg_test.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_suite_test.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_unit_test.go create mode 100644 webhooks/virtualmachineservice/v1alpha2/webhooks.go create mode 100644 webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator.go create mode 100644 webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_intg_test.go create mode 100644 webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_suite_test.go create mode 100644 webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_unit_test.go create mode 100644 webhooks/virtualmachinesetresourcepolicy/v1alpha2/webhooks.go diff --git a/.golangci.yml b/.golangci.yml index c2b975c4c..8589e7656 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,9 +14,7 @@ linters-settings: # it's a comma-separated list of prefixes local-prefixes: github.com/vmware-tanzu importas: - no-unaliased: true alias: - # Kubernetes - pkg: k8s.io/api/core/v1 alias: corev1 - pkg: github.com/vmware-tanzu/vm-operator/api/v1alpha1 diff --git a/test/builder/fake.go b/test/builder/fake.go index 53917807d..a0e213e9a 100644 --- a/test/builder/fake.go +++ b/test/builder/fake.go @@ -11,13 +11,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" imgregv1a1 "github.com/vmware-tanzu/image-registry-operator-api/api/v1alpha1" - - vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" - ncpv1alpha1 "github.com/vmware-tanzu/vm-operator/external/ncp/api/v1alpha1" netopv1alpha1 "github.com/vmware-tanzu/vm-operator/external/net-operator/api/v1alpha1" topologyv1 "github.com/vmware-tanzu/vm-operator/external/tanzu-topology/api/v1alpha1" cnsv1alpha1 "github.com/vmware-tanzu/vm-operator/external/vsphere-csi-driver/pkg/syncer/cnsoperator/apis/cnsnodevmattachment/v1alpha1" + + "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + "github.com/vmware-tanzu/vm-operator/api/v1alpha2" "github.com/vmware-tanzu/vm-operator/pkg/record" ) @@ -35,7 +35,8 @@ func NewFakeRecorder() (record.Recorder, chan string) { func NewScheme() *runtime.Scheme { scheme := runtime.NewScheme() _ = clientgoscheme.AddToScheme(scheme) - _ = vmopv1.AddToScheme(scheme) + _ = v1alpha1.AddToScheme(scheme) + _ = v1alpha2.AddToScheme(scheme) _ = ncpv1alpha1.AddToScheme(scheme) _ = cnsv1alpha1.AddToScheme(scheme) _ = netopv1alpha1.AddToScheme(scheme) diff --git a/test/builder/utila2.go b/test/builder/utila2.go new file mode 100644 index 000000000..fd3fb711c --- /dev/null +++ b/test/builder/utila2.go @@ -0,0 +1,174 @@ +// Copyright (c) 2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package builder + +import ( + "fmt" + + "github.com/google/uuid" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" +) + +func DummyVirtualMachineSetResourcePolicyA2() *vmopv1.VirtualMachineSetResourcePolicy { + return &vmopv1.VirtualMachineSetResourcePolicy{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + }, + Spec: vmopv1.VirtualMachineSetResourcePolicySpec{ + ResourcePool: vmopv1.ResourcePoolSpec{ + Name: "dummy-resource-pool", + Reservations: vmopv1.VirtualMachineResourceSpec{ + Cpu: resource.MustParse("1Gi"), + Memory: resource.MustParse("2Gi"), + }, + Limits: vmopv1.VirtualMachineResourceSpec{ + Cpu: resource.MustParse("2Gi"), + Memory: resource.MustParse("4Gi"), + }, + }, + Folder: "dummy-folder", + ClusterModuleGroups: []string{"dummy-cluster-modules"}, + }, + } +} + +func DummyVirtualMachineServiceA2() *vmopv1.VirtualMachineService { + return &vmopv1.VirtualMachineService{ + ObjectMeta: metav1.ObjectMeta{ + // Using image.GenerateName causes problems with unit tests + Name: fmt.Sprintf("test-%s", uuid.New()), + }, + Spec: vmopv1.VirtualMachineServiceSpec{ + Type: vmopv1.VirtualMachineServiceTypeLoadBalancer, + Ports: []vmopv1.VirtualMachineServicePort{ + { + Name: "dummy-port", + Protocol: "TCP", + Port: 42, + TargetPort: 4242, + }, + }, + Selector: map[string]string{ + "foo": "bar", + }, + }, + } +} + +func DummyVirtualMachineA2() *vmopv1.VirtualMachine { + return &vmopv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Labels: map[string]string{}, + Annotations: map[string]string{}, + }, + Spec: vmopv1.VirtualMachineSpec{ + ImageName: DummyImageName, + ClassName: DummyClassName, + PowerState: vmopv1.VirtualMachinePowerStateOn, + Volumes: []vmopv1.VirtualMachineVolume{ + { + Name: DummyVolumeName, + VirtualMachineVolumeSource: vmopv1.VirtualMachineVolumeSource{ + PersistentVolumeClaim: &vmopv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: DummyPVCName, + }, + }, + }, + }, + }, + }, + } +} + +func DummyVirtualMachinePublishRequestA2(name, namespace, sourceName, itemName, clName string) *vmopv1.VirtualMachinePublishRequest { + return &vmopv1.VirtualMachinePublishRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Finalizers: []string{"virtualmachinepublishrequest.vmoperator.vmware.com"}, + }, + Spec: vmopv1.VirtualMachinePublishRequestSpec{ + Source: vmopv1.VirtualMachinePublishRequestSource{ + Name: sourceName, + APIVersion: "vmoperator.vmware.com/v1alpha2", + Kind: "VirtualMachine", + }, + Target: vmopv1.VirtualMachinePublishRequestTarget{ + Item: vmopv1.VirtualMachinePublishRequestTargetItem{ + Name: itemName, + }, + Location: vmopv1.VirtualMachinePublishRequestTargetLocation{ + Name: clName, + APIVersion: "imageregistry.vmware.com/v1alpha1", + Kind: "ContentLibrary", + }, + }, + }, + } +} + +func DummyVirtualMachineClassA2() *vmopv1.VirtualMachineClass { + return &vmopv1.VirtualMachineClass{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + }, + Spec: vmopv1.VirtualMachineClassSpec{ + Hardware: vmopv1.VirtualMachineClassHardware{ + Cpus: int64(2), + Memory: resource.MustParse("4Gi"), + }, + Policies: vmopv1.VirtualMachineClassPolicies{ + Resources: vmopv1.VirtualMachineClassResources{ + Requests: vmopv1.VirtualMachineResourceSpec{ + Cpu: resource.MustParse("1Gi"), + Memory: resource.MustParse("2Gi"), + }, + Limits: vmopv1.VirtualMachineResourceSpec{ + Cpu: resource.MustParse("2Gi"), + Memory: resource.MustParse("4Gi"), + }, + }, + }, + }, + } +} + +func DummyInstanceStorageVirtualMachineVolumesA2() []vmopv1.VirtualMachineVolume { + return []vmopv1.VirtualMachineVolume{ + { + Name: "instance-pvc-1", + VirtualMachineVolumeSource: vmopv1.VirtualMachineVolumeSource{ + PersistentVolumeClaim: &vmopv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "instance-pvc-1", + }, + InstanceVolumeClaim: &vmopv1.InstanceVolumeClaimVolumeSource{ + StorageClass: DummyStorageClassName, + Size: resource.MustParse("256Gi"), + }, + }, + }, + }, + { + Name: "instance-pvc-2", + VirtualMachineVolumeSource: vmopv1.VirtualMachineVolumeSource{ + PersistentVolumeClaim: &vmopv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "instance-pvc-2", + }, + InstanceVolumeClaim: &vmopv1.InstanceVolumeClaimVolumeSource{ + StorageClass: DummyStorageClassName, + Size: resource.MustParse("512Gi"), + }, + }, + }, + }, + } +} diff --git a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go new file mode 100644 index 000000000..ea3ed8430 --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator.go @@ -0,0 +1,144 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation + +import ( + "encoding/json" + "net/http" + "reflect" + "strings" + "time" + + "github.com/pkg/errors" + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" +) + +const ( + webHookName = "default" +) + +// -kubebuilder:webhook:path=/default-mutate-vmoperator-vmware-com-v1alpha2-virtualmachine,mutating=true,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachines,verbs=create;update,versions=v1alpha2,name=default.mutating.virtualmachine.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachine,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachine/status,verbs=get + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewMutatingWebhook(ctx, mgr, webHookName, NewMutator(mgr.GetClient())) + if err != nil { + return errors.Wrapf(err, "failed to create mutation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewMutator returns the package's Mutator. +func NewMutator(client client.Client) builder.Mutator { + return mutator{ + client: client, + converter: runtime.DefaultUnstructuredConverter, + } +} + +type mutator struct { + client client.Client + converter runtime.UnstructuredConverter +} + +func (m mutator) Mutate(ctx *context.WebhookRequestContext) admission.Response { + if ctx.Op == admissionv1.Delete { + return admission.Allowed("") + } + + vm, err := m.vmFromUnstructured(ctx.Obj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + var wasMutated bool + original := vm + modified := original.DeepCopy() + + //nolint:gocritic + switch ctx.Op { + case admissionv1.Update: + oldVM, err := m.vmFromUnstructured(ctx.OldObj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + if ok, err := SetNextRestartTime(ctx, modified, oldVM); err != nil { + return admission.Denied(err.Error()) + } else if ok { + wasMutated = true + } + } + + if !wasMutated { + return admission.Allowed("") + } + + rawOriginal, err := json.Marshal(original) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + rawModified, err := json.Marshal(modified) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + return admission.PatchResponseFromRaw(rawOriginal, rawModified) +} + +func (m mutator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachine{}).Name()) +} + +// vmFromUnstructured returns the VirtualMachine from the unstructured object. +func (m mutator) vmFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachine, error) { + vm := &vmopv1.VirtualMachine{} + if err := m.converter.FromUnstructured(obj.UnstructuredContent(), vm); err != nil { + return nil, err + } + return vm, nil +} + +// SetNextRestartTime sets spec.nextRestartTime for a VM if the field's +// current value is equal to "now" (case-insensitive). +// Return true if set, otherwise false. +func SetNextRestartTime( + ctx *context.WebhookRequestContext, + newVM, oldVM *vmopv1.VirtualMachine) (bool, error) { + + if newVM.Spec.NextRestartTime == "" { + newVM.Spec.NextRestartTime = oldVM.Spec.NextRestartTime + return oldVM.Spec.NextRestartTime != "", nil + } + if strings.EqualFold("now", newVM.Spec.NextRestartTime) { + if oldVM.Spec.PowerState != vmopv1.VirtualMachinePowerStateOn { + return false, field.Invalid( + field.NewPath("spec", "nextRestartTime"), + newVM.Spec.NextRestartTime, + "can only restart powered on vm") + } + newVM.Spec.NextRestartTime = time.Now().UTC().Format(time.RFC3339Nano) + return true, nil + } + if newVM.Spec.NextRestartTime == oldVM.Spec.NextRestartTime { + return false, nil + } + return false, field.Invalid( + field.NewPath("spec", "nextRestartTime"), + newVM.Spec.NextRestartTime, + `may only be set to "now"`) +} diff --git a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_intg_test.go b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_intg_test.go new file mode 100644 index 000000000..c3b02546b --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_intg_test.go @@ -0,0 +1,151 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + Describe("Invoking Mutation", intgTestsMutating) +} + +type intgMutatingWebhookContext struct { + builder.IntegrationTestContext + vm *vmopv1.VirtualMachine +} + +func newIntgMutatingWebhookContext() *intgMutatingWebhookContext { + ctx := &intgMutatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vm = builder.DummyVirtualMachineA2() + ctx.vm.Namespace = ctx.Namespace + + return ctx +} + +func intgTestsMutating() { + var ( + ctx *intgMutatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgMutatingWebhookContext() + }) + + AfterEach(func() { + ctx = nil + }) + + Context("SetNextRestartTime", func() { + When("create a VM", func() { + When("spec.nextRestartTime is empty", func() { + It("should not mutate", func() { + ctx.vm.Spec.NextRestartTime = "" + Expect(ctx.Client.Create(ctx, ctx.vm)).To(Succeed()) + modified := &vmopv1.VirtualMachine{} + Expect(ctx.Client.Get(ctx, client.ObjectKeyFromObject(ctx.vm), modified)).Should(Succeed()) + Expect(modified.Spec.NextRestartTime).Should(BeEmpty()) + }) + }) + When("spec.nextRestartTime is not empty", func() { + It("should not mutate", func() { + ctx.vm.Spec.NextRestartTime = "hello" + Expect(ctx.Client.Create(ctx, ctx.vm)).To(Succeed()) + modified := &vmopv1.VirtualMachine{} + Expect(ctx.Client.Get(ctx, client.ObjectKeyFromObject(ctx.vm), modified)).Should(Succeed()) + Expect(modified.Spec.NextRestartTime).Should(Equal("hello")) + }) + }) + }) + + When("updating VirtualMachine", func() { + var ( + lastRestartTime time.Time + modified *vmopv1.VirtualMachine + ) + BeforeEach(func() { + lastRestartTime = time.Now().UTC() + modified = &vmopv1.VirtualMachine{} + }) + JustBeforeEach(func() { + Expect(ctx.Client.Create(ctx, ctx.vm)).To(Succeed()) + Expect(ctx.Client.Get(ctx, client.ObjectKeyFromObject(ctx.vm), modified)).Should(Succeed()) + }) + When("spec.nextRestartTime is empty", func() { + BeforeEach(func() { + ctx.vm.Spec.NextRestartTime = "" + }) + When("spec.nextRestartTime is empty", func() { + It("should not mutate", func() { + modified.Spec.NextRestartTime = "" + Expect(ctx.Client.Update(ctx, modified)).To(Succeed()) + Expect(ctx.Client.Get(ctx, client.ObjectKeyFromObject(ctx.vm), modified)).Should(Succeed()) + Expect(modified.Spec.NextRestartTime).Should(BeEmpty()) + }) + }) + When("spec.nextRestartTime is now", func() { + It("should mutate", func() { + modified.Spec.NextRestartTime = "Now" + Expect(ctx.Client.Update(ctx, modified)).To(Succeed()) + Expect(ctx.Client.Get(ctx, client.ObjectKeyFromObject(ctx.vm), modified)).Should(Succeed()) + Expect(modified.Spec.NextRestartTime).ShouldNot(BeEmpty()) + _, err := time.Parse(time.RFC3339Nano, modified.Spec.NextRestartTime) + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) + When("existing spec.nextRestartTime is not empty", func() { + BeforeEach(func() { + ctx.vm.Spec.NextRestartTime = lastRestartTime.Format(time.RFC3339Nano) + }) + When("spec.nextRestartTime is empty", func() { + It("should mutate to original value", func() { + modified.Spec.NextRestartTime = "" + Expect(ctx.Client.Update(ctx, modified)).To(Succeed()) + Expect(ctx.Client.Get(ctx, client.ObjectKeyFromObject(ctx.vm), modified)).Should(Succeed()) + Expect(modified.Spec.NextRestartTime).Should(Equal(ctx.vm.Spec.NextRestartTime)) + }) + }) + When("spec.nextRestartTime is now", func() { + It("should mutate", func() { + modified.Spec.NextRestartTime = "Now" + Expect(ctx.Client.Update(ctx, modified)).To(Succeed()) + Expect(ctx.Client.Get(ctx, client.ObjectKeyFromObject(ctx.vm), modified)).Should(Succeed()) + Expect(modified.Spec.NextRestartTime).ShouldNot(BeEmpty()) + nextRestartTime, err := time.Parse(time.RFC3339Nano, modified.Spec.NextRestartTime) + Expect(err).ToNot(HaveOccurred()) + Expect(lastRestartTime.Before(nextRestartTime)).To(BeTrue()) + }) + }) + DescribeTable( + `spec.nextRestartTime is a non-empty value that is not "now"`, + func(nextRestartTime string) { + modified.Spec.NextRestartTime = nextRestartTime + err := ctx.Client.Update(ctx, modified) + Expect(err).To(HaveOccurred()) + expectedErr := field.Invalid( + field.NewPath("spec", "nextRestartTime"), + nextRestartTime, + `may only be set to "now"`) + Expect(err.Error()).To(ContainSubstring(expectedErr.Error())) + }, + newInvalidNextRestartTimeTableEntries("should return an invalid field error")..., + ) + }) + }) + }) +} diff --git a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_suite_test.go b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_suite_test.go new file mode 100644 index 000000000..9addb7ee7 --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_suite_test.go @@ -0,0 +1,43 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + "github.com/onsi/ginkgo/extensions/table" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha2/mutation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForMutatingWebhook( + mutation.AddToManager, + mutation.NewMutator, + "default.mutating.virtualmachine.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Mutating webhook suite", nil /*intgTests*/, uniTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) + +func newInvalidNextRestartTimeTableEntries(description string) []table.TableEntry { + return []table.TableEntry{ + table.Entry(description, "5m"), + table.Entry(description, "1s"), + table.Entry(description, "2h45m"), + table.Entry(description, "1.5h"), + table.Entry(description, "2023-06-01T13:00:00Z"), + table.Entry(description, "2023-06-01T13:00:00-06:00"), + table.Entry(description, "2023-06-01T13:00:00+05:30"), + table.Entry(description, "hello"), + table.Entry(description, "world"), + } +} diff --git a/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go new file mode 100644 index 000000000..77eecc8cd --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/mutation/virtualmachine_mutator_unit_test.go @@ -0,0 +1,225 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha2/mutation" +) + +func uniTests() { + Describe("Invoking Mutate", unitTestsMutating) +} + +type unitMutationWebhookContext struct { + builder.UnitTestContextForMutatingWebhook + vm *vmopv1.VirtualMachine +} + +func newUnitTestContextForMutatingWebhook() *unitMutationWebhookContext { + vm := builder.DummyVirtualMachineA2() + obj, err := builder.ToUnstructured(vm) + Expect(err).ToNot(HaveOccurred()) + + return &unitMutationWebhookContext{ + UnitTestContextForMutatingWebhook: *suite.NewUnitTestContextForMutatingWebhook(obj), + vm: vm, + } +} + +func unitTestsMutating() { + var ( + ctx *unitMutationWebhookContext + ) + + BeforeEach(func() { + ctx = newUnitTestContextForMutatingWebhook() + }) + + AfterEach(func() { + ctx = nil + }) + + Describe("VirtualMachineMutator should admit updates when object is under deletion", func() { + Context("when update request comes in while deletion in progress ", func() { + It("should admit update operation", func() { + t := metav1.Now() + ctx.WebhookRequestContext.Obj.SetDeletionTimestamp(&t) + response := ctx.Mutate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(BeTrue()) + }) + }) + }) + + Describe("SetNextRestartTime", func() { + + var ( + oldVM *vmopv1.VirtualMachine + ) + + BeforeEach(func() { + oldVM = ctx.vm.DeepCopy() + }) + + When("oldVM has empty spec.nextRestartTime", func() { + BeforeEach(func() { + oldVM.Spec.NextRestartTime = "" + }) + Context("newVM has spec.nextRestartTime set to an empty value", func() { + It("should not mutate anything", func() { + ctx.vm.Spec.NextRestartTime = "" + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeFalse()) + Expect(err).ToNot(HaveOccurred()) + Expect(ctx.vm.Spec.NextRestartTime).To(BeEmpty()) + }) + }) + Context("newVM has spec.nextRestartTime set to 'now' (case-insensitive)", func() { + Context("vm is powered on", func() { + BeforeEach(func() { + oldVM.Spec.PowerState = vmopv1.VirtualMachinePowerStateOn + }) + It("should mutate the field to a valid UTC timestamp", func() { + for _, s := range []string{"now", "Now", "NOW"} { + ctx.vm.Spec.NextRestartTime = s + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeTrue()) + Expect(err).ToNot(HaveOccurred()) + Expect(ctx.vm.Spec.NextRestartTime).ToNot(BeEmpty()) + _, err = time.Parse(time.RFC3339Nano, ctx.vm.Spec.NextRestartTime) + Expect(err).ShouldNot(HaveOccurred()) + } + }) + }) + Context("vm is powered off", func() { + BeforeEach(func() { + oldVM.Spec.PowerState = vmopv1.VirtualMachinePowerStateOff + }) + It("should return an error", func() { + ctx.vm.Spec.NextRestartTime = "now" + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeFalse()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(field.Invalid( + field.NewPath("spec", "nextRestartTime"), + "now", + "can only restart powered on vm").Error())) + }) + }) + Context("vm is suspended", func() { + BeforeEach(func() { + oldVM.Spec.PowerState = vmopv1.VirtualMachinePowerStateSuspended + }) + It("should return an error", func() { + ctx.vm.Spec.NextRestartTime = "now" + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeFalse()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(field.Invalid( + field.NewPath("spec", "nextRestartTime"), + "now", + "can only restart powered on vm").Error())) + }) + }) + }) + DescribeTable( + `newVM has spec.nextRestartTime set a non-empty value that is not "now"`, + func(nextRestartTime string) { + ctx.vm.Spec.NextRestartTime = nextRestartTime + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeFalse()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(field.Invalid( + field.NewPath("spec", "nextRestartTime"), + nextRestartTime, + `may only be set to "now"`).Error())) + Expect(ctx.vm.Spec.NextRestartTime).To(Equal(nextRestartTime)) + }, + newInvalidNextRestartTimeTableEntries("should return an invalid field error")..., + ) + }) + + When("oldVM has non-empty spec.nextRestartTime", func() { + var ( + lastRestartTime time.Time + ) + BeforeEach(func() { + lastRestartTime = time.Now().UTC() + oldVM.Spec.NextRestartTime = lastRestartTime.Format(time.RFC3339Nano) + }) + Context("newVM has spec.nextRestartTime set to an empty value", func() { + It("should mutate to match oldVM", func() { + ctx.vm.Spec.NextRestartTime = "" + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeTrue()) + Expect(err).ToNot(HaveOccurred()) + Expect(ctx.vm.Spec.NextRestartTime).To(Equal(oldVM.Spec.NextRestartTime)) + }) + }) + Context("newVM has spec.nextRestartTime set to 'now' (case-insensitive)", func() { + It("should mutate the field to a valid UTC timestamp", func() { + for _, s := range []string{"now", "Now", "NOW"} { + ctx.vm.Spec.NextRestartTime = s + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeTrue()) + Expect(err).ToNot(HaveOccurred()) + Expect(ctx.vm.Spec.NextRestartTime).ToNot(BeEmpty()) + nextRestartTime, err := time.Parse(time.RFC3339Nano, ctx.vm.Spec.NextRestartTime) + Expect(err).ShouldNot(HaveOccurred()) + Expect(lastRestartTime.Before(nextRestartTime)).To(BeTrue()) + } + }) + }) + DescribeTable( + `newVM has spec.nextRestartTime set a non-empty value that is not "now"`, + func(nextRestartTime string) { + ctx.vm.Spec.NextRestartTime = nextRestartTime + ok, err := mutation.SetNextRestartTime( + &ctx.WebhookRequestContext, + ctx.vm, + oldVM) + Expect(ok).To(BeFalse()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(field.Invalid( + field.NewPath("spec", "nextRestartTime"), + nextRestartTime, + `may only be set to "now"`).Error())) + Expect(ctx.vm.Spec.NextRestartTime).To(Equal(nextRestartTime)) + }, + newInvalidNextRestartTimeTableEntries("should return an invalid field error")..., + ) + }) + }) +} diff --git a/webhooks/virtualmachine/v1alpha2/validation/helpers.go b/webhooks/virtualmachine/v1alpha2/validation/helpers.go new file mode 100644 index 000000000..3368e7ead --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/validation/helpers.go @@ -0,0 +1,24 @@ +// Copyright (c) 2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + +// This file contains v1a2 related changes in other packages that haven't been +// committed yet. + +func cNSAttachmentNameForVolume(vmName string, volumeName string) string { + return vmName + "-" + volumeName +} + +func filterVolumesA2(vm *v1alpha2.VirtualMachine) []v1alpha2.VirtualMachineVolume { + var volumes []v1alpha2.VirtualMachineVolume + for _, vol := range vm.Spec.Volumes { + if pvc := vol.PersistentVolumeClaim; pvc != nil && pvc.InstanceVolumeClaim != nil { + volumes = append(volumes, vol) + } + } + + return volumes +} diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go new file mode 100644 index 000000000..183060626 --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go @@ -0,0 +1,680 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "fmt" + "net" + "net/http" + "reflect" + "strconv" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/pkg/errors" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/api/v1alpha2/cloudinit" + "github.com/vmware-tanzu/vm-operator/api/v1alpha2/sysprep" + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/pkg/lib" + "github.com/vmware-tanzu/vm-operator/pkg/topology" + "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/config" + "github.com/vmware-tanzu/vm-operator/webhooks/common" +) + +const ( + webHookName = "default" + storageResourceQuotaStrPattern = ".storageclass.storage.k8s.io/" + isRestrictedNetworkKey = "IsRestrictedNetwork" + allowedRestrictedNetworkTCPProbePort = 6443 + + readinessProbeOnlyOneAction = "only one action can be specified" + updatesNotAllowedWhenPowerOn = "updates to this field is not allowed when VM power is on" + storageClassNotAssignedFmt = "Storage policy is not associated with the namespace %s" + storageClassNotFoundFmt = "Storage policy is not associated with the namespace %s" + vSphereVolumeSizeNotMBMultiple = "value must be a multiple of MB" + addingModifyingInstanceVolumesNotAllowed = "adding or modifying instance storage volume claim(s) is not allowed" + featureNotEnabled = "the %s feature is not enabled" + invalidPowerStateOnCreateFmt = "cannot set a new VM's power state to %s" + invalidPowerStateOnUpdateFmt = "cannot %s a VM that is %s" + invalidPowerStateOnUpdateEmptyString = "cannot set power state to empty string" + invalidNextRestartTimeOnCreate = "cannot restart VM on create" + invalidNextRestartTimeOnUpdate = "must be formatted as RFC3339Nano" + invalidNextRestartTimeOnUpdateNow = "mutation webhooks are required to restart VM" +) + +// -kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha2-virtualmachine,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachines,versions=v1alpha2,name=default.validating.virtualmachine.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachines,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachines/status,verbs=get + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewValidatingWebhook(ctx, mgr, webHookName, NewValidator(mgr.GetClient())) + if err != nil { + return errors.Wrap(err, "failed to create VirtualMachine validation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewValidator returns the package's Validator. +func NewValidator(client client.Client) builder.Validator { + return validator{ + client: client, + // TODO BMV Use the Context.scheme instead + converter: runtime.DefaultUnstructuredConverter, + } +} + +type validator struct { + client client.Client + converter runtime.UnstructuredConverter +} + +func (v validator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachine{}).Name()) +} + +func (v validator) ValidateCreate(ctx *context.WebhookRequestContext) admission.Response { + vm, err := v.vmFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + + fieldErrs = append(fieldErrs, v.validateAvailabilityZone(ctx, vm, nil)...) + fieldErrs = append(fieldErrs, v.validateImage(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateClass(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateStorageClass(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateBootstrap(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateNetwork(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateVolumes(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateInstanceStorageVolumes(ctx, vm, nil)...) + fieldErrs = append(fieldErrs, v.validateReadinessProbe(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateAdvanced(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validatePowerStateOnCreate(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateNextRestartTimeOnCreate(ctx, vm)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) ValidateDelete(*context.WebhookRequestContext) admission.Response { + return admission.Allowed("") +} + +// ValidateUpdate validates if the given VirtualMachineSpec update is valid. +// Changes to following fields are not allowed: +// - ImageName +// - ClassName +// - StorageClass +// - ResourcePolicyName +// +// Following fields can only be changed when the VM is powered off. +// - Bootstrap +// - Network +func (v validator) ValidateUpdate(ctx *context.WebhookRequestContext) admission.Response { + vm, err := v.vmFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + oldVM, err := v.vmFromUnstructured(ctx.OldObj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + + // Check if an immutable field has been modified. + fieldErrs = append(fieldErrs, v.validateImmutableFields(ctx, vm, oldVM)...) + + // First validate any updates to the desired state based on the current + // power state of the VM. + fieldErrs = append(fieldErrs, v.validatePowerStateOnUpdate(ctx, vm, oldVM)...) + + // Validations for allowed updates. Return validation responses here for conditional updates regardless + // of whether the update is allowed or not. + fieldErrs = append(fieldErrs, v.validateAvailabilityZone(ctx, vm, oldVM)...) + fieldErrs = append(fieldErrs, v.validateBootstrap(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateNetwork(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateVolumes(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateInstanceStorageVolumes(ctx, vm, oldVM)...) + fieldErrs = append(fieldErrs, v.validateReadinessProbe(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateAdvanced(ctx, vm)...) + fieldErrs = append(fieldErrs, v.validateInstanceStorageVolumes(ctx, vm, oldVM)...) + fieldErrs = append(fieldErrs, v.validateNextRestartTimeOnUpdate(ctx, vm, oldVM)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) validateBootstrap( + ctx *context.WebhookRequestContext, + vm *vmopv1.VirtualMachine) field.ErrorList { + + var allErrs field.ErrorList + bootstrapPath := field.NewPath("spec", "bootstrap") + + cloudInit := vm.Spec.Bootstrap.CloudInit + linuxPrep := vm.Spec.Bootstrap.LinuxPrep + sysPrep := vm.Spec.Bootstrap.Sysprep + vAppConfig := vm.Spec.Bootstrap.VAppConfig + + if cloudInit != nil { + p := bootstrapPath.Child("cloudInit") + + if linuxPrep != nil || sysPrep != nil || vAppConfig != nil { + allErrs = append(allErrs, field.Forbidden(p, + "CloudInit may not be used with any other bootstrap provider")) + } + + hasCloudConfig := !equality.Semantic.DeepEqual(cloudInit.CloudConfig, cloudinit.CloudConfig{}) + hasRawCloudConfig := !equality.Semantic.DeepEqual(cloudInit.RawCloudConfig, corev1.SecretKeySelector{}) + if hasCloudConfig && hasRawCloudConfig { + allErrs = append(allErrs, field.Invalid(p, cloudInit, + "cloudConfig and rawCloudConfig are mutually exclusive")) + } + } + + if linuxPrep != nil { + p := bootstrapPath.Child("linuxPrep") + + if cloudInit != nil || sysPrep != nil { + allErrs = append(allErrs, field.Forbidden(p, + "LinuxPrep may not be used with either CloudInit or Sysprep bootstrap providers")) + } + } + + if sysPrep != nil { + p := bootstrapPath.Child("sysprep") + + if cloudInit != nil || linuxPrep != nil { + allErrs = append(allErrs, field.Forbidden(p, + "Sysprep may not be used with either CloudInit or LinuxPrep bootstrap providers")) + } + + if lib.IsWindowsSysprepFSSEnabled() { + hasSysPrep := !equality.Semantic.DeepEqual(sysPrep.Sysprep, sysprep.Sysprep{}) + hasRawSysPrep := !equality.Semantic.DeepEqual(sysPrep.RawSysprep, corev1.SecretKeySelector{}) + if hasSysPrep && hasRawSysPrep { + allErrs = append(allErrs, field.Invalid(p, sysPrep, + "sysprep and rawSysprep are mutually exclusive")) + } + } else { + allErrs = append(allErrs, field.Invalid(p, "sysprep", fmt.Sprintf(featureNotEnabled, "sysprep"))) + } + } + + if vAppConfig != nil { + p := bootstrapPath.Child("vAppConfig") + + if cloudInit != nil { + allErrs = append(allErrs, field.Forbidden(p, + "vAppConfig may not be used in conjunction with CloudInit bootstrap provider")) + } + + if len(vAppConfig.Properties) != 0 && len(vAppConfig.RawProperties) != 0 { + allErrs = append(allErrs, field.TypeInvalid(p, vAppConfig, + "properties and rawProperties are mutually exclusive")) + } + } + + return allErrs +} + +func (v validator) validateImage(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + + if vm.Spec.ImageName == "" { + allErrs = append(allErrs, field.Required(field.NewPath("spec", "imageName"), "")) + } + + return allErrs +} + +func (v validator) validateClass(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + + if vm.Spec.ClassName == "" { + allErrs = append(allErrs, field.Required(field.NewPath("spec", "className"), "")) + } + + return allErrs +} + +func (v validator) validateStorageClass(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + + if vm.Spec.StorageClass == "" { + return allErrs + } + + scPath := field.NewPath("spec", "storageClass") + scName := vm.Spec.StorageClass + + // TODO: This validation shouldn't be done in the webhook. + + sc := &storagev1.StorageClass{} + if err := v.client.Get(ctx, client.ObjectKey{Name: scName}, sc); err != nil { + return append(allErrs, field.Invalid(scPath, scName, fmt.Sprintf(storageClassNotFoundFmt, vm.Namespace))) + } + + resourceQuotas := &corev1.ResourceQuotaList{} + if err := v.client.List(ctx, resourceQuotas, client.InNamespace(vm.Namespace)); err != nil { + return append(allErrs, field.Invalid(scPath, scName, err.Error())) + } + + prefix := scName + storageResourceQuotaStrPattern + for _, resourceQuota := range resourceQuotas.Items { + for resourceName := range resourceQuota.Spec.Hard { + if strings.HasPrefix(resourceName.String(), prefix) { + return nil + } + } + } + + return append(allErrs, field.Invalid(scPath, scName, fmt.Sprintf(storageClassNotAssignedFmt, vm.Namespace))) +} + +func (v validator) validateNetwork(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + + networkSpec := &vm.Spec.Network + networkPath := field.NewPath("spec", "network") + + var hasIPv4Address, hasIPv6Address bool + for i, ipCIDR := range networkSpec.Addresses { + ip, _, err := net.ParseCIDR(ipCIDR) + if err != nil { + p := networkPath.Child("addresses").Index(i) + allErrs = append(allErrs, field.Invalid(p, ipCIDR, err.Error())) + continue + } + + hasIPv4Address = hasIPv4Address || ip.To4() != nil + hasIPv6Address = hasIPv6Address || ip.To16() != nil + } + + if networkSpec.DHCP4 { + if hasIPv4Address { + p := networkPath.Child("dhcp4") + allErrs = append(allErrs, field.Invalid(p, networkSpec.Addresses, + "dhcp4 cannot be used with IP4 addresses in network.addresses field")) + } + + if networkSpec.Gateway4 != "" { + p := networkPath.Child("gateway4") + allErrs = append(allErrs, field.Invalid(p, true, "gateway4 is mutually exclusive with dhcp4")) + } + } + + if networkSpec.DHCP6 { + if hasIPv6Address { + p := networkPath.Child("dhcp6") + allErrs = append(allErrs, field.Invalid(p, networkSpec.Addresses, + "dhcp6 cannot be used with IP6 addresses in network.addresses field")) + } + + if networkSpec.Gateway6 != "" { + p := networkPath.Child("gateway6") + allErrs = append(allErrs, field.Invalid(p, true, "gateway6 is mutually exclusive with dhcp6")) + } + } + + // TODO: Much more + + return allErrs +} + +func (v validator) validateVolumes(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + volumesPath := field.NewPath("spec", "volumes") + volumeNames := map[string]bool{} + + for i, vol := range vm.Spec.Volumes { + volPath := volumesPath.Index(i) + + if vol.Name != "" { + if volumeNames[vol.Name] { + allErrs = append(allErrs, field.Duplicate(volPath.Child("name"), vol.Name)) + } else { + volumeNames[vol.Name] = true + } + } else { + allErrs = append(allErrs, field.Required(volPath.Child("name"), "")) + } + + if vol.Name != "" { + errs := validation.NameIsDNSSubdomain(cNSAttachmentNameForVolume(vm.Name, vol.Name), false) + for _, msg := range errs { + allErrs = append(allErrs, field.Invalid(volPath.Child("name"), vol.Name, msg)) + } + } + + if vol.PersistentVolumeClaim == nil { + allErrs = append(allErrs, field.Required(volPath.Child("persistentVolumeClaim"), "")) + } else { + allErrs = append(allErrs, v.validateVolumeWithPVC(ctx, vol, volPath)...) + } + } + + // TODO: InstanceStorage + + return allErrs +} + +func (v validator) validateVolumeWithPVC( + ctx *context.WebhookRequestContext, + vol vmopv1.VirtualMachineVolume, + volPath *field.Path) field.ErrorList { + + var allErrs field.ErrorList + pvcPath := volPath.Child("persistentVolumeClaim") + + if vol.PersistentVolumeClaim.ClaimName == "" { + allErrs = append(allErrs, field.Required(pvcPath.Child("claimName"), "")) + } + if vol.PersistentVolumeClaim.ReadOnly { + allErrs = append(allErrs, field.NotSupported(pvcPath.Child("readOnly"), true, []string{"false"})) + } + + return allErrs +} + +// validateInstanceStorageVolumes validates if instance storage volumes are added/modified. +func (v validator) validateInstanceStorageVolumes( + ctx *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { + + var allErrs field.ErrorList + + if ctx.IsPrivilegedAccount { + return allErrs + } + + var oldVMInstanceStorageVolumes []vmopv1.VirtualMachineVolume + if oldVM != nil { + oldVMInstanceStorageVolumes = filterVolumesA2(oldVM) + } + + if !equality.Semantic.DeepEqual(filterVolumesA2(vm), oldVMInstanceStorageVolumes) { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "volumes"), addingModifyingInstanceVolumesNotAllowed)) + } + + return allErrs +} + +func (v validator) validateReadinessProbe(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + readinessProbePath := field.NewPath("spec", "readinessProbe") + + probe := &vm.Spec.ReadinessProbe + + if probe.TCPSocket != nil && probe.GuestHeartbeat != nil { + allErrs = append(allErrs, field.Forbidden(readinessProbePath, readinessProbeOnlyOneAction)) + } + + if probe.TCPSocket != nil { + tcpSocketPath := readinessProbePath.Child("tcpSocket") + + // Validate port if environment is a restricted network environment between SV CP VMs and Workload VMs e.g. VMC. + if probe.TCPSocket.Port.IntValue() != allowedRestrictedNetworkTCPProbePort { + isRestrictedEnv, err := v.isNetworkRestrictedForReadinessProbe(ctx) + if err != nil { + allErrs = append(allErrs, field.Forbidden(tcpSocketPath, err.Error())) + } else if isRestrictedEnv { + allErrs = append(allErrs, + field.NotSupported(tcpSocketPath.Child("port"), probe.TCPSocket.Port.IntValue(), + []string{strconv.Itoa(allowedRestrictedNetworkTCPProbePort)})) + } + } + } + + return allErrs +} + +var megaByte = resource.MustParse("1Mi") + +func (v validator) validateAdvanced(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + advancedPath := field.NewPath("spec", "advanced") + advanced := &vm.Spec.Advanced + + if diskCap := advanced.BootDiskCapacity; !diskCap.IsZero() { + if diskCap.Value()%megaByte.Value() != 0 { + allErrs = append(allErrs, field.Invalid(advancedPath.Child("bootDiskCapacity"), + diskCap.Value(), vSphereVolumeSizeNotMBMultiple)) + } + } + + return allErrs +} + +func (v validator) validateNextRestartTimeOnCreate( + ctx *context.WebhookRequestContext, + vm *vmopv1.VirtualMachine) field.ErrorList { + + var allErrs field.ErrorList + + if vm.Spec.NextRestartTime != "" { + allErrs = append( + allErrs, + field.Invalid( + field.NewPath("spec").Child("nextRestartTime"), + vm.Spec.NextRestartTime, + invalidNextRestartTimeOnCreate)) + } + + return allErrs +} + +func (v validator) validateNextRestartTimeOnUpdate( + ctx *context.WebhookRequestContext, + newVM, oldVM *vmopv1.VirtualMachine) field.ErrorList { + + if newVM.Spec.NextRestartTime == oldVM.Spec.NextRestartTime { + return nil + } + + var allErrs field.ErrorList + + nextRestartTimePath := field.NewPath("spec").Child("nextRestartTime") + + if strings.EqualFold(newVM.Spec.NextRestartTime, "now") { + allErrs = append( + allErrs, + field.Invalid( + nextRestartTimePath, + newVM.Spec.NextRestartTime, + invalidNextRestartTimeOnUpdateNow)) + } else if _, err := time.Parse(time.RFC3339Nano, newVM.Spec.NextRestartTime); err != nil { + allErrs = append( + allErrs, + field.Invalid( + nextRestartTimePath, + newVM.Spec.NextRestartTime, + invalidNextRestartTimeOnUpdate)) + } + + return allErrs +} + +func (v validator) validatePowerStateOnCreate( + ctx *context.WebhookRequestContext, + newVM *vmopv1.VirtualMachine) field.ErrorList { + + var ( + allErrs field.ErrorList + newPowerState = newVM.Spec.PowerState + ) + + if newPowerState == vmopv1.VirtualMachinePowerStateSuspended { + allErrs = append( + allErrs, + field.Invalid( + field.NewPath("spec").Child("powerState"), + newPowerState, + fmt.Sprintf(invalidPowerStateOnCreateFmt, newPowerState))) + } + + return allErrs +} + +func (v validator) validatePowerStateOnUpdate( + ctx *context.WebhookRequestContext, + newVM, oldVM *vmopv1.VirtualMachine) field.ErrorList { + + var allErrs field.ErrorList + powerStatePath := field.NewPath("spec").Child("powerState") + + // If a VM is powered off, all config changes are allowed except for setting + // the power state to shutdown or suspend. + // If a VM is requesting a power off, we can Reconfigure the VM _after_ + // we power it off - all changes are allowed. + // If a VM is requesting a power on, we can Reconfigure the VM _before_ + // we power it on - all changes are allowed. + newPowerState, oldPowerState := newVM.Spec.PowerState, oldVM.Spec.PowerState + + if newPowerState == "" { + allErrs = append( + allErrs, + field.Invalid( + powerStatePath, + newPowerState, + invalidPowerStateOnUpdateEmptyString)) + return allErrs + } + + switch oldPowerState { + case vmopv1.VirtualMachinePowerStateOn: + // The request is attempting to power on a VM that is already powered + // on. Validate fields that may or may not be mutable while a VM is in + // a powered on state. + if newPowerState == vmopv1.VirtualMachinePowerStateOn { + invalidFields := v.validateUpdatesWhenPoweredOn(ctx, newVM, oldVM) + allErrs = append(allErrs, invalidFields...) + } + + case vmopv1.VirtualMachinePowerStateOff: + // Mark the power state as invalid if the request is attempting to + // suspend a VM that is powered off. + var msg string + if newPowerState == vmopv1.VirtualMachinePowerStateSuspended { + msg = "suspend" + } + if msg != "" { + allErrs = append( + allErrs, + field.Invalid( + powerStatePath, + newPowerState, + fmt.Sprintf(invalidPowerStateOnUpdateFmt, msg, "powered off"))) + } + } + + return allErrs +} + +func (v validator) validateUpdatesWhenPoweredOn(ctx *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + specPath := field.NewPath("spec") + + if !equality.Semantic.DeepEqual(vm.Spec.Bootstrap, oldVM.Spec.Bootstrap) { + allErrs = append(allErrs, field.Forbidden(specPath.Child("bootstrap"), updatesNotAllowedWhenPowerOn)) + } + if !equality.Semantic.DeepEqual(vm.Spec.Network, oldVM.Spec.Network) { + allErrs = append(allErrs, field.Forbidden(specPath.Child("network"), updatesNotAllowedWhenPowerOn)) + } + + // TODO: More checks. + + return allErrs +} + +func (v validator) validateImmutableFields(ctx *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + specPath := field.NewPath("spec") + + allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ImageName, oldVM.Spec.ImageName, specPath.Child("imageName"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.ClassName, oldVM.Spec.ClassName, specPath.Child("className"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.StorageClass, oldVM.Spec.StorageClass, specPath.Child("storageClass"))...) + // TODO: More checks. + + // TODO: Allow privilege? + allErrs = append(allErrs, validation.ValidateImmutableField(vm.Spec.Reserved.ResourcePolicyName, oldVM.Spec.Reserved.ResourcePolicyName, specPath.Child("reserved", "resourcePolicyName"))...) + + return allErrs +} + +func (v validator) validateAvailabilityZone(ctx *context.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { + var allErrs field.ErrorList + + if !lib.IsWcpFaultDomainsFSSEnabled() { + return allErrs + } + + zoneLabelPath := field.NewPath("metadata", "labels").Key(topology.KubernetesTopologyZoneLabelKey) + + if oldVM != nil { + // Once the zone has been set then make sure the field is immutable. + if oldVal := oldVM.Labels[topology.KubernetesTopologyZoneLabelKey]; oldVal != "" { + newVal := vm.Labels[topology.KubernetesTopologyZoneLabelKey] + return append(allErrs, validation.ValidateImmutableField(newVal, oldVal, zoneLabelPath)...) + } + } + + // Validate the name of the provided availability zone. + if zone := vm.Labels[topology.KubernetesTopologyZoneLabelKey]; zone != "" { + if _, err := topology.GetAvailabilityZone(ctx.Context, v.client, zone); err != nil { + return append(allErrs, field.Invalid(zoneLabelPath, zone, err.Error())) + } + } + + return allErrs +} + +// vmFromUnstructured returns the VirtualMachine from the unstructured object. +func (v validator) vmFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachine, error) { + vm := &vmopv1.VirtualMachine{} + if err := v.converter.FromUnstructured(obj.UnstructuredContent(), vm); err != nil { + return nil, err + } + return vm, nil +} + +func (v validator) isNetworkRestrictedForReadinessProbe(ctx *context.WebhookRequestContext) (bool, error) { + configMap := &corev1.ConfigMap{} + configMapKey := client.ObjectKey{Name: config.ProviderConfigMapName, Namespace: ctx.Namespace} + if err := v.client.Get(ctx, configMapKey, configMap); err != nil { + return false, fmt.Errorf("error get ConfigMap: %s while validating TCP readiness probe port: %v", configMapKey, err) + } + + return configMap.Data[isRestrictedNetworkKey] == "true", nil +} diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go new file mode 100644 index 000000000..e241ec925 --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_intg_test.go @@ -0,0 +1,222 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + Describe("Invoking Create", intgTestsValidateCreate) + Describe("Invoking Update", intgTestsValidateUpdate) + Describe("Invoking Delete", intgTestsValidateDelete) +} + +type intgValidatingWebhookContext struct { + builder.IntegrationTestContext + vm *vmopv1.VirtualMachine +} + +func newIntgValidatingWebhookContext() *intgValidatingWebhookContext { + ctx := &intgValidatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vm = builder.DummyVirtualMachineA2() + ctx.vm.Namespace = ctx.Namespace + + return ctx +} + +func intgTestsValidateCreate() { + var ( + ctx *intgValidatingWebhookContext + ) + + type createArgs struct { + invalidImageName bool + } + + validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + if args.invalidImageName { + ctx.vm.Spec.ImageName = "" + } + + err := ctx.Client.Create(ctx, ctx.vm) + if expectedAllowed { + Expect(err).ToNot(HaveOccurred()) + } else { + Expect(err).To(HaveOccurred()) + } + if expectedReason != "" { + Expect(err.Error()).To(ContainSubstring(expectedReason)) + } + } + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + }) + + AfterEach(func() { + ctx.AfterEach() + ctx = nil + }) + + specPath := field.NewPath("spec") + + DescribeTable("create table", validateCreate, + Entry("should work", createArgs{}, true, "", nil), + Entry("should not work for invalid image name", createArgs{invalidImageName: true}, false, + field.Required(specPath.Child("imageName"), "").Error(), nil), + ) +} + +func intgTestsValidateUpdate() { + const ( + immutableFieldMsg = "field is immutable" + ) + + var ( + ctx *intgValidatingWebhookContext + err error + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + Expect(ctx.Client.Create(ctx, ctx.vm)).To(Succeed()) + }) + + JustBeforeEach(func() { + err = ctx.Client.Update(suite, ctx.vm) + }) + + AfterEach(func() { + ctx.AfterEach() + ctx = nil + }) + + When("update is performed with changed image name", func() { + BeforeEach(func() { + ctx.vm.Spec.ImageName += "-2" + }) + + It("should deny the request", func() { + Expect(err).To(HaveOccurred()) + expectedPathStr := field.NewPath("spec", "imageName").String() + Expect(err.Error()).To(ContainSubstring(expectedPathStr)) + Expect(err.Error()).To(ContainSubstring(immutableFieldMsg)) + }) + }) + When("update is performed with changed storageClass name", func() { + BeforeEach(func() { + ctx.vm.Spec.StorageClass += "-2" + }) + It("should deny the request", func() { + Expect(err).To(HaveOccurred()) + expectedPath := field.NewPath("spec", "storageClass") + Expect(err.Error()).To(ContainSubstring(expectedPath.String())) + Expect(err.Error()).To(ContainSubstring(immutableFieldMsg)) + }) + }) + + Context("VirtualMachine update while VM is powered on", func() { + BeforeEach(func() { + ctx.vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOn + }) + + When("Bootstrap is updated", func() { + BeforeEach(func() { + ctx.vm.Spec.Bootstrap = vmopv1.VirtualMachineBootstrapSpec{ + CloudInit: &vmopv1.VirtualMachineBootstrapCloudInitSpec{ + RawCloudConfig: corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{}, + Key: "", + Optional: nil, + }, + }, + } + }) + + It("rejects the request", func() { + expectedReason := field.Forbidden(field.NewPath("spec", "bootstrap"), + "updates to this field is not allowed when VM power is on").Error() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(expectedReason)) + }) + }) + + When("Network is updated", func() { + BeforeEach(func() { + ctx.vm.Spec.Network = vmopv1.VirtualMachineNetworkSpec{ + HostName: "my-new-name", + } + }) + + It("rejects the request", func() { + expectedReason := field.Forbidden(field.NewPath("spec", "network"), + "updates to this field is not allowed when VM power is on").Error() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(expectedReason)) + }) + }) + + When("Volume for PVC is added", func() { + BeforeEach(func() { + ctx.vm.Spec.Volumes = append(ctx.vm.Spec.Volumes, + vmopv1.VirtualMachineVolume{ + Name: "dummy-new-pv-volume", + VirtualMachineVolumeSource: vmopv1.VirtualMachineVolumeSource{ + PersistentVolumeClaim: &vmopv1.PersistentVolumeClaimVolumeSource{ + PersistentVolumeClaimVolumeSource: corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "dummy-new-claim-name", + }, + }, + }, + }) + }) + + It("does not reject the request", func() { + Expect(err).NotTo(HaveOccurred()) + }) + }) + }) +} + +func intgTestsValidateDelete() { + var ( + ctx *intgValidatingWebhookContext + err error + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + + err := ctx.Client.Create(ctx, ctx.vm) + Expect(err).ToNot(HaveOccurred()) + }) + + JustBeforeEach(func() { + err = ctx.Client.Delete(suite, ctx.vm) + }) + + AfterEach(func() { + ctx.AfterEach() + ctx = nil + }) + + When("delete is performed", func() { + It("should allow the request", func() { + Expect(ctx.Namespace).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + }) + }) +} diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_suite_test.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_suite_test.go new file mode 100644 index 000000000..9411e716c --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha2/validation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForValidatingWebhook( + validation.AddToManager, + validation.NewValidator, + "default.validating.virtualmachine.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Validation webhook suite", nil /*intgTests*/, unitTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go new file mode 100644 index 000000000..0af8f87fd --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go @@ -0,0 +1,557 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "fmt" + "os" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/api/v1alpha2/common" + "github.com/vmware-tanzu/vm-operator/pkg/lib" + "github.com/vmware-tanzu/vm-operator/pkg/topology" + "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/config" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +const ( + updateSuffix = "-updated" +) + +func unitTests() { + Describe("Invoking ValidateCreate", unitTestsValidateCreate) + Describe("Invoking ValidateUpdate", unitTestsValidateUpdate) + Describe("Invoking ValidateDelete", unitTestsValidateDelete) +} + +type unitValidatingWebhookContext struct { + builder.UnitTestContextForValidatingWebhook + vm, oldVM *vmopv1.VirtualMachine +} + +func newUnitTestContextForValidatingWebhook(isUpdate bool) *unitValidatingWebhookContext { + vm := builder.DummyVirtualMachineA2() + vm.Name = "dummy-vm-for-webhook-validation" + vm.Namespace = "dummy-vm-namespace-for-webhook-validation" + obj, err := builder.ToUnstructured(vm) + Expect(err).ToNot(HaveOccurred()) + + var oldVM *vmopv1.VirtualMachine + var oldObj *unstructured.Unstructured + + if isUpdate { + oldVM = vm.DeepCopy() + oldObj, err = builder.ToUnstructured(oldVM) + Expect(err).ToNot(HaveOccurred()) + } + + zone := builder.DummyAvailabilityZone() + initObjects := []client.Object{zone} + + return &unitValidatingWebhookContext{ + UnitTestContextForValidatingWebhook: *suite.NewUnitTestContextForValidatingWebhook(obj, oldObj, initObjects...), + vm: vm, + oldVM: oldVM, + } +} + +//nolint:gocyclo +func unitTestsValidateCreate() { + var ( + ctx *unitValidatingWebhookContext + ) + + type createArgs struct { + isServiceUser bool + invalidImageName bool + invalidClassName bool + invalidVolumeName bool + dupVolumeName bool + invalidVolumeSource bool + invalidPVCName bool + invalidPVCReadOnly bool + invalidStorageClass bool + notFoundStorageClass bool + validStorageClass bool + withInstanceStorageVolumes bool + invalidReadinessProbe bool + isRestrictedNetworkEnv bool + isRestrictedNetworkValidProbePort bool + isNonRestrictedNetworkEnv bool + isNoAvailabilityZones bool + isWCPFaultDomainsFSSEnabled bool + isInvalidAvailabilityZone bool + isEmptyAvailabilityZone bool + isBootstrapCloudInit bool + isBootstrapCloudInitInline bool + isBootstrapLinuxPrep bool + isSysprepFeatureEnabled bool + isBootstrapSysPrep bool + isBootstrapSysPrepInline bool + isBootstrapVAppConfig bool + isBootstrapVAppConfigInline bool + powerState vmopv1.VirtualMachinePowerState + nextRestartTime string + } + + validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + if args.isServiceUser { + ctx.IsPrivilegedAccount = true + } + + if args.invalidImageName { + ctx.vm.Spec.ImageName = "" + } + if args.invalidClassName { + ctx.vm.Spec.ClassName = "" + } + + if args.invalidVolumeName { + ctx.vm.Spec.Volumes[0].Name = "underscore_not_valid" + } + if args.dupVolumeName { + ctx.vm.Spec.Volumes[0].Name = "duplicate-name" + ctx.vm.Spec.Volumes = append(ctx.vm.Spec.Volumes, ctx.vm.Spec.Volumes[0]) + } + if args.invalidVolumeSource { + ctx.vm.Spec.Volumes[0].PersistentVolumeClaim = nil + } + if args.invalidPVCName { + ctx.vm.Spec.Volumes[0].PersistentVolumeClaim.ClaimName = "" + } + if args.invalidPVCReadOnly { + ctx.vm.Spec.Volumes[0].PersistentVolumeClaim.ReadOnly = true + } + + if args.invalidStorageClass { + // StorageClass specifies but not assigned to ResourceQuota. + storageClass := builder.DummyStorageClass() + ctx.vm.Spec.StorageClass = storageClass.Name + Expect(ctx.Client.Create(ctx, storageClass)).To(Succeed()) + } + if args.notFoundStorageClass { + // StorageClass specified but no ResourceQuotas. + ctx.vm.Spec.StorageClass = builder.DummyStorageClassName + } + if args.validStorageClass { + // StorageClass specified and is assigned to ResourceQuota. + storageClass := builder.DummyStorageClass() + Expect(ctx.Client.Create(ctx, storageClass)).To(Succeed()) + ctx.vm.Spec.StorageClass = storageClass.Name + + rlName := storageClass.Name + ".storageclass.storage.k8s.io/persistentvolumeclaims" + resourceQuota := builder.DummyResourceQuota(ctx.vm.Namespace, rlName) + Expect(ctx.Client.Create(ctx, resourceQuota)).To(Succeed()) + } + + if args.withInstanceStorageVolumes { + instanceStorageVolumes := builder.DummyInstanceStorageVirtualMachineVolumesA2() + ctx.vm.Spec.Volumes = append(ctx.vm.Spec.Volumes, instanceStorageVolumes...) + } + + if args.invalidReadinessProbe { + ctx.vm.Spec.ReadinessProbe = vmopv1.VirtualMachineReadinessProbeSpec{ + TCPSocket: &vmopv1.TCPSocketAction{}, + GuestHeartbeat: &vmopv1.GuestHeartbeatAction{}, + } + } + if args.isRestrictedNetworkEnv || args.isNonRestrictedNetworkEnv { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.ProviderConfigMapName, + Namespace: ctx.Namespace, + }, + Data: make(map[string]string), + } + if args.isRestrictedNetworkEnv { + cm.Data["IsRestrictedNetwork"] = "true" + } + Expect(ctx.Client.Create(ctx, cm)).To(Succeed()) + + portValue := 6443 + if !args.isRestrictedNetworkValidProbePort { + portValue = 443 + } + ctx.vm.Spec.ReadinessProbe = vmopv1.VirtualMachineReadinessProbeSpec{ + TCPSocket: &vmopv1.TCPSocketAction{Port: intstr.FromInt(portValue)}, + } + } + + if args.isWCPFaultDomainsFSSEnabled { + Expect(os.Setenv(lib.WcpFaultDomainsFSS, "true")).To(Succeed()) + } + if args.isNoAvailabilityZones { + Expect(ctx.Client.Delete(ctx, builder.DummyAvailabilityZone())).To(Succeed()) + } + //nolint:gocritic // Ignore linter complaint about converting to switch case since the following is more readable. + if args.isEmptyAvailabilityZone { + delete(ctx.vm.Labels, topology.KubernetesTopologyZoneLabelKey) + } else if args.isInvalidAvailabilityZone { + ctx.vm.Labels[topology.KubernetesTopologyZoneLabelKey] = "invalid" + } else { + zoneName := builder.DummyAvailabilityZoneName + if !lib.IsWcpFaultDomainsFSSEnabled() { + zoneName = topology.DefaultAvailabilityZoneName + } + ctx.vm.Labels[topology.KubernetesTopologyZoneLabelKey] = zoneName + } + + if args.isBootstrapCloudInit || args.isBootstrapCloudInitInline { + ctx.vm.Spec.Bootstrap.CloudInit = &vmopv1.VirtualMachineBootstrapCloudInitSpec{} + if args.isBootstrapCloudInit { + ctx.vm.Spec.Bootstrap.CloudInit.RawCloudConfig.Key = "cloud-init-key" + } + if args.isBootstrapCloudInitInline { + ctx.vm.Spec.Bootstrap.CloudInit.CloudConfig.Timezone = " dummy-tz" + } + } + if args.isBootstrapLinuxPrep { + ctx.vm.Spec.Bootstrap.LinuxPrep = &vmopv1.VirtualMachineBootstrapLinuxPrepSpec{} + } + if args.isSysprepFeatureEnabled { + Expect(os.Setenv(lib.WindowsSysprepFSS, "true")).To(Succeed()) + } + if args.isBootstrapSysPrep || args.isBootstrapSysPrepInline { + ctx.vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOff + ctx.vm.Spec.Bootstrap.Sysprep = &vmopv1.VirtualMachineBootstrapSysprepSpec{} + if args.isBootstrapSysPrep { + ctx.vm.Spec.Bootstrap.Sysprep.RawSysprep.Key = "sysprep-key" + } + if args.isBootstrapSysPrepInline { + ctx.vm.Spec.Bootstrap.Sysprep.Sysprep.GUIRunOnce.Commands = []string{"hello"} + } + } + if args.isBootstrapVAppConfig || args.isBootstrapVAppConfigInline { + ctx.vm.Spec.Bootstrap.VAppConfig = &vmopv1.VirtualMachineBootstrapVAppConfigSpec{} + if args.isBootstrapVAppConfig { + ctx.vm.Spec.Bootstrap.VAppConfig.RawProperties = "some-vapp-prop" + } + if args.isBootstrapVAppConfigInline { + ctx.vm.Spec.Bootstrap.VAppConfig.Properties = []common.KeyValueOrSecretKeySelectorPair{ + { + Key: "key", + }, + } + } + } + + ctx.vm.Spec.PowerState = args.powerState + ctx.vm.Spec.NextRestartTime = args.nextRestartTime + + var err error + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vm) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(ContainSubstring(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + + AfterEach(func() { + Expect(os.Unsetenv(lib.WcpFaultDomainsFSS)).To(Succeed()) + Expect(os.Unsetenv(lib.WindowsSysprepFSS)).To(Succeed()) + ctx = nil + }) + + specPath := field.NewPath("spec") + volPath := specPath.Child("volumes") + nextRestartTimePath := specPath.Child("nextRestartTime") + now := time.Now().UTC() + + DescribeTable("create table", validateCreate, + Entry("should allow valid", createArgs{}, true, nil, nil), + + Entry("should deny invalid class name", createArgs{invalidClassName: true}, false, + field.Required(specPath.Child("className"), "").Error(), nil), + Entry("should deny invalid image name", createArgs{invalidImageName: true}, false, + field.Required(specPath.Child("imageName"), "").Error(), nil), + + Entry("should fail when Readiness probe has multiple actions", createArgs{invalidReadinessProbe: true}, false, + field.Forbidden(specPath.Child("readinessProbe"), "only one action can be specified").Error(), nil), + + Entry("should deny invalid volume name", createArgs{invalidVolumeName: true}, false, + field.Invalid(volPath.Index(0).Child("name"), "underscore_not_valid", validation.IsDNS1123Subdomain("underscore_not_valid")[0]).Error(), nil), + Entry("should deny duplicated volume names", createArgs{dupVolumeName: true}, false, + field.Duplicate(volPath.Index(1).Child("name"), "duplicate-name").Error(), nil), + Entry("should deny invalid volume source spec", createArgs{invalidVolumeSource: true}, false, + field.Required(volPath.Index(0).Child("persistentVolumeClaim"), "").Error(), nil), + Entry("should deny invalid PVC name", createArgs{invalidPVCName: true}, false, + field.Required(volPath.Index(0).Child("persistentVolumeClaim", "claimName"), "").Error(), nil), + Entry("should deny invalid PVC read only", createArgs{invalidPVCReadOnly: true}, false, + field.NotSupported(volPath.Index(0).Child("persistentVolumeClaim", "readOnly"), true, []string{"false"}).Error(), nil), + Entry("should deny a StorageClass that does not exist", createArgs{notFoundStorageClass: true}, false, + field.Invalid(specPath.Child("storageClass"), builder.DummyStorageClassName, fmt.Sprintf("Storage policy is not associated with the namespace %s", "")).Error(), nil), + Entry("should deny a StorageClass that is not associated with the namespace", createArgs{invalidStorageClass: true}, false, + field.Invalid(specPath.Child("storageClass"), builder.DummyStorageClassName, fmt.Sprintf("Storage policy is not associated with the namespace %s", "")).Error(), nil), + Entry("should allow valid storage class and resource quota", createArgs{validStorageClass: true}, true, nil, nil), + Entry("should deny when there are instance storage volumes and user is SSO user", createArgs{withInstanceStorageVolumes: true}, false, + field.Forbidden(volPath, "adding or modifying instance storage volume claim(s) is not allowed").Error(), nil), + Entry("should allow when there are instance storage volumes and user is service user", createArgs{isServiceUser: true, withInstanceStorageVolumes: true}, true, nil, nil), + + Entry("should deny when restricted network and TCP port in readiness probe is not 6443", createArgs{isRestrictedNetworkEnv: true, isRestrictedNetworkValidProbePort: false}, false, + field.NotSupported(specPath.Child("readinessProbe", "tcpSocket", "port"), 443, []string{"6443"}).Error(), nil), + Entry("should allow when restricted network and TCP port in readiness probe is 6443", createArgs{isRestrictedNetworkEnv: true, isRestrictedNetworkValidProbePort: true}, true, nil, nil), + Entry("should allow when not restricted network and TCP port in readiness probe is not 6443", createArgs{isNonRestrictedNetworkEnv: true, isRestrictedNetworkValidProbePort: false}, true, nil, nil), + + Entry("should allow when VM specifies no availability zone, there are availability zones, and WCP FaultDomains FSS is disabled", createArgs{isEmptyAvailabilityZone: true}, true, nil, nil), + Entry("should allow when VM specifies no availability zone, there are no availability zones, and WCP FaultDomains FSS is disabled", createArgs{isEmptyAvailabilityZone: true, isNoAvailabilityZones: true}, true, nil, nil), + Entry("should allow when VM specifies no availability zone, there are availability zones, and WCP FaultDomains FSS is enabled", createArgs{isEmptyAvailabilityZone: true, isWCPFaultDomainsFSSEnabled: true}, true, nil, nil), + Entry("should allow when VM specifies no availability zone, there are no availability zones, and WCP FaultDomains FSS is enabled", createArgs{isEmptyAvailabilityZone: true, isNoAvailabilityZones: true, isWCPFaultDomainsFSSEnabled: true}, true, nil, nil), + Entry("should allow when VM specifies valid availability zone, there are availability zones, and WCP FaultDomains FSS is enabled", createArgs{isWCPFaultDomainsFSSEnabled: true}, true, nil, nil), + Entry("should allow when VM specifies valid availability zone, there are no availability zones, and WCP FaultDomains FSS is disabled", createArgs{isNoAvailabilityZones: true}, true, nil, nil), + Entry("should allow when VM specifies invalid availability zone, there are availability zones, and WCP FaultDomains FSS is disabled", createArgs{isInvalidAvailabilityZone: true}, true, nil, nil), + Entry("should deny when VM specifies invalid availability zone, there are availability zones, and WCP FaultDomains FSS is enabled", createArgs{isInvalidAvailabilityZone: true, isWCPFaultDomainsFSSEnabled: true}, false, nil, nil), + Entry("should allow when VM specifies invalid availability zone, there are no availability zones, and WCP FaultDomains FSS is disabled", createArgs{isInvalidAvailabilityZone: true, isNoAvailabilityZones: true}, true, nil, nil), + Entry("should deny when VM specifies invalid availability zone, there are no availability zones, and WCP FaultDomains FSS is enabled", createArgs{isInvalidAvailabilityZone: true, isNoAvailabilityZones: true, isWCPFaultDomainsFSSEnabled: true}, false, nil, nil), + Entry("should deny when there are no availability zones and WCP FaultDomains FSS is enabled", createArgs{isNoAvailabilityZones: true, isWCPFaultDomainsFSSEnabled: true}, false, nil, nil), + + Entry("should allow CloudInit", createArgs{isBootstrapCloudInit: true}, true, nil, nil), + Entry("should deny CloudInit with raw and inline", createArgs{isBootstrapCloudInit: true, isBootstrapCloudInitInline: true}, false, "cloudConfig and rawCloudConfig are mutually exclusive", nil), + Entry("should deny CloudInit with LinuxPrep", createArgs{isBootstrapCloudInit: true, isBootstrapLinuxPrep: true}, false, "CloudInit may not be used with any other bootstrap provider", nil), + Entry("should deny CloudInit with SysPrep", createArgs{isBootstrapCloudInit: true, isBootstrapSysPrep: true}, false, "CloudInit may not be used with any other bootstrap provider", nil), + Entry("should deny CloudInit with vApp", createArgs{isBootstrapCloudInit: true, isBootstrapVAppConfig: true}, false, "CloudInit may not be used with any other bootstrap provider", nil), + + Entry("should allow LinuxPrep", createArgs{isBootstrapLinuxPrep: true}, true, nil, nil), + Entry("should deny LinuxPrep with CloudInit", createArgs{isBootstrapLinuxPrep: true, isBootstrapCloudInit: true}, false, "LinuxPrep may not be used with either CloudInit or Sysprep bootstrap providers", nil), + Entry("should deny LinuxPrep with SysPrep", createArgs{isBootstrapLinuxPrep: true, isBootstrapSysPrep: true}, false, "LinuxPrep may not be used with either CloudInit or Sysprep bootstrap providers", nil), + + Entry("should allow sysprep when FSS is enabled", createArgs{isSysprepFeatureEnabled: true, isBootstrapSysPrep: true}, true, nil, nil), + Entry("should disallow sysprep when FSS is disabled", createArgs{isSysprepFeatureEnabled: false, isBootstrapSysPrep: true}, false, + field.Invalid(specPath.Child("bootstrap", "sysprep"), "sysprep", "the sysprep feature is not enabled").Error(), nil), + Entry("should deny sysprep with CloudInit", createArgs{isSysprepFeatureEnabled: true, isBootstrapSysPrep: true, isBootstrapCloudInit: true}, false, nil, nil), + + Entry("should allow vApp", createArgs{isBootstrapVAppConfig: true}, true, nil, nil), + Entry("should deny with raw and inline", createArgs{isBootstrapVAppConfig: true, isBootstrapVAppConfigInline: true}, false, "properties and rawProperties are mutually exclusive", nil), + Entry("should allow vApp with LinuxPrep", createArgs{isBootstrapVAppConfig: true, isBootstrapLinuxPrep: true}, true, nil, nil), + Entry("should allow vApp with SysPrep", createArgs{isBootstrapVAppConfig: true, isSysprepFeatureEnabled: true, isBootstrapSysPrep: true}, true, nil, nil), + + Entry("should disallow creating VM with suspended power state", createArgs{powerState: vmopv1.VirtualMachinePowerStateSuspended}, false, + field.Invalid(specPath.Child("powerState"), vmopv1.VirtualMachinePowerStateSuspended, "cannot set a new VM's power state to Suspended").Error(), nil), + + Entry("should allow creating VM with empty nextRestartTime value", createArgs{}, true, nil, nil), + Entry("should disallow creating VM with non-empty, valid nextRestartTime value", createArgs{ + nextRestartTime: now.Format(time.RFC3339Nano)}, false, + field.Invalid(nextRestartTimePath, now.Format(time.RFC3339Nano), "cannot restart VM on create").Error(), nil), + Entry("should disallow creating VM with non-empty, valid nextRestartTime value if mutation webhooks were running", + createArgs{nextRestartTime: "now"}, false, + field.Invalid(nextRestartTimePath, "now", "cannot restart VM on create").Error(), nil), + Entry("should disallow creating VM with non-empty, invalid nextRestartTime value", + createArgs{nextRestartTime: "hello"}, false, + field.Invalid(nextRestartTimePath, "hello", "cannot restart VM on create").Error(), nil), + ) +} + +func unitTestsValidateUpdate() { + var ( + ctx *unitValidatingWebhookContext + ) + + type updateArgs struct { + isServiceUser bool + changeClassName bool + changeImageName bool + changeStorageClass bool + changeResourcePolicy bool + assignZoneName bool + changeZoneName bool + isSysprepFeatureEnabled bool + isSysprepTransportUsed bool + withInstanceStorageVolumes bool + changeInstanceStorageVolume bool + oldPowerState vmopv1.VirtualMachinePowerState + newPowerState vmopv1.VirtualMachinePowerState + newPowerStateEmptyAllowed bool + nextRestartTime string + lastRestartTime string + } + + validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + if args.isServiceUser { + ctx.IsPrivilegedAccount = true + } + + if args.changeImageName { + ctx.vm.Spec.ImageName += updateSuffix + } + if args.changeClassName { + ctx.vm.Spec.ClassName += updateSuffix + } + if args.changeStorageClass { + ctx.vm.Spec.StorageClass += updateSuffix + } + if args.changeResourcePolicy { + ctx.vm.Spec.Reserved.ResourcePolicyName = updateSuffix + } + if args.assignZoneName { + ctx.vm.Labels[topology.KubernetesTopologyZoneLabelKey] = builder.DummyAvailabilityZoneName + } + if args.changeZoneName { + ctx.oldVM.Labels[topology.KubernetesTopologyZoneLabelKey] = builder.DummyAvailabilityZoneName + ctx.vm.Labels[topology.KubernetesTopologyZoneLabelKey] = builder.DummyAvailabilityZoneName + updateSuffix + } + + if args.withInstanceStorageVolumes { + instanceStorageVolumes := builder.DummyInstanceStorageVirtualMachineVolumesA2() + ctx.vm.Spec.Volumes = append(ctx.vm.Spec.Volumes, instanceStorageVolumes...) + } + if args.changeInstanceStorageVolume { + instanceStorageVolumes := builder.DummyInstanceStorageVirtualMachineVolumesA2() + ctx.oldVM.Spec.Volumes = append(ctx.oldVM.Spec.Volumes, instanceStorageVolumes...) + instanceStorageVolumes[0].Name += updateSuffix + ctx.vm.Spec.Volumes = append(ctx.vm.Spec.Volumes, instanceStorageVolumes...) + } + + if args.isSysprepFeatureEnabled { + Expect(os.Setenv(lib.WindowsSysprepFSS, "true")).To(Succeed()) + } + if args.isSysprepTransportUsed { + ctx.vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOff + ctx.vm.Spec.Bootstrap.Sysprep = &vmopv1.VirtualMachineBootstrapSysprepSpec{} + } + + if args.oldPowerState != "" { + ctx.oldVM.Spec.PowerState = args.oldPowerState + } + if args.newPowerState != "" || args.newPowerStateEmptyAllowed { + ctx.vm.Spec.PowerState = args.newPowerState + } + + ctx.oldVM.Spec.NextRestartTime = args.lastRestartTime + ctx.vm.Spec.NextRestartTime = args.nextRestartTime + + var err error + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vm) + Expect(err).ToNot(HaveOccurred()) + ctx.WebhookRequestContext.OldObj, err = builder.ToUnstructured(ctx.oldVM) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateUpdate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(HaveSuffix(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(true) + }) + + AfterEach(func() { + ctx = nil + Expect(os.Unsetenv(lib.WcpFaultDomainsFSS)).To(Succeed()) + Expect(os.Unsetenv(lib.WindowsSysprepFSS)).To(Succeed()) + }) + + msg := "field is immutable" + volumesPath := field.NewPath("spec", "volumes") + powerStatePath := field.NewPath("spec", "powerState") + nextRestartTimePath := field.NewPath("spec", "nextRestartTime") + + DescribeTable("update table", validateUpdate, + Entry("should allow", updateArgs{}, true, nil, nil), + + Entry("should deny image name change", updateArgs{changeImageName: true}, false, msg, nil), + Entry("should deny class name change", updateArgs{changeClassName: true}, false, msg, nil), + Entry("should deny storageClass change", updateArgs{changeStorageClass: true}, false, msg, nil), + Entry("should deny resourcePolicy change", updateArgs{changeResourcePolicy: true}, false, msg, nil), + + Entry("should allow initial zone assignment", updateArgs{assignZoneName: true}, true, nil, nil), + Entry("should allow zone name change when WCP FaultDomains FSS is disabled", updateArgs{changeZoneName: true}, true, nil, nil), + + Entry("should deny instance storage volume name change, when user is SSO user", updateArgs{changeInstanceStorageVolume: true}, false, + field.Forbidden(volumesPath, "adding or modifying instance storage volume claim(s) is not allowed").Error(), nil), + Entry("should deny adding new instance storage volume, when user is SSO user", updateArgs{withInstanceStorageVolumes: true}, false, + field.Forbidden(volumesPath, "adding or modifying instance storage volume claim(s) is not allowed").Error(), nil), + Entry("should allow adding new instance storage volume, when user type is service user", updateArgs{withInstanceStorageVolumes: true, isServiceUser: true}, true, nil, nil), + Entry("should allow instance storage volume name change, when user type is service user", updateArgs{changeInstanceStorageVolume: true, isServiceUser: true}, true, nil, nil), + + Entry("should allow sysprep when FSS is enabled", updateArgs{isSysprepFeatureEnabled: true, isSysprepTransportUsed: true}, true, nil, nil), + Entry("should disallow sysprep when FSS is disabled", updateArgs{isSysprepFeatureEnabled: false, isSysprepTransportUsed: true}, false, + field.Invalid(field.NewPath("spec", "bootstrap", "sysprep"), "sysprep", "the sysprep feature is not enabled").Error(), nil), + Entry("should not error if sysprep FSS is disabled when sysprep is not used", updateArgs{isSysprepFeatureEnabled: false, isSysprepTransportUsed: false}, true, nil, nil), + + Entry("should allow updating suspended VM to powered on", updateArgs{oldPowerState: vmopv1.VirtualMachinePowerStateSuspended, newPowerState: vmopv1.VirtualMachinePowerStateOn}, true, + nil, nil), + Entry("should allow updating suspended VM to powered off", updateArgs{oldPowerState: vmopv1.VirtualMachinePowerStateSuspended, newPowerState: vmopv1.VirtualMachinePowerStateOff}, true, + nil, nil), + Entry("should disallow updating powered off VM to suspended", updateArgs{oldPowerState: vmopv1.VirtualMachinePowerStateOff, newPowerState: vmopv1.VirtualMachinePowerStateSuspended}, false, + field.Invalid(powerStatePath, vmopv1.VirtualMachinePowerStateSuspended, "cannot suspend a VM that is powered off").Error(), nil), + + Entry("should allow updating VM with non-empty, valid nextRestartTime value", updateArgs{ + nextRestartTime: time.Now().UTC().Format(time.RFC3339Nano)}, true, nil, nil), + Entry("should allow updating VM with empty nextRestartTime value if existing value is also empty", + updateArgs{nextRestartTime: ""}, true, nil, nil), + Entry("should disallow updating VM with empty nextRestartTime value", + updateArgs{lastRestartTime: time.Now().UTC().Format(time.RFC3339Nano), nextRestartTime: ""}, false, + field.Invalid(nextRestartTimePath, "", "must be formatted as RFC3339Nano").Error(), nil), + Entry("should disallow updating VM with non-empty, valid nextRestartTime value if mutation webhooks were running", + updateArgs{nextRestartTime: "now"}, false, + field.Invalid(nextRestartTimePath, "now", "mutation webhooks are required to restart VM").Error(), nil), + Entry("should disallow updating VM with non-empty, invalid nextRestartTime value ", + updateArgs{nextRestartTime: "hello"}, false, + field.Invalid(nextRestartTimePath, "hello", "must be formatted as RFC3339Nano").Error(), nil), + ) + + When("the update is performed while object deletion", func() { + It("should allow the request", func() { + t := metav1.Now() + ctx.WebhookRequestContext.Obj.SetDeletionTimestamp(&t) + response := ctx.ValidateUpdate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} + +func unitTestsValidateDelete() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + + AfterEach(func() { + ctx = nil + }) + + When("the delete is performed", func() { + JustBeforeEach(func() { + response = ctx.ValidateDelete(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} diff --git a/webhooks/virtualmachine/v1alpha2/webhooks.go b/webhooks/virtualmachine/v1alpha2/webhooks.go new file mode 100644 index 000000000..5748f73e9 --- /dev/null +++ b/webhooks/virtualmachine/v1alpha2/webhooks.go @@ -0,0 +1,23 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha2 + +import ( + "github.com/pkg/errors" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha2/mutation" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha2/validation" +) + +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + if err := validation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize validation webhook") + } + if err := mutation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize mutation webhook") + } + return nil +} diff --git a/webhooks/virtualmachine/webhooks.go b/webhooks/virtualmachine/webhooks.go index e2115380f..49bf76683 100644 --- a/webhooks/virtualmachine/webhooks.go +++ b/webhooks/virtualmachine/webhooks.go @@ -1,9 +1,11 @@ -// Copyright (c) 2023 VMware, Inc. All Rights Reserved. +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package virtualmachine import ( + "github.com/pkg/errors" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/vmware-tanzu/vm-operator/pkg/context" @@ -11,5 +13,9 @@ import ( ) func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { - return v1alpha1.AddToManager(ctx, mgr) + if err := v1alpha1.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize v1alpha1 webhooks") + } + + return nil } diff --git a/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator.go b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator.go new file mode 100644 index 000000000..25f66a7f0 --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator.go @@ -0,0 +1,116 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation + +import ( + "encoding/json" + "net/http" + "reflect" + + "github.com/pkg/errors" + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" +) + +const ( + webHookName = "default" +) + +// -kubebuilder:webhook:path=/default-mutate-vmoperator-vmware-com-v1alpha2-virtualmachineclass,mutating=true,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachineclasses,verbs=create;update,versions=v1alpha2,name=default.mutating.virtualmachineclass.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineclass,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineclass/status,verbs=get + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewMutatingWebhook(ctx, mgr, webHookName, NewMutator(nil)) + if err != nil { + return errors.Wrapf(err, "failed to create mutation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewMutator returns the package's Mutator. +func NewMutator(_ client.Client) builder.Mutator { + return mutator{ + converter: runtime.DefaultUnstructuredConverter, + } +} + +type mutator struct { + converter runtime.UnstructuredConverter +} + +func (m mutator) Mutate(ctx *context.WebhookRequestContext) admission.Response { + vmClass, err := m.vmClassFromUnstructured(ctx.Obj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + var wasMutated bool + original := vmClass + modified := original.DeepCopy() + + switch ctx.Op { + case admissionv1.Create: + // No-op + case admissionv1.Update: + oldObj, err := m.vmClassFromUnstructured(ctx.OldObj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + if SetControllerName(modified, oldObj) { + wasMutated = true + } + } + + if !wasMutated { + return admission.Allowed("") + } + + rawOriginal, err := json.Marshal(original) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + rawModified, err := json.Marshal(modified) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + return admission.PatchResponseFromRaw(rawOriginal, rawModified) +} + +func (m mutator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachineClass{}).Name()) +} + +// vmClassFromUnstructured returns the VirtualMachineClass from the unstructured object. +func (m mutator) vmClassFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachineClass, error) { + vmClass := &vmopv1.VirtualMachineClass{} + if err := m.converter.FromUnstructured(obj.UnstructuredContent(), vmClass); err != nil { + return nil, err + } + return vmClass, nil +} + +// SetControllerName preserves the original value of spec.controllerName if +// an attempt is made to set the field with a non-empty value to empty. +// Return true if the field was preserved, otherwise false. +func SetControllerName(newObj, oldObj *vmopv1.VirtualMachineClass) bool { + if newObj.Spec.ControllerName == "" { + if oldObj.Spec.ControllerName != "" { + newObj.Spec.ControllerName = oldObj.Spec.ControllerName + return true + } + } + return false +} diff --git a/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_intg_test.go b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_intg_test.go new file mode 100644 index 000000000..a67e68c4c --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_intg_test.go @@ -0,0 +1,58 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + XDescribe("Invoking Mutation", intgTestsMutating) +} + +type intgMutatingWebhookContext struct { + builder.IntegrationTestContext + vmClass *vmopv1.VirtualMachineClass +} + +func newIntgMutatingWebhookContext() *intgMutatingWebhookContext { + ctx := &intgMutatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vmClass = builder.DummyVirtualMachineClassA2() + + return ctx +} + +func intgTestsMutating() { + var ( + ctx *intgMutatingWebhookContext + vmClass *vmopv1.VirtualMachineClass + ) + + BeforeEach(func() { + ctx = newIntgMutatingWebhookContext() + vmClass = ctx.vmClass.DeepCopy() + }) + AfterEach(func() { + ctx = nil + }) + + Describe("mutate", func() { + Context("placeholder", func() { + BeforeEach(func() { + }) + + It("should work", func() { + err := ctx.Client.Create(ctx, vmClass) + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) +} diff --git a/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_suite_test.go b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_suite_test.go new file mode 100644 index 000000000..9c3831732 --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineclass/v1alpha2/mutation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForMutatingWebhook( + mutation.AddToManager, + mutation.NewMutator, + "default.mutating.virtualmachineclass.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Mutating webhook suite", nil /*intgTests*/, uniTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_unit_test.go b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_unit_test.go new file mode 100644 index 000000000..70c1543b9 --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/mutation/virtualmachineclass_mutator_unit_test.go @@ -0,0 +1,89 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineclass/v1alpha2/mutation" +) + +func uniTests() { + Describe("Invoking Mutate", unitTestsMutating) +} + +type unitMutationWebhookContext struct { + builder.UnitTestContextForMutatingWebhook + vmClass *vmopv1.VirtualMachineClass +} + +func newUnitTestContextForMutatingWebhook() *unitMutationWebhookContext { + vmClass := builder.DummyVirtualMachineClassA2() + obj, err := builder.ToUnstructured(vmClass) + Expect(err).ToNot(HaveOccurred()) + + return &unitMutationWebhookContext{ + UnitTestContextForMutatingWebhook: *suite.NewUnitTestContextForMutatingWebhook(obj), + vmClass: vmClass, + } +} + +func unitTestsMutating() { + var ( + ctx *unitMutationWebhookContext + ) + + BeforeEach(func() { + ctx = newUnitTestContextForMutatingWebhook() + }) + AfterEach(func() { + ctx = nil + }) + + Describe("VirtualMachineClassMutator should admit updates when object is under deletion", func() { + Context("when update request comes in while deletion in progress ", func() { + It("should admit update operation", func() { + t := metav1.Now() + ctx.WebhookRequestContext.Obj.SetDeletionTimestamp(&t) + response := ctx.Mutate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(BeTrue()) + }) + }) + }) + + Describe("SetControllerName", func() { + When("an update attempts to set an empty spec.controllerName field to empty", func() { + It("should not indicate anything was mutated", func() { + oldObj, newObj := ctx.vmClass.DeepCopy(), ctx.vmClass.DeepCopy() + oldObj.Spec.ControllerName = "" + newObj.Spec.ControllerName = "" + Expect(mutation.SetControllerName(newObj, oldObj)).To(BeFalse()) + Expect(newObj.Spec.ControllerName).To(BeEmpty()) + }) + }) + When("an update attempts to set a non-empty spec.controllerName field to empty", func() { + It("should preserve the original value", func() { + oldObj, newObj := ctx.vmClass.DeepCopy(), ctx.vmClass.DeepCopy() + oldObj.Spec.ControllerName = "hello" + newObj.Spec.ControllerName = "" + Expect(mutation.SetControllerName(newObj, oldObj)).To(BeTrue()) + Expect(newObj.Spec.ControllerName).To(Equal("hello")) + }) + }) + When("an update attempts to set a non-empty spec.controllerName field to a new, non-empty value", func() { + It("should not mutate the request", func() { + oldObj, newObj := ctx.vmClass.DeepCopy(), ctx.vmClass.DeepCopy() + oldObj.Spec.ControllerName = "hello" + newObj.Spec.ControllerName = "world" + Expect(mutation.SetControllerName(newObj, oldObj)).To(BeFalse()) + Expect(newObj.Spec.ControllerName).To(Equal("world")) + }) + }) + }) +} diff --git a/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator.go b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator.go new file mode 100644 index 000000000..a514a4ec6 --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator.go @@ -0,0 +1,131 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "net/http" + "reflect" + + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/pkg/errors" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/common" +) + +const ( + webHookName = "default" + + invalidCPUReqMsg = "CPU request must not be larger than the CPU limit" + invalidMemoryReqMsg = "memory request must not be larger than the memory limit" +) + +// -kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha2-virtualmachineclass,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachineclasses,versions=v1alpha2,name=default.validating.virtualmachineclass.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineclasses,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineclasses/status,verbs=get + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewValidatingWebhook(ctx, mgr, webHookName, NewValidator(mgr.GetClient())) + if err != nil { + return errors.Wrapf(err, "failed to create VirtualMachineClass validation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewValidator returns the package's Validator. +func NewValidator(_ client.Client) builder.Validator { + return validator{ + converter: runtime.DefaultUnstructuredConverter, + } +} + +type validator struct { + converter runtime.UnstructuredConverter +} + +func (v validator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachineClass{}).Name()) +} + +func (v validator) ValidateCreate(ctx *context.WebhookRequestContext) admission.Response { + vmClass, err := v.vmClassFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + + fieldErrs = append(fieldErrs, v.validatePolicies(ctx, vmClass, field.NewPath("spec", "policies"))...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) ValidateDelete(*context.WebhookRequestContext) admission.Response { + return admission.Allowed("") +} + +func (v validator) ValidateUpdate(ctx *context.WebhookRequestContext) admission.Response { + var fieldErrs field.ErrorList + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) validatePolicies(ctx *context.WebhookRequestContext, vmClass *vmopv1.VirtualMachineClass, + polPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + request, limits := vmClass.Spec.Policies.Resources.Requests, vmClass.Spec.Policies.Resources.Limits + reqPath := polPath.Child("resources", "requests") + + // Validate the CPU request. + if !isRequestLimitValid(request.Cpu, limits.Cpu) { + allErrs = append(allErrs, field.Invalid(reqPath.Child("cpu"), request.Cpu.String(), invalidCPUReqMsg)) + } + + // Validate the memory request. + if !isRequestLimitValid(request.Memory, limits.Memory) { + allErrs = append(allErrs, field.Invalid(reqPath.Child("memory"), request.Memory.String(), invalidMemoryReqMsg)) + } + + // TODO: Validate req and limit against hardware configuration of the class + + return allErrs +} + +// vmClassFromUnstructured returns the VirtualMachineClass from the unstructured object. +func (v validator) vmClassFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachineClass, error) { + vmClass := &vmopv1.VirtualMachineClass{} + if err := v.converter.FromUnstructured(obj.UnstructuredContent(), vmClass); err != nil { + return nil, err + } + return vmClass, nil +} + +func isRequestLimitValid(request, limit resource.Quantity) bool { + return request.IsZero() || limit.IsZero() || request.Value() <= limit.Value() +} diff --git a/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_intg_test.go b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_intg_test.go new file mode 100644 index 000000000..c990c8006 --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_intg_test.go @@ -0,0 +1,102 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation/field" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + Describe("Invoking Create", intgTestsValidateCreate) + Describe("Invoking Delete", intgTestsValidateDelete) +} + +type intgValidatingWebhookContext struct { + builder.IntegrationTestContext + vmClass *vmopv1.VirtualMachineClass +} + +func newIntgValidatingWebhookContext() *intgValidatingWebhookContext { + ctx := &intgValidatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vmClass = builder.DummyVirtualMachineClassA2() + + return ctx +} + +func intgTestsValidateCreate() { + var ( + ctx *intgValidatingWebhookContext + ) + + validateCreate := func(expectedAllowed bool, expectedReason string, expectedErr error) { + vmClass := ctx.vmClass.DeepCopy() + + if !expectedAllowed { + vmClass.Spec.Policies.Resources.Requests.Memory = resource.MustParse("2Gi") + vmClass.Spec.Policies.Resources.Limits.Memory = resource.MustParse("1Gi") + } + + err := ctx.Client.Create(ctx, vmClass) + if expectedAllowed { + Expect(err).ToNot(HaveOccurred()) + } else { + Expect(err).To(HaveOccurred()) + } + if expectedReason != "" { + Expect(err.Error()).To(ContainSubstring(expectedReason)) + } + } + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + }) + AfterEach(func() { + ctx = nil + }) + + fieldPath := field.NewPath("spec", "policies", "resources", "requests", "memory") + invalidMemField := field.Invalid(fieldPath, "2Gi", "memory request must not be larger than the memory limit") + DescribeTable("create table", validateCreate, + Entry("should work", true, "", nil), + Entry("should not work for invalid", false, invalidMemField.Error(), nil), + ) +} + +func intgTestsValidateDelete() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + err = ctx.Client.Create(ctx, ctx.vmClass) + Expect(err).ToNot(HaveOccurred()) + }) + JustBeforeEach(func() { + err = ctx.Client.Delete(suite, ctx.vmClass) + }) + AfterEach(func() { + err = nil + ctx = nil + }) + + When("delete is performed", func() { + It("should allow the request", func() { + Expect(ctx.Namespace).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + }) + }) +} diff --git a/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_suite_test.go b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_suite_test.go new file mode 100644 index 000000000..c66b4fc13 --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineclass/v1alpha2/validation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForValidatingWebhook( + validation.AddToManager, + validation.NewValidator, + "default.validating.virtualmachineclass.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Validation webhook suite", nil /*intgTests*/, unitTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_unit_test.go b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_unit_test.go new file mode 100644 index 000000000..43055331e --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/validation/virtualmachineclass_validator_unit_test.go @@ -0,0 +1,223 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func unitTests() { + Describe("Invoking ValidateCreate", unitTestsValidateCreate) + Describe("Invoking ValidateUpdate", unitTestsValidateUpdate) + Describe("Invoking ValidateDelete", unitTestsValidateDelete) +} + +type unitValidatingWebhookContext struct { + builder.UnitTestContextForValidatingWebhook + vmClass *vmopv1.VirtualMachineClass + oldVMClass *vmopv1.VirtualMachineClass +} + +func newUnitTestContextForValidatingWebhook(isUpdate bool) *unitValidatingWebhookContext { + vmClass := builder.DummyVirtualMachineClassA2() + obj, err := builder.ToUnstructured(vmClass) + Expect(err).ToNot(HaveOccurred()) + + var oldVMClass *vmopv1.VirtualMachineClass + var oldObj *unstructured.Unstructured + + if isUpdate { + oldVMClass = vmClass.DeepCopy() + oldObj, err = builder.ToUnstructured(oldVMClass) + Expect(err).ToNot(HaveOccurred()) + } + + return &unitValidatingWebhookContext{ + UnitTestContextForValidatingWebhook: *suite.NewUnitTestContextForValidatingWebhook(obj, oldObj), + vmClass: vmClass, + oldVMClass: oldVMClass, + } +} + +func unitTestsValidateCreate() { + var ( + ctx *unitValidatingWebhookContext + ) + + type createArgs struct { + invalidCPURequest bool + invalidMemoryRequest bool + noCPULimit bool + noMemoryLimit bool + } + + validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + var err error + + if args.invalidCPURequest { + ctx.vmClass.Spec.Policies.Resources.Requests.Cpu = resource.MustParse("2Gi") + ctx.vmClass.Spec.Policies.Resources.Limits.Cpu = resource.MustParse("1Gi") + } + if args.invalidMemoryRequest { + ctx.vmClass.Spec.Policies.Resources.Requests.Memory = resource.MustParse("2Gi") + ctx.vmClass.Spec.Policies.Resources.Limits.Memory = resource.MustParse("1Gi") + } + if args.noCPULimit { + ctx.vmClass.Spec.Policies.Resources.Limits.Cpu = resource.MustParse("0") + } + if args.noMemoryLimit { + ctx.vmClass.Spec.Policies.Resources.Limits.Memory = resource.MustParse("0") + } + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmClass) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(Equal(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + AfterEach(func() { + ctx = nil + }) + + reqPath := field.NewPath("spec", "policies", "resources", "requests") + invalidCPUField := field.Invalid(reqPath.Child("cpu"), "2Gi", "CPU request must not be larger than the CPU limit") + invalidMemField := field.Invalid(reqPath.Child("memory"), "2Gi", "memory request must not be larger than the memory limit") + DescribeTable("create table", validateCreate, + Entry("should allow valid", createArgs{}, true, nil, nil), + Entry("should allow no cpu limit", createArgs{noCPULimit: true}, true, nil, nil), + Entry("should allow no memory limit", createArgs{noMemoryLimit: true}, true, nil, nil), + Entry("should deny invalid cpu request", createArgs{invalidCPURequest: true}, false, invalidCPUField.Error(), nil), + Entry("should deny invalid memory request", createArgs{invalidMemoryRequest: true}, false, invalidMemField.Error(), nil), + ) +} + +func unitTestsValidateUpdate() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + type updateArgs struct { + changeHwCPU bool + changeHwMemory bool + changeCPU bool + changeMemory bool + } + + validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + var err error + + if args.changeHwCPU { + ctx.vmClass.Spec.Hardware.Cpus = 64 + } + if args.changeHwMemory { + ctx.vmClass.Spec.Hardware.Memory = resource.MustParse("5Gi") + } + if args.changeCPU { + ctx.vmClass.Spec.Policies.Resources.Requests.Cpu = resource.MustParse("5Gi") + ctx.vmClass.Spec.Policies.Resources.Limits.Cpu = resource.MustParse("10Gi") + } + if args.changeMemory { + ctx.vmClass.Spec.Policies.Resources.Requests.Memory = resource.MustParse("5Gi") + ctx.vmClass.Spec.Policies.Resources.Limits.Memory = resource.MustParse("10Gi") + } + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmClass) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateUpdate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + // Construct the complete reason here as ctx.vmClass.Spec is nil when initializing the DescribeTable below. + completeReason := field.Invalid(field.NewPath("spec"), ctx.vmClass.Spec, expectedReason).Error() + Expect(string(response.Result.Reason)).To(ContainSubstring(completeReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(ContainSubstring(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(true) + }) + AfterEach(func() { + ctx = nil + }) + + DescribeTable("update VM Class spec", validateUpdate, + Entry("should allow", updateArgs{}, true, nil, nil), + Entry("should allow hw cpu change", updateArgs{changeHwCPU: true}, true, nil, nil), + Entry("should allow hw memory change", updateArgs{changeHwMemory: true}, true, nil, nil), + Entry("should allow policy cpu change", updateArgs{changeCPU: true}, true, nil, nil), + Entry("should allow policy memory change", updateArgs{changeMemory: true}, true, nil, nil), + ) + + DescribeTable("update table", validateUpdate, + Entry("should allow", updateArgs{}, true, nil, nil), + Entry("should deny hw cpu change", updateArgs{changeHwCPU: true}, true, nil, nil), + Entry("should deny hw memory change", updateArgs{changeHwMemory: true}, true, nil, nil), + Entry("should deny policy cpu change", updateArgs{changeCPU: true}, true, nil, nil), + Entry("should deny policy memory change", updateArgs{changeMemory: true}, true, nil, nil), + ) + + When("the update is performed while object deletion", func() { + JustBeforeEach(func() { + t := metav1.Now() + ctx.WebhookRequestContext.Obj.SetDeletionTimestamp(&t) + response = ctx.ValidateUpdate(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} + +func unitTestsValidateDelete() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + AfterEach(func() { + ctx = nil + }) + + When("the delete is performed", func() { + JustBeforeEach(func() { + response = ctx.ValidateDelete(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} diff --git a/webhooks/virtualmachineclass/v1alpha2/webhooks.go b/webhooks/virtualmachineclass/v1alpha2/webhooks.go new file mode 100644 index 000000000..f727028ec --- /dev/null +++ b/webhooks/virtualmachineclass/v1alpha2/webhooks.go @@ -0,0 +1,23 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package virtualmachineclass + +import ( + "github.com/pkg/errors" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineclass/v1alpha2/mutation" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineclass/v1alpha2/validation" +) + +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + if err := validation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize validation webhook") + } + if err := mutation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize mutation webhook") + } + return nil +} diff --git a/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator.go b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator.go new file mode 100644 index 000000000..9d44a1e4e --- /dev/null +++ b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator.go @@ -0,0 +1,183 @@ +// Copyright (c) 2022-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "net/http" + "reflect" + + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + imgregv1a1 "github.com/vmware-tanzu/image-registry-operator-api/api/v1alpha1" + + "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/pkg/lib" + "github.com/vmware-tanzu/vm-operator/webhooks/common" +) + +const ( + webHookName = "default" +) + +// -kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha2-virtualmachinepublishrequest,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachinepublishrequests,versions=v1alpha2,name=default.validating.virtualmachinepublishrequest.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinepublishrequests,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinepublishrequests/status,verbs=get +// -kubebuilder:rbac:groups=imageregistry.vmware.com,resources=contentlibraries,verbs=get;list; + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewValidatingWebhook(ctx, mgr, webHookName, NewValidator(mgr.GetClient())) + if err != nil { + return errors.Wrapf(err, "failed to create VirtualMachinePublishRequest validation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewValidator returns the package's Validator. +func NewValidator(client client.Client) builder.Validator { + return validator{ + client: client, + converter: runtime.DefaultUnstructuredConverter, + } +} + +type validator struct { + client client.Client + converter runtime.UnstructuredConverter +} + +func (v validator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachinePublishRequest{}).Name()) +} + +func (v validator) ValidateCreate(ctx *context.WebhookRequestContext) admission.Response { + if !lib.IsWCPVMImageRegistryEnabled() { + return common.BuildValidationResponse(ctx, []string{"WCP_VM_Image_Registry feature not enabled"}, nil) + } + + vmpub, err := v.vmPublishRequestFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + + fieldErrs = append(fieldErrs, v.validateSource(ctx, vmpub)...) + fieldErrs = append(fieldErrs, v.validateTargetLocation(ctx, vmpub)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) ValidateDelete(*context.WebhookRequestContext) admission.Response { + return admission.Allowed("") +} + +func (v validator) ValidateUpdate(ctx *context.WebhookRequestContext) admission.Response { + vmpub, err := v.vmPublishRequestFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + oldVMpub, err := v.vmPublishRequestFromUnstructured(ctx.OldObj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + + // Check if an immutable field has been modified. + fieldErrs = append(fieldErrs, v.validateImmutableFields(vmpub, oldVMpub)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) validateSource(ctx *context.WebhookRequestContext, vmpub *vmopv1.VirtualMachinePublishRequest) field.ErrorList { + var allErrs field.ErrorList + + sourcePath := field.NewPath("spec").Child("source") + if apiVersion := vmpub.Spec.Source.APIVersion; apiVersion != "" { + v1a1GV, v1a2GV := v1alpha1.SchemeGroupVersion.String(), vmopv1.SchemeGroupVersion.String() + switch apiVersion { + case v1a1GV, v1a2GV: + default: + allErrs = append(allErrs, field.NotSupported(sourcePath.Child("apiVersion"), + apiVersion, []string{v1a1GV, v1a2GV, ""})) + } + } + + if kind := vmpub.Spec.Source.Kind; kind != reflect.TypeOf(vmopv1.VirtualMachine{}).Name() && kind != "" { + allErrs = append(allErrs, field.NotSupported(sourcePath.Child("kind"), + vmpub.Spec.Source.Kind, []string{reflect.TypeOf(vmopv1.VirtualMachine{}).Name(), ""})) + } + + return allErrs +} + +func (v validator) validateTargetLocation(ctx *context.WebhookRequestContext, vmpub *vmopv1.VirtualMachinePublishRequest) field.ErrorList { + var allErrs field.ErrorList + + targetLocationPath := field.NewPath("spec").Child("target"). + Child("location") + targetLocationName := vmpub.Spec.Target.Location.Name + targetLocationNamePath := targetLocationPath.Child("name") + if targetLocationName == "" { + allErrs = append(allErrs, field.Required(targetLocationNamePath, "")) + } + + if vmpub.Spec.Target.Location.APIVersion != imgregv1a1.GroupVersion.String() { + allErrs = append(allErrs, field.NotSupported(targetLocationPath.Child("apiVersion"), + vmpub.Spec.Target.Location.APIVersion, []string{imgregv1a1.GroupVersion.String(), ""})) + } + + if vmpub.Spec.Target.Location.Kind != reflect.TypeOf(imgregv1a1.ContentLibrary{}).Name() { + allErrs = append(allErrs, field.NotSupported(targetLocationPath.Child("kind"), + vmpub.Spec.Target.Location.Kind, []string{reflect.TypeOf(imgregv1a1.ContentLibrary{}).Name(), ""})) + } + + return allErrs +} + +func (v validator) validateImmutableFields(vmpub, oldvmpub *vmopv1.VirtualMachinePublishRequest) field.ErrorList { + var allErrs field.ErrorList + specPath := field.NewPath("spec") + + // all updates to source and target are not allowed. + // Otherwise, we may end up in a situation where multiple OVFs are published for a single VMPub. + allErrs = append(allErrs, validation.ValidateImmutableField(vmpub.Spec.Source, oldvmpub.Spec.Source, specPath.Child("source"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vmpub.Spec.Target, oldvmpub.Spec.Target, specPath.Child("target"))...) + + return allErrs +} + +// vmPublishRequestFromUnstructured returns the VirtualMachineService from the unstructured object. +func (v validator) vmPublishRequestFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachinePublishRequest, error) { + vmPubReq := &vmopv1.VirtualMachinePublishRequest{} + if err := v.converter.FromUnstructured(obj.UnstructuredContent(), vmPubReq); err != nil { + return nil, err + } + return vmPubReq, nil +} diff --git a/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_intg_test.go b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_intg_test.go new file mode 100644 index 000000000..f41c599ce --- /dev/null +++ b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_intg_test.go @@ -0,0 +1,158 @@ +// Copyright (c) 2022 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/pkg/lib" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + Describe("Invoking Create", intgTestsValidateCreate) + Describe("Invoking Update", intgTestsValidateUpdate) + Describe("Invoking Delete", intgTestsValidateDelete) +} + +type intgValidatingWebhookContext struct { + builder.IntegrationTestContext + vmPub *vmopv1.VirtualMachinePublishRequest + + oldIsWCPVMImageRegistryEnabledFunc func() bool +} + +func newIntgValidatingWebhookContext() *intgValidatingWebhookContext { + ctx := &intgValidatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vmPub = builder.DummyVirtualMachinePublishRequestA2("dummy-vmpub", ctx.Namespace, "dummy-vm", + "dummy-item", "dummy-cl") + vm := builder.DummyVirtualMachine() + vm.Name = "dummy-vm" + vm.Namespace = ctx.Namespace + + ctx.oldIsWCPVMImageRegistryEnabledFunc = lib.IsWCPVMImageRegistryEnabled + + return ctx +} + +func intgTestsValidateCreate() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + lib.IsWCPVMImageRegistryEnabled = func() bool { + return true + } + ctx = newIntgValidatingWebhookContext() + }) + + AfterEach(func() { + lib.IsWCPVMImageRegistryEnabled = ctx.oldIsWCPVMImageRegistryEnabledFunc + err = nil + ctx = nil + }) + + When("WCP_VM_Image_Registry is enabled: create is performed", func() { + It("should allow the request", func() { + Eventually(func() error { + return ctx.Client.Create(ctx, ctx.vmPub) + }).Should(Succeed()) + }) + }) + + When("WCP_VM_Image_Registry is not enabled", func() { + BeforeEach(func() { + lib.IsWCPVMImageRegistryEnabled = func() bool { + return false + } + err = ctx.Client.Create(ctx, ctx.vmPub) + }) + + It("should deny the request", func() { + Eventually(func() string { + if err = ctx.Client.Create(ctx, ctx.vmPub); err != nil { + return err.Error() + } + return "" + }).Should(ContainSubstring("WCP_VM_Image_Registry feature not enabled")) + }) + }) +} + +func intgTestsValidateUpdate() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + + Expect(ctx.Client.Create(ctx, ctx.vmPub)).To(Succeed()) + }) + + JustBeforeEach(func() { + err = ctx.Client.Update(suite, ctx.vmPub) + }) + + AfterEach(func() { + Expect(ctx.Client.Delete(ctx, ctx.vmPub)).To(Succeed()) + + err = nil + ctx = nil + }) + + When("update is performed with changed source name", func() { + BeforeEach(func() { + ctx.vmPub.Spec.Source.Name = "alternate-vm-name" + }) + It("should deny the request", func() { + Expect(err).To(HaveOccurred()) + }) + }) + + When("update is performed with changed target info", func() { + BeforeEach(func() { + ctx.vmPub.Spec.Target.Location.Name = "alternate-cl" + }) + It("should deny the request", func() { + Expect(err).To(HaveOccurred()) + }) + }) +} + +func intgTestsValidateDelete() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + + Expect(ctx.Client.Create(ctx, ctx.vmPub)).To(Succeed()) + }) + + JustBeforeEach(func() { + err = ctx.Client.Delete(suite, ctx.vmPub) + }) + + AfterEach(func() { + err = nil + ctx = nil + }) + + When("delete is performed", func() { + It("should allow the request", func() { + Expect(err).ToNot(HaveOccurred()) + }) + }) +} diff --git a/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_suite_test.go b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_suite_test.go new file mode 100644 index 000000000..af26e91bb --- /dev/null +++ b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2022-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinepublishrequest/v1alpha2/validation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForValidatingWebhook( + validation.AddToManager, + validation.NewValidator, + "default.validating.virtualmachinepublishrequest.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Validation webhook suite", nil /*intgTests*/, unitTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_unit_test.go b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_unit_test.go new file mode 100644 index 000000000..1285b8b3b --- /dev/null +++ b/webhooks/virtualmachinepublishrequest/v1alpha2/validation/virtualmachinepublishrequest_validator_unit_test.go @@ -0,0 +1,245 @@ +// Copyright (c) 2022-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + imgregv1a1 "github.com/vmware-tanzu/image-registry-operator-api/api/v1alpha1" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/controllers/contentlibrary/utils" + "github.com/vmware-tanzu/vm-operator/pkg/lib" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func unitTests() { + Describe("Invoking ValidateCreate", unitTestsValidateCreate) + Describe("Invoking ValidateUpdate", unitTestsValidateUpdate) + Describe("Invoking ValidateDelete", unitTestsValidateDelete) +} + +type unitValidatingWebhookContext struct { + builder.UnitTestContextForValidatingWebhook + vmPub *vmopv1.VirtualMachinePublishRequest + oldVMPub *vmopv1.VirtualMachinePublishRequest + vm *vmopv1.VirtualMachine + cl *imgregv1a1.ContentLibrary +} + +func newUnitTestContextForValidatingWebhook(isUpdate bool) *unitValidatingWebhookContext { + vm := builder.DummyVirtualMachineA2() + vm.Name = "dummy-vm" + vm.Namespace = "dummy-ns" + cl := builder.DummyContentLibrary("dummy-cl", "dummy-ns", "dummy-uuid") + + vmPub := builder.DummyVirtualMachinePublishRequestA2("dummy-vmpub", "dummy-ns", vm.Name, + "dummy-item", cl.Name) + obj, err := builder.ToUnstructured(vmPub) + Expect(err).ToNot(HaveOccurred()) + + var oldVMPub *vmopv1.VirtualMachinePublishRequest + var oldObj *unstructured.Unstructured + + if isUpdate { + oldVMPub = vmPub.DeepCopy() + oldObj, err = builder.ToUnstructured(oldVMPub) + Expect(err).ToNot(HaveOccurred()) + } + + return &unitValidatingWebhookContext{ + UnitTestContextForValidatingWebhook: *suite.NewUnitTestContextForValidatingWebhook(obj, oldObj, vm, cl), + vmPub: vmPub, + oldVMPub: oldVMPub, + vm: vm, + cl: cl, + } +} + +func unitTestsValidateCreate() { + var ( + ctx *unitValidatingWebhookContext + err error + + invalidAPIVersion = "vmoperator.vmware.com/v1" + ) + + type createArgs struct { + invalidSourceAPIVersion bool + invalidSourceKind bool + invalidTargetLocationAPIVersion bool + invalidTargetLocationKind bool + sourceNotFound bool + defaultSourceNotFound bool + defaultSourceExists bool + targetLocationNotWritable bool + targetLocationNameEmpty bool + targetLocationNotFound bool + targetItemAlreadyExists bool + } + + validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + if args.invalidSourceAPIVersion { + ctx.vmPub.Spec.Source.APIVersion = invalidAPIVersion + } + + if args.invalidSourceKind { + ctx.vmPub.Spec.Source.Kind = "Machine" + } + + if args.invalidTargetLocationAPIVersion { + ctx.vmPub.Spec.Target.Location.APIVersion = invalidAPIVersion + } + + if args.invalidTargetLocationKind { + ctx.vmPub.Spec.Target.Location.Kind = "ClusterContentLibrary" + } + + if args.sourceNotFound { + Expect(ctx.Client.Delete(ctx, ctx.vm)).To(Succeed()) + } + + if args.defaultSourceNotFound { + ctx.vmPub.Spec.Source.Name = "" + } + + if args.defaultSourceExists { + defaultVM := builder.DummyVirtualMachine() + defaultVM.Name = ctx.vmPub.Name + defaultVM.Namespace = ctx.vmPub.Namespace + Expect(ctx.Client.Create(ctx, defaultVM)).To(Succeed()) + } + + if args.targetLocationNotWritable { + ctx.cl.Spec.Writable = false + Expect(ctx.Client.Update(ctx, ctx.cl)).To(Succeed()) + } + + if args.targetLocationNameEmpty { + ctx.vmPub.Spec.Target.Location.Name = "" + } + + if args.targetLocationNotFound { + Expect(ctx.Client.Delete(ctx, ctx.cl)).To(Succeed()) + } + + if args.targetItemAlreadyExists { + clItem := utils.DummyContentLibraryItem("dummy-item", ctx.vmPub.Namespace) + Expect(ctx.Client.Create(ctx, clItem)).To(Succeed()) + clItem.Status.Name = ctx.vmPub.Spec.Target.Item.Name + Expect(ctx.Client.Status().Update(ctx, clItem)).To(Succeed()) + } + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmPub) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(ContainSubstring(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + lib.IsWCPVMImageRegistryEnabled = func() bool { + return true + } + }) + + AfterEach(func() { + ctx = nil + }) + + sourcePath := field.NewPath("spec").Child("source") + targetLocationPath := field.NewPath("spec").Child("target", "location") + DescribeTable("create table", validateCreate, + Entry("should allow valid", createArgs{}, true, nil, nil), + Entry("should deny invalid source API version", createArgs{invalidSourceAPIVersion: true}, false, + field.NotSupported(sourcePath.Child("apiVersion"), invalidAPIVersion, + []string{"vmoperator.vmware.com/v1alpha1", "vmoperator.vmware.com/v1alpha2", ""}).Error(), nil), + Entry("should deny invalid source kind", createArgs{invalidSourceKind: true}, false, + field.NotSupported(sourcePath.Child("kind"), "Machine", + []string{"VirtualMachine", ""}).Error(), nil), + Entry("should deny invalid target location API version", createArgs{invalidTargetLocationAPIVersion: true}, false, + field.NotSupported(targetLocationPath.Child("apiVersion"), invalidAPIVersion, + []string{"imageregistry.vmware.com/v1alpha1", ""}).Error(), nil), + Entry("should deny invalid target location kind", createArgs{invalidTargetLocationKind: true}, false, + field.NotSupported(targetLocationPath.Child("kind"), "ClusterContentLibrary", + []string{"ContentLibrary", ""}).Error(), nil), + Entry("should deny if target location name is empty", createArgs{targetLocationNameEmpty: true}, false, + field.Required(targetLocationPath.Child("name"), "").Error(), nil), + ) +} + +func unitTestsValidateUpdate() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(true) + }) + + AfterEach(func() { + ctx = nil + }) + + JustBeforeEach(func() { + response = ctx.ValidateUpdate(&ctx.WebhookRequestContext) + }) + + Context("Source/Target is updated", func() { + var err error + + BeforeEach(func() { + ctx.vmPub.Spec.Source.Name = "updated-vm" + ctx.vmPub.Spec.Target.Location.Name = "updated-cl" + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmPub) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should not allow the request", func() { + Expect(response.Allowed).To(BeFalse()) + Expect(response.Result).ToNot(BeNil()) + Expect(string(response.Result.Reason)).To(ContainSubstring("field is immutable")) + }) + }) +} + +func unitTestsValidateDelete() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + + AfterEach(func() { + ctx = nil + }) + + When("the delete is performed", func() { + JustBeforeEach(func() { + response = ctx.ValidateDelete(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} diff --git a/webhooks/virtualmachinepublishrequest/v1alpha2/webhooks.go b/webhooks/virtualmachinepublishrequest/v1alpha2/webhooks.go new file mode 100644 index 000000000..213223611 --- /dev/null +++ b/webhooks/virtualmachinepublishrequest/v1alpha2/webhooks.go @@ -0,0 +1,19 @@ +// Copyright (c) 2022-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package virtualmachinepublishrequest + +import ( + "github.com/pkg/errors" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinepublishrequest/v1alpha2/validation" +) + +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + if err := validation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize validation webhook") + } + return nil +} diff --git a/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator.go b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator.go new file mode 100644 index 000000000..ba9462982 --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator.go @@ -0,0 +1,89 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation + +import ( + "encoding/json" + "net/http" + "reflect" + + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" +) + +const ( + webHookName = "default" +) + +// -kubebuilder:webhook:path=/default-mutate-vmoperator-vmware-com-v1alpha2-virtualmachineservice,mutating=true,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachineservices,verbs=create;update,versions=v1alpha2,name=default.mutating.virtualmachineservice.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineservice,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineservice/status,verbs=get + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewMutatingWebhook(ctx, mgr, webHookName, NewMutator(nil)) + if err != nil { + return errors.Wrapf(err, "failed to create mutation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewMutator returns the package's Mutator. +func NewMutator(_ client.Client) builder.Mutator { + return mutator{ + converter: runtime.DefaultUnstructuredConverter, + } +} + +type mutator struct { + converter runtime.UnstructuredConverter +} + +func (m mutator) Mutate(ctx *context.WebhookRequestContext) admission.Response { + vmService, err := m.vmServiceFromUnstructured(ctx.Obj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + var wasMutated bool + original := vmService + modified := original.DeepCopy() + + if !wasMutated { + return admission.Allowed("") + } + + rawOriginal, err := json.Marshal(original) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + rawModified, err := json.Marshal(modified) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + return admission.PatchResponseFromRaw(rawOriginal, rawModified) +} + +func (m mutator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachineService{}).Name()) +} + +// vmServiceFromUnstructured returns the VirtualMachineService from the unstructured object. +func (m mutator) vmServiceFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachineService, error) { + vmService := &vmopv1.VirtualMachineService{} + if err := m.converter.FromUnstructured(obj.UnstructuredContent(), vmService); err != nil { + return nil, err + } + return vmService, nil +} diff --git a/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_intg_test.go b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_intg_test.go new file mode 100644 index 000000000..702ee1c6d --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_intg_test.go @@ -0,0 +1,60 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + XDescribe("Invoking Mutation", intgTestsMutating) +} + +type intgMutatingWebhookContext struct { + builder.IntegrationTestContext + vmService *vmopv1.VirtualMachineService +} + +func newIntgMutatingWebhookContext() *intgMutatingWebhookContext { + ctx := &intgMutatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vmService = builder.DummyVirtualMachineServiceA2() + ctx.vmService.Namespace = ctx.Namespace + + return ctx +} + +func intgTestsMutating() { + var ( + ctx *intgMutatingWebhookContext + vmService *vmopv1.VirtualMachineService + ) + + BeforeEach(func() { + ctx = newIntgMutatingWebhookContext() + vmService = ctx.vmService.DeepCopy() + }) + AfterEach(func() { + ctx = nil + }) + + Describe("mutate", func() { + Context("placeholder", func() { + BeforeEach(func() { + }) + + It("should work", func() { + err := ctx.Client.Create(ctx, vmService) + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) +} diff --git a/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_suite_test.go b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_suite_test.go new file mode 100644 index 000000000..2571a88fa --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineservice/v1alpha2/mutation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForMutatingWebhook( + mutation.AddToManager, + mutation.NewMutator, + "default.mutating.virtualmachineservice.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Mutating webhook suite", intgTests, uniTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_unit_test.go b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_unit_test.go new file mode 100644 index 000000000..21c886a04 --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/mutation/virtualmachineservice_mutator_unit_test.go @@ -0,0 +1,58 @@ +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package mutation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func uniTests() { + Describe("Invoking Mutate", unitTestsMutating) +} + +type unitMutationWebhookContext struct { + builder.UnitTestContextForMutatingWebhook + vmService *vmopv1.VirtualMachineService +} + +func newUnitTestContextForMutatingWebhook() *unitMutationWebhookContext { + vmService := builder.DummyVirtualMachineServiceA2() + obj, err := builder.ToUnstructured(vmService) + Expect(err).ToNot(HaveOccurred()) + + return &unitMutationWebhookContext{ + UnitTestContextForMutatingWebhook: *suite.NewUnitTestContextForMutatingWebhook(obj), + vmService: vmService, + } +} + +func unitTestsMutating() { + var ( + ctx *unitMutationWebhookContext + ) + + BeforeEach(func() { + ctx = newUnitTestContextForMutatingWebhook() + }) + AfterEach(func() { + ctx = nil + }) + + Describe("VirtualMachineServiceMutator should admit updates when object is under deletion", func() { + Context("when update request comes in while deletion in progress ", func() { + It("should admit update operation", func() { + t := metav1.Now() + ctx.WebhookRequestContext.Obj.SetDeletionTimestamp(&t) + response := ctx.Mutate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(BeTrue()) + }) + }) + }) +} diff --git a/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator.go b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator.go new file mode 100644 index 000000000..df416ab18 --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator.go @@ -0,0 +1,298 @@ +// Copyright 2014 The Kubernetes Authors. +// Copyright (c) 2019-2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "fmt" + "net/http" + "reflect" + "strings" + + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + unversionedvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + utilnet "k8s.io/utils/net" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/common" +) + +const ( + webHookName = "default" +) + +var ( + supportedServiceType = sets.NewString( + string(vmopv1.VirtualMachineServiceTypeLoadBalancer), + string(vmopv1.VirtualMachineServiceTypeClusterIP), + string(vmopv1.VirtualMachineServiceTypeExternalName), + ) + + supportedPortProtocols = sets.NewString( + string(corev1.ProtocolTCP), + string(corev1.ProtocolUDP), + string(corev1.ProtocolSCTP), + ) +) + +// -kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha2-virtualmachineservice,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachineservices,versions=v1alpha2,name=default.validating.virtualmachineservice.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineservices,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachineservices/status,verbs=get + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewValidatingWebhook(ctx, mgr, webHookName, NewValidator(mgr.GetClient())) + if err != nil { + return errors.Wrapf(err, "failed to create VirtualMachineService validation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewValidator returns the package's Validator. +func NewValidator(_ client.Client) builder.Validator { + return validator{ + converter: runtime.DefaultUnstructuredConverter, + } +} + +// NOTE: Much of this is based on the ValidateService() from k8s's pkg/apis/core/validation/validation.go +// since we should do our best at not allowing the create or update of a VirtualMachineService that cannot +// be transformed into a valid Service. + +type validator struct { + converter runtime.UnstructuredConverter +} + +func (v validator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachineService{}).Name()) +} + +func (v validator) ValidateCreate(ctx *context.WebhookRequestContext) admission.Response { + vmService, err := v.vmServiceFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + fieldErrs = append(fieldErrs, v.validateMetadata(ctx, vmService)...) + fieldErrs = append(fieldErrs, v.validateSpec(ctx, vmService)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) ValidateDelete(*context.WebhookRequestContext) admission.Response { + return admission.Allowed("") +} + +func (v validator) ValidateUpdate(ctx *context.WebhookRequestContext) admission.Response { + vmService, err := v.vmServiceFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + oldVMService, err := v.vmServiceFromUnstructured(ctx.OldObj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + fieldErrs = append(fieldErrs, v.validateAllowedChanges(ctx, vmService, oldVMService)...) + fieldErrs = append(fieldErrs, v.validateSpec(ctx, vmService)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) validateMetadata(ctx *context.WebhookRequestContext, vmService *vmopv1.VirtualMachineService) field.ErrorList { + mdPath := field.NewPath("metadata") + + var allErrs field.ErrorList + allErrs = append(allErrs, ValidateDNS1123Label(vmService.Name, mdPath.Child("name"))...) + + return allErrs +} + +func (v validator) validateSpec(ctx *context.WebhookRequestContext, vmService *vmopv1.VirtualMachineService) field.ErrorList { + var allErrs field.ErrorList + specPath := field.NewPath("spec") + + switch vmService.Spec.Type { + case vmopv1.VirtualMachineServiceTypeLoadBalancer: + if isHeadlessVMService(vmService) { + allErrs = append(allErrs, field.Invalid(specPath.Child("clusterIP"), vmService.Spec.ClusterIP, + "may not be set to 'None' for LoadBalancer services")) + } + + case vmopv1.VirtualMachineServiceTypeExternalName: + if vmService.Spec.ClusterIP != "" { + allErrs = append(allErrs, field.Forbidden(specPath.Child("clusterIP"), "may not be set for ExternalName services")) + } + + // The value (a CNAME) may have a trailing dot to denote it as fully qualified. + cname := strings.TrimSuffix(vmService.Spec.ExternalName, ".") + if len(cname) > 0 { + for _, msg := range validation.IsDNS1123Subdomain(cname) { + allErrs = append(allErrs, field.Invalid(specPath.Child("externalName"), cname, msg)) + } + } else { + allErrs = append(allErrs, field.Required(specPath.Child("externalName"), "")) + } + } + + allErrs = append(allErrs, validatePorts(vmService, specPath)...) + + if vmService.Spec.Selector != nil { + allErrs = append(allErrs, unversionedvalidation.ValidateLabels(vmService.Spec.Selector, specPath.Child("selector"))...) + } + + if clusterIP := vmService.Spec.ClusterIP; clusterIP != "" && clusterIP != corev1.ClusterIPNone { + for _, msg := range validation.IsValidIP(clusterIP) { + allErrs = append(allErrs, field.Invalid(specPath.Child("clusterIP"), clusterIP, msg)) + } + } + + if vmService.Spec.Type == "" { + allErrs = append(allErrs, field.Required(specPath.Child("type"), "")) + } else if !supportedServiceType.Has(string(vmService.Spec.Type)) { + allErrs = append(allErrs, field.NotSupported(specPath.Child("type"), vmService.Spec.Type, supportedServiceType.List())) + } + + if len(vmService.Spec.LoadBalancerSourceRanges) > 0 { + fldPath := specPath.Child("loadBalancerSourceRanges") + + if vmService.Spec.Type != vmopv1.VirtualMachineServiceTypeLoadBalancer { + allErrs = append(allErrs, field.Forbidden(fldPath, "may only be used when `type` is 'LoadBalancer'")) + } + + _, err := utilnet.ParseIPNets(vmService.Spec.LoadBalancerSourceRanges...) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath, fmt.Sprintf("%v", vmService.Spec.LoadBalancerSourceRanges), + "must be a list of IP ranges. For example, 10.240.0.0/24,10.250.0.0/24")) + } + } + + return allErrs +} + +func validatePorts(vmService *vmopv1.VirtualMachineService, specPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + portsPath := specPath.Child("ports") + + if len(vmService.Spec.Ports) == 0 && !isHeadlessVMService(vmService) && vmService.Spec.Type != vmopv1.VirtualMachineServiceTypeExternalName { + allErrs = append(allErrs, field.Required(portsPath, "")) + } + + allPortNames := sets.Set[string]{} + for i := range vmService.Spec.Ports { + portPath := portsPath.Index(i) + allErrs = append(allErrs, validateServicePort(&vmService.Spec.Ports[i], len(vmService.Spec.Ports) > 1, &allPortNames, portPath)...) + } + + // Check for duplicate Ports, considering (protocol,port) pairs. + ports := make(map[vmopv1.VirtualMachineServicePort]bool) + for i, port := range vmService.Spec.Ports { + portPath := portsPath.Index(i) + key := vmopv1.VirtualMachineServicePort{Protocol: port.Protocol, Port: port.Port} + _, found := ports[key] + if found { + allErrs = append(allErrs, field.Duplicate(portPath, key)) + } + ports[key] = true + } + + return allErrs +} + +func validateServicePort(sp *vmopv1.VirtualMachineServicePort, requireName bool, allNames *sets.Set[string], fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + + if requireName && len(sp.Name) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) + } else if len(sp.Name) != 0 { + allErrs = append(allErrs, ValidateDNS1123Label(sp.Name, fldPath.Child("name"))...) + if allNames.Has(sp.Name) { + allErrs = append(allErrs, field.Duplicate(fldPath.Child("name"), sp.Name)) + } else { + allNames.Insert(sp.Name) + } + } + + for _, msg := range validation.IsValidPortNum(int(sp.Port)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("port"), sp.Port, msg)) + } + + if len(sp.Protocol) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("protocol"), "")) + } else if !supportedPortProtocols.Has(sp.Protocol) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("protocol"), sp.Protocol, supportedPortProtocols.List())) + } + + for _, msg := range validation.IsValidPortNum(int(sp.TargetPort)) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("targetPort"), sp.TargetPort, msg)) + } + + return allErrs +} + +// There is much more nuance to this for a Service - like changing the Type - but let this be +// pretty simple for now. +func (v validator) validateAllowedChanges(ctx *context.WebhookRequestContext, vmService, oldVMService *vmopv1.VirtualMachineService) field.ErrorList { + var allErrs field.ErrorList + specPath := field.NewPath("spec") + + if vmService.Spec.Type != oldVMService.Spec.Type { + allErrs = append(allErrs, field.Forbidden(specPath.Child("type"), "field is immutable")) + } + + // Service's ClusterIP cannot be changed through updates so neither should ours. + if vmService.Spec.ClusterIP != oldVMService.Spec.ClusterIP { + allErrs = append(allErrs, field.Forbidden(specPath.Child("clusterIP"), "field is immutable")) + } + + return allErrs +} + +// vmServiceFromUnstructured returns the VirtualMachineService from the unstructured object. +func (v validator) vmServiceFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachineService, error) { + vmService := &vmopv1.VirtualMachineService{} + if err := v.converter.FromUnstructured(obj.UnstructuredContent(), vmService); err != nil { + return nil, err + } + return vmService, nil +} + +func isHeadlessVMService(vmService *vmopv1.VirtualMachineService) bool { + return vmService.Spec.ClusterIP == corev1.ClusterIPNone +} + +func ValidateDNS1123Label(value string, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + for _, msg := range validation.IsDNS1123Label(value) { + allErrs = append(allErrs, field.Invalid(fldPath, value, msg)) + } + return allErrs +} diff --git a/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_intg_test.go b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_intg_test.go new file mode 100644 index 000000000..59ebd464b --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_intg_test.go @@ -0,0 +1,126 @@ +// Copyright (c) 2019 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + Describe("Invoking Create", intgTestsValidateCreate) + Describe("Invoking Update", intgTestsValidateUpdate) + Describe("Invoking Delete", intgTestsValidateDelete) +} + +type intgValidatingWebhookContext struct { + builder.IntegrationTestContext + vmService *vmopv1.VirtualMachineService +} + +func newIntgValidatingWebhookContext() *intgValidatingWebhookContext { + ctx := &intgValidatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vmService = builder.DummyVirtualMachineServiceA2() + ctx.vmService.Namespace = ctx.Namespace + + return ctx +} + +func intgTestsValidateCreate() { + var ( + ctx *intgValidatingWebhookContext + ) + + validateCreate := func(expectedAllowed bool, expectedReason string, expectedErr error) { + vmService := ctx.vmService.DeepCopy() + + if !expectedAllowed { + vmService.Spec.Type = "" + } + + err := ctx.Client.Create(ctx, vmService) + if expectedAllowed { + Expect(err).ToNot(HaveOccurred()) + } else { + Expect(err).To(HaveOccurred()) + } + if expectedReason != "" { + Expect(err.Error()).To(ContainSubstring(expectedReason)) + } + } + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + }) + AfterEach(func() { + ctx = nil + }) + + DescribeTable("create table", validateCreate, + Entry("should work", true, "", nil), + Entry("should not work for invalid", false, "spec.type: Required value", nil), + ) +} + +func intgTestsValidateUpdate() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + err = ctx.Client.Create(ctx, ctx.vmService) + Expect(err).ToNot(HaveOccurred()) + }) + JustBeforeEach(func() { + err = ctx.Client.Update(suite, ctx.vmService) + }) + AfterEach(func() { + err = nil + ctx = nil + }) + + When("update is performed with changed ... placeholder", func() { + BeforeEach(func() { + }) + It("should deny the request", func() { + Expect(err).ToNot(HaveOccurred()) + }) + }) +} + +func intgTestsValidateDelete() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + err = ctx.Client.Create(ctx, ctx.vmService) + Expect(err).ToNot(HaveOccurred()) + }) + JustBeforeEach(func() { + err = ctx.Client.Delete(suite, ctx.vmService) + }) + AfterEach(func() { + err = nil + ctx = nil + }) + + When("delete is performed", func() { + It("should allow the request", func() { + Expect(ctx.Namespace).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + }) + }) +} diff --git a/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_suite_test.go b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_suite_test.go new file mode 100644 index 000000000..82e70aec1 --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2019 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineservice/v1alpha2/validation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForValidatingWebhook( + validation.AddToManager, + validation.NewValidator, + "default.validating.virtualmachineservice.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Validation webhook suite", nil /*intgTests*/, unitTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_unit_test.go b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_unit_test.go new file mode 100644 index 000000000..c17e2a2f4 --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/validation/virtualmachineservice_validator_unit_test.go @@ -0,0 +1,306 @@ +// Copyright (c) 2019 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func unitTests() { + Describe("Invoking ValidateCreate", unitTestsValidateCreate) + Describe("Invoking ValidateUpdate", unitTestsValidateUpdate) + Describe("Invoking ValidateDelete", unitTestsValidateDelete) +} + +type unitValidatingWebhookContext struct { + builder.UnitTestContextForValidatingWebhook + vmService *vmopv1.VirtualMachineService + oldVMService *vmopv1.VirtualMachineService +} + +func newUnitTestContextForValidatingWebhook(isUpdate bool) *unitValidatingWebhookContext { + vmService := builder.DummyVirtualMachineServiceA2() + obj, err := builder.ToUnstructured(vmService) + Expect(err).ToNot(HaveOccurred()) + + var oldVMService *vmopv1.VirtualMachineService + var oldObj *unstructured.Unstructured + + if isUpdate { + oldVMService = vmService.DeepCopy() + oldObj, err = builder.ToUnstructured(oldVMService) + Expect(err).ToNot(HaveOccurred()) + } + + return &unitValidatingWebhookContext{ + UnitTestContextForValidatingWebhook: *suite.NewUnitTestContextForValidatingWebhook(obj, oldObj), + vmService: vmService, + oldVMService: oldVMService, + } +} + +func unitTestsValidateCreate() { + var ( + ctx *unitValidatingWebhookContext + ) + + type createArgs struct { + invalidDNSName bool + emptyType bool + invalidType bool + invalidPorts bool + invalidSelector bool + invalidClusterIP bool + invalidLBSourceRanges bool + invalidExternalName bool + } + + validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + var err error + + if args.invalidDNSName { + // Name that doesn't conform to DNS Label format "[a-z0-9]([-a-z0-9]*[a-z0-9])?" + ctx.vmService.Name = "MyVMService" + } + if args.emptyType { + ctx.vmService.Spec.Type = "" + } + if args.invalidType { + ctx.vmService.Spec.Type = "InvalidLB" + } + if args.invalidPorts { + ctx.vmService.Spec.Ports = nil + } + if args.invalidSelector { + ctx.vmService.Spec.Selector = map[string]string{"THIS_NOT_VALID!": "foo"} + } + if args.invalidClusterIP { + ctx.vmService.Spec.ClusterIP = "100.1000.1.1" + } + if args.invalidLBSourceRanges { + ctx.vmService.Spec.Type = vmopv1.VirtualMachineServiceTypeLoadBalancer + ctx.vmService.Spec.LoadBalancerSourceRanges = []string{"10.1.1.1/42"} + } + if args.invalidExternalName { + ctx.vmService.Spec.Type = vmopv1.VirtualMachineServiceTypeExternalName + ctx.vmService.Spec.ExternalName = "InValid!" + } + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmService) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(ContainSubstring(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + AfterEach(func() { + ctx = nil + }) + + DescribeTable("create table", validateCreate, + Entry("should allow valid", createArgs{}, true, nil, nil), + Entry("should deny invalid name", createArgs{invalidDNSName: true}, false, "metadata.name: Invalid value: \"MyVMService\": a lowercase RFC 1123 label must consist of lower case alphanumeric character", nil), + Entry("should deny no type", createArgs{emptyType: true}, false, "spec.type: Required value", nil), + Entry("should deny invalid type", createArgs{invalidType: true}, false, "spec.type: Unsupported value: \"InvalidLB\":", nil), + Entry("should deny invalid ports", createArgs{invalidPorts: true}, false, "spec.ports: Required value", nil), + Entry("should deny invalid selector", createArgs{invalidSelector: true}, false, "spec.selector: Invalid value: \"THIS_NOT_VALID!\": name part must consist of alphanumeric characters", nil), + Entry("should deny invalid ClusterIP", createArgs{invalidClusterIP: true}, false, "spec.clusterIP: Invalid value: \"100.1000.1.1\": must be a valid IP address", nil), + Entry("should deny invalid LoadBalancerSourceRanges", createArgs{invalidLBSourceRanges: true}, false, "spec.loadBalancerSourceRanges: Invalid value: \"[10.1.1.1/42]", nil), + Entry("should deny invalid ExternalName", createArgs{invalidExternalName: true}, false, "spec.externalName: Invalid value: \"InValid!\": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters", nil), + ) + + validatePortCreate := func(expectedReason string, ports []vmopv1.VirtualMachineServicePort) { + var err error + + ctx.vmService.Spec.Ports = ports + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmService) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + if expectedReason != "" { + Expect(response.Allowed).To(BeFalse()) + Expect(string(response.Result.Reason)).To(ContainSubstring(expectedReason)) + } else { + Expect(response.Allowed).To(BeTrue()) + } + } + + DescribeTable("create service ports", validatePortCreate, + Entry("should allow valid port", "", + []vmopv1.VirtualMachineServicePort{ + { + Name: "http", + Protocol: "TCP", + Port: 80, + TargetPort: 8080, + }, + }, + ), + Entry("should deny invalid name", "spec.ports[0].name: Invalid value: \"INVALID\"", + []vmopv1.VirtualMachineServicePort{ + { + Name: "INVALID", + }, + }, + ), + Entry("should deny invalid port", "spec.ports[0].port: Invalid value: 100000:", + []vmopv1.VirtualMachineServicePort{ + { + Port: 100000, + }, + }, + ), + Entry("should deny invalid protocol", "spec.ports[0].protocol: Unsupported value: \"INVALID\"", + []vmopv1.VirtualMachineServicePort{ + { + Protocol: "INVALID", + }, + }, + ), + Entry("should deny invalid target port", "spec.ports[0].targetPort: Invalid value: 200000:", + []vmopv1.VirtualMachineServicePort{ + { + TargetPort: 200000, + }, + }, + ), + Entry("should deny duplicate names", "spec.ports[1].name: Duplicate value: \"port1\"", + []vmopv1.VirtualMachineServicePort{ + { + Name: "port1", + Protocol: "TCP", + Port: 80, + TargetPort: 8080, + }, + { + Name: "port1", + Protocol: "TCP", + Port: 433, + TargetPort: 6443, + }, + }, + ), + Entry("should deny duplicate protocol/port", "spec.ports[1]: Duplicate value: v1alpha2.VirtualMachineServicePort", + []vmopv1.VirtualMachineServicePort{ + { + Name: "port1", + Protocol: "TCP", + Port: 80, + TargetPort: 8080, + }, + { + Name: "port2", + Protocol: "TCP", + Port: 80, + TargetPort: 8080, + }, + }, + ), + ) +} + +func unitTestsValidateUpdate() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + type updateArgs struct { + updateType bool + updateClusterIP bool + } + + validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + var err error + + if args.updateType { + ctx.vmService.Spec.Type = vmopv1.VirtualMachineServiceTypeClusterIP + } + if args.updateClusterIP { + ctx.vmService.Spec.ClusterIP = "9.9.9.9" + } + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmService) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateUpdate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(Equal(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(true) + }) + AfterEach(func() { + ctx = nil + }) + + DescribeTable("update table", validateUpdate, + Entry("should allow", updateArgs{}, true, nil, nil), + Entry("should deny Type change", updateArgs{updateType: true}, false, "spec.type: Forbidden: field is immutable", nil), + Entry("should deny ClusterIP change", updateArgs{updateClusterIP: true}, false, "spec.clusterIP: Forbidden: field is immutable", nil), + ) + + When("the update is performed while object deletion", func() { + JustBeforeEach(func() { + t := metav1.Now() + ctx.WebhookRequestContext.Obj.SetDeletionTimestamp(&t) + response = ctx.ValidateUpdate(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} + +func unitTestsValidateDelete() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + AfterEach(func() { + ctx = nil + }) + + When("the delete is performed", func() { + JustBeforeEach(func() { + response = ctx.ValidateDelete(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} diff --git a/webhooks/virtualmachineservice/v1alpha2/webhooks.go b/webhooks/virtualmachineservice/v1alpha2/webhooks.go new file mode 100644 index 000000000..8403d7106 --- /dev/null +++ b/webhooks/virtualmachineservice/v1alpha2/webhooks.go @@ -0,0 +1,23 @@ +// Copyright (c) 2023 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha2 + +import ( + "github.com/pkg/errors" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineservice/v1alpha2/mutation" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineservice/v1alpha2/validation" +) + +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + if err := validation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize validation webhook") + } + if err := mutation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize mutation webhook") + } + return nil +} diff --git a/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator.go b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator.go new file mode 100644 index 000000000..16a7dd9b3 --- /dev/null +++ b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator.go @@ -0,0 +1,180 @@ +// Copyright (c) 2019-2021 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation + +import ( + "net/http" + "reflect" + + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/pkg/errors" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + + "github.com/vmware-tanzu/vm-operator/pkg/builder" + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/common" +) + +const ( + webHookName = "default" +) + +// -kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha2-virtualmachinesetresourcepolicy,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachinesetresourcepolicies,versions=v1alpha2,name=default.validating.virtualmachinesetresourcepolicy.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinesetresourcepolicies,verbs=get;list +// -kubebuilder:rbac:groups=vmoperator.vmware.com,resources=virtualmachinesetresourcepolicies/status,verbs=get + +// AddToManager adds the webhook to the provided manager. +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + hook, err := builder.NewValidatingWebhook(ctx, mgr, webHookName, NewValidator(mgr.GetClient())) + if err != nil { + return errors.Wrapf(err, "failed to create VirtualMachineSetResourcePolicy validation webhook") + } + mgr.GetWebhookServer().Register(hook.Path, hook) + + return nil +} + +// NewValidator returns the package's Validator. +func NewValidator(_ client.Client) builder.Validator { + return validator{ + converter: runtime.DefaultUnstructuredConverter, + } +} + +type validator struct { + converter runtime.UnstructuredConverter +} + +func (v validator) For() schema.GroupVersionKind { + return vmopv1.SchemeGroupVersion.WithKind(reflect.TypeOf(vmopv1.VirtualMachineSetResourcePolicy{}).Name()) +} + +func (v validator) ValidateCreate(ctx *context.WebhookRequestContext) admission.Response { + vmRP, err := v.vmRPFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + fieldErrs = append(fieldErrs, v.validateSpec(ctx, vmRP)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) ValidateDelete(*context.WebhookRequestContext) admission.Response { + return admission.Allowed("") +} + +func (v validator) ValidateUpdate(ctx *context.WebhookRequestContext) admission.Response { + vmRP, err := v.vmRPFromUnstructured(ctx.Obj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + oldVMRP, err := v.vmRPFromUnstructured(ctx.OldObj) + if err != nil { + return webhook.Errored(http.StatusBadRequest, err) + } + + var fieldErrs field.ErrorList + fieldErrs = append(fieldErrs, v.validateAllowedChanges(ctx, vmRP, oldVMRP)...) + + validationErrs := make([]string, 0, len(fieldErrs)) + for _, fieldErr := range fieldErrs { + validationErrs = append(validationErrs, fieldErr.Error()) + } + + return common.BuildValidationResponse(ctx, validationErrs, nil) +} + +func (v validator) validateSpec(ctx *context.WebhookRequestContext, vmRP *vmopv1.VirtualMachineSetResourcePolicy) field.ErrorList { + var fieldErrs field.ErrorList + specPath := field.NewPath("spec") + + fieldErrs = append(fieldErrs, v.validateResourcePool(ctx, specPath.Child("resourcepool"), vmRP.Spec.ResourcePool)...) + fieldErrs = append(fieldErrs, v.validateFolder(ctx, specPath.Child("folder"), vmRP.Spec.Folder)...) + fieldErrs = append(fieldErrs, v.validateClusterModules(ctx, specPath.Child("clustermodules"), vmRP.Spec.ClusterModuleGroups)...) + + return fieldErrs +} + +func (v validator) validateResourcePool(ctx *context.WebhookRequestContext, fldPath *field.Path, rp vmopv1.ResourcePoolSpec) field.ErrorList { + var fieldErrs field.ErrorList + + reservation, limits := rp.Reservations, rp.Limits + reservationsPath := fldPath.Child("reservations") + + fieldErrs = append(fieldErrs, validateReservationAndLimit(reservationsPath.Child("cpu"), reservation.Cpu, limits.Cpu)...) + fieldErrs = append(fieldErrs, validateReservationAndLimit(reservationsPath.Child("memory"), reservation.Memory, limits.Memory)...) + + return fieldErrs +} + +func (v validator) validateFolder(ctx *context.WebhookRequestContext, specPath *field.Path, folder string) field.ErrorList { + var fieldErrs field.ErrorList + return fieldErrs +} + +func (v validator) validateClusterModules(ctx *context.WebhookRequestContext, fldPath *field.Path, clusterModuleGroups []string) field.ErrorList { + var fieldErrs field.ErrorList + + groupNames := map[string]struct{}{} + for i, name := range clusterModuleGroups { + if _, ok := groupNames[name]; ok { + fieldErrs = append(fieldErrs, field.Duplicate(fldPath.Index(i).Child("clusterModuleGroups"), name)) + continue + } + groupNames[name] = struct{}{} + } + + return fieldErrs +} + +// validateAllowedChanges returns true only if immutable fields have not been modified. +func (v validator) validateAllowedChanges(ctx *context.WebhookRequestContext, vmRP, oldVMRP *vmopv1.VirtualMachineSetResourcePolicy) field.ErrorList { + var allErrs field.ErrorList + specPath := field.NewPath("spec") + + // Validate all fields under spec which are not allowed to change. + allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.ResourcePool, oldVMRP.Spec.ResourcePool, specPath.Child("resourcepool"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.Folder, oldVMRP.Spec.Folder, specPath.Child("folder"))...) + allErrs = append(allErrs, validation.ValidateImmutableField(vmRP.Spec.ClusterModuleGroups, oldVMRP.Spec.ClusterModuleGroups, specPath.Child("clustermodules"))...) + + return allErrs +} + +// vmRPFromUnstructured returns the VirtualMachineSetResourcePolicy from the unstructured object. +func (v validator) vmRPFromUnstructured(obj runtime.Unstructured) (*vmopv1.VirtualMachineSetResourcePolicy, error) { + vmRP := &vmopv1.VirtualMachineSetResourcePolicy{} + if err := v.converter.FromUnstructured(obj.UnstructuredContent(), vmRP); err != nil { + return nil, err + } + return vmRP, nil +} + +func validateReservationAndLimit(reservationPath *field.Path, reservation, limit resource.Quantity) field.ErrorList { + if reservation.IsZero() || limit.IsZero() || reservation.Value() <= limit.Value() { + return nil + } + + return field.ErrorList{ + field.Invalid(reservationPath, reservation.String(), "reservation value cannot exceed the limit value"), + } +} diff --git a/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_intg_test.go b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_intg_test.go new file mode 100644 index 000000000..7f0ca70b9 --- /dev/null +++ b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_intg_test.go @@ -0,0 +1,135 @@ +// Copyright (c) 2019 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation/field" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func intgTests() { + Describe("Invoking Create", intgTestsValidateCreate) + Describe("Invoking Update", intgTestsValidateUpdate) + Describe("Invoking Delete", intgTestsValidateDelete) +} + +type intgValidatingWebhookContext struct { + builder.IntegrationTestContext + vmRP *vmopv1.VirtualMachineSetResourcePolicy +} + +func newIntgValidatingWebhookContext() *intgValidatingWebhookContext { + ctx := &intgValidatingWebhookContext{ + IntegrationTestContext: *suite.NewIntegrationTestContext(), + } + + ctx.vmRP = builder.DummyVirtualMachineSetResourcePolicyA2() + ctx.vmRP.Namespace = ctx.Namespace + + return ctx +} + +func intgTestsValidateCreate() { + var ( + ctx *intgValidatingWebhookContext + ) + + validateCreate := func(expectedAllowed bool, expectedReason string, expectedErr error) { + vmRP := ctx.vmRP.DeepCopy() + + if !expectedAllowed { + vmRP.Spec.ResourcePool.Reservations.Memory = resource.MustParse("2Gi") + vmRP.Spec.ResourcePool.Limits.Memory = resource.MustParse("1Gi") + } + + err := ctx.Client.Create(ctx, vmRP) + if expectedAllowed { + Expect(err).ToNot(HaveOccurred()) + } else { + Expect(err).To(HaveOccurred()) + } + if expectedReason != "" { + Expect(err.Error()).To(ContainSubstring(expectedReason)) + } + } + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + }) + AfterEach(func() { + ctx = nil + }) + + fieldPath := field.NewPath("spec", "resourcepool", "reservations", "memory") + DescribeTable("create table", validateCreate, + Entry("should work", true, "", nil), + Entry("should not work for invalid", false, + field.Invalid(fieldPath, "2Gi", "reservation value cannot exceed the limit value").Error(), nil), + ) +} + +func intgTestsValidateUpdate() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + err = ctx.Client.Create(ctx, ctx.vmRP) + Expect(err).ToNot(HaveOccurred()) + }) + JustBeforeEach(func() { + err = ctx.Client.Update(suite, ctx.vmRP) + }) + AfterEach(func() { + err = nil + ctx = nil + }) + + When("update is performed with changed cpu request", func() { + BeforeEach(func() { + ctx.vmRP.Spec.ResourcePool.Reservations.Memory = resource.MustParse("10Gi") + }) + It("should deny the request", func() { + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("field is immutable")) + }) + }) +} + +func intgTestsValidateDelete() { + var ( + err error + ctx *intgValidatingWebhookContext + ) + + BeforeEach(func() { + ctx = newIntgValidatingWebhookContext() + err = ctx.Client.Create(ctx, ctx.vmRP) + Expect(err).ToNot(HaveOccurred()) + }) + JustBeforeEach(func() { + err = ctx.Client.Delete(suite, ctx.vmRP) + }) + AfterEach(func() { + err = nil + ctx = nil + }) + + When("delete is performed", func() { + It("should allow the request", func() { + Expect(ctx.Namespace).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + }) + }) +} diff --git a/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_suite_test.go b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_suite_test.go new file mode 100644 index 000000000..41bf6e075 --- /dev/null +++ b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_suite_test.go @@ -0,0 +1,28 @@ +// Copyright (c) 2019 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + + "github.com/vmware-tanzu/vm-operator/test/builder" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation" +) + +// suite is used for unit and integration testing this webhook. +var suite = builder.NewTestSuiteForValidatingWebhook( + validation.AddToManager, + validation.NewValidator, + "default.validating.virtualmachinesetresourcepolicy.v1alpha2.vmoperator.vmware.com") + +func TestWebhook(t *testing.T) { + _ = intgTests + suite.Register(t, "Validation webhook suite", nil /*intgTests*/, unitTests) +} + +var _ = BeforeSuite(suite.BeforeSuite) + +var _ = AfterSuite(suite.AfterSuite) diff --git a/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_unit_test.go b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_unit_test.go new file mode 100644 index 000000000..d91dca5dc --- /dev/null +++ b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation/virtualmachinesetresourcepolicy_validator_unit_test.go @@ -0,0 +1,205 @@ +// Copyright (c) 2019 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package validation_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + "github.com/vmware-tanzu/vm-operator/test/builder" +) + +func unitTests() { + Describe("Invoking ValidateCreate", unitTestsValidateCreate) + Describe("Invoking ValidateUpdate", unitTestsValidateUpdate) + Describe("Invoking ValidateDelete", unitTestsValidateDelete) +} + +type unitValidatingWebhookContext struct { + builder.UnitTestContextForValidatingWebhook + vmRP *vmopv1.VirtualMachineSetResourcePolicy + oldVMRP *vmopv1.VirtualMachineSetResourcePolicy +} + +func newUnitTestContextForValidatingWebhook(isUpdate bool) *unitValidatingWebhookContext { + vmRP := builder.DummyVirtualMachineSetResourcePolicyA2() + obj, err := builder.ToUnstructured(vmRP) + Expect(err).ToNot(HaveOccurred()) + + var oldVMRP *vmopv1.VirtualMachineSetResourcePolicy + var oldObj *unstructured.Unstructured + + if isUpdate { + oldVMRP = vmRP.DeepCopy() + oldObj, err = builder.ToUnstructured(oldVMRP) + Expect(err).ToNot(HaveOccurred()) + } + + return &unitValidatingWebhookContext{ + UnitTestContextForValidatingWebhook: *suite.NewUnitTestContextForValidatingWebhook(obj, oldObj), + vmRP: vmRP, + oldVMRP: oldVMRP, + } +} + +func unitTestsValidateCreate() { + var ( + ctx *unitValidatingWebhookContext + ) + + type createArgs struct { + noCPULimit bool + noMemoryLimit bool + invalidCPURequest bool + invalidMemoryRequest bool + } + + validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + var err error + + if args.noCPULimit { + ctx.vmRP.Spec.ResourcePool.Limits.Cpu = resource.MustParse("0") + } + if args.noMemoryLimit { + ctx.vmRP.Spec.ResourcePool.Limits.Memory = resource.MustParse("0") + } + if args.invalidCPURequest { + ctx.vmRP.Spec.ResourcePool.Reservations.Cpu = resource.MustParse("2Gi") + ctx.vmRP.Spec.ResourcePool.Limits.Cpu = resource.MustParse("1Gi") + } + if args.invalidMemoryRequest { + ctx.vmRP.Spec.ResourcePool.Reservations.Memory = resource.MustParse("4Gi") + ctx.vmRP.Spec.ResourcePool.Limits.Memory = resource.MustParse("1Gi") + } + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmRP) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateCreate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(Equal(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + AfterEach(func() { + ctx = nil + }) + + reservationsPath := field.NewPath("spec", "resourcepool", "reservations") + detailMsg := "reservation value cannot exceed the limit value" + DescribeTable("create table", validateCreate, + Entry("should allow valid", createArgs{}, true, nil, nil), + Entry("should allow no cpu limit", createArgs{noCPULimit: true}, true, nil, nil), + Entry("should allow no memory limit", createArgs{noMemoryLimit: true}, true, nil, nil), + Entry("should deny invalid cpu reservation", createArgs{invalidCPURequest: true}, false, + field.Invalid(reservationsPath.Child("cpu"), "2Gi", detailMsg).Error(), nil), + Entry("should deny invalid memory reservation", createArgs{invalidMemoryRequest: true}, false, + field.Invalid(reservationsPath.Child("memory"), "4Gi", detailMsg).Error(), nil), + ) +} + +func unitTestsValidateUpdate() { + var ( + ctx *unitValidatingWebhookContext + ) + + type updateArgs struct { + changeCPU bool + changeMemory bool + } + + validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) { + var err error + + if args.changeCPU { + ctx.vmRP.Spec.ResourcePool.Reservations.Cpu = resource.MustParse("5Gi") + ctx.vmRP.Spec.ResourcePool.Limits.Cpu = resource.MustParse("10Gi") + } + if args.changeMemory { + ctx.vmRP.Spec.ResourcePool.Reservations.Memory = resource.MustParse("5Gi") + ctx.vmRP.Spec.ResourcePool.Limits.Memory = resource.MustParse("10Gi") + } + + ctx.WebhookRequestContext.Obj, err = builder.ToUnstructured(ctx.vmRP) + Expect(err).ToNot(HaveOccurred()) + + response := ctx.ValidateUpdate(&ctx.WebhookRequestContext) + Expect(response.Allowed).To(Equal(expectedAllowed)) + if expectedReason != "" { + Expect(string(response.Result.Reason)).To(ContainSubstring(expectedReason)) + } + if expectedErr != nil { + Expect(response.Result.Message).To(Equal(expectedErr.Error())) + } + } + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(true) + }) + AfterEach(func() { + ctx = nil + }) + + immutableFieldMsg := "field is immutable" + DescribeTable("update table", validateUpdate, + Entry("should allow", updateArgs{}, true, nil, nil), + Entry("should deny policy cpu change", updateArgs{changeCPU: true}, false, immutableFieldMsg, nil), + Entry("should deny policy memory change", updateArgs{changeMemory: true}, false, immutableFieldMsg, nil), + ) + + When("the update is performed while object deletion", func() { + var response admission.Response + + JustBeforeEach(func() { + t := metav1.Now() + ctx.WebhookRequestContext.Obj.SetDeletionTimestamp(&t) + response = ctx.ValidateUpdate(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} + +func unitTestsValidateDelete() { + var ( + ctx *unitValidatingWebhookContext + response admission.Response + ) + + BeforeEach(func() { + ctx = newUnitTestContextForValidatingWebhook(false) + }) + AfterEach(func() { + ctx = nil + }) + + When("the delete is performed", func() { + JustBeforeEach(func() { + response = ctx.ValidateDelete(&ctx.WebhookRequestContext) + }) + + It("should allow the request", func() { + Expect(response.Allowed).To(BeTrue()) + Expect(response.Result).ToNot(BeNil()) + }) + }) +} diff --git a/webhooks/virtualmachinesetresourcepolicy/v1alpha2/webhooks.go b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/webhooks.go new file mode 100644 index 000000000..1c3e76a29 --- /dev/null +++ b/webhooks/virtualmachinesetresourcepolicy/v1alpha2/webhooks.go @@ -0,0 +1,20 @@ +// Copyright (c) 2019 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha2 + +import ( + "github.com/pkg/errors" + + ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/vmware-tanzu/vm-operator/pkg/context" + "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinesetresourcepolicy/v1alpha2/validation" +) + +func AddToManager(ctx *context.ControllerManagerContext, mgr ctrlmgr.Manager) error { + if err := validation.AddToManager(ctx, mgr); err != nil { + return errors.Wrap(err, "failed to initialize validation webhook") + } + return nil +}