diff --git a/api/v1alpha1/virtualmachine_webhook.go b/api/v1alpha1/virtualmachine_webhook.go index de33a451f..dcd0c34e2 100644 --- a/api/v1alpha1/virtualmachine_webhook.go +++ b/api/v1alpha1/virtualmachine_webhook.go @@ -3,7 +3,9 @@ package v1alpha1 -import ctrl "sigs.k8s.io/controller-runtime" +import ( + ctrl "sigs.k8s.io/controller-runtime" +) func (r *VirtualMachine) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). diff --git a/api/v1alpha2/virtualmachine_webhook.go b/api/v1alpha2/virtualmachine_webhook.go index f4f73e0a7..20e7b0cb3 100644 --- a/api/v1alpha2/virtualmachine_webhook.go +++ b/api/v1alpha2/virtualmachine_webhook.go @@ -3,7 +3,9 @@ package v1alpha2 -import ctrl "sigs.k8s.io/controller-runtime" +import ( + ctrl "sigs.k8s.io/controller-runtime" +) func (r *VirtualMachine) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). diff --git a/controllers/virtualmachine/v1alpha1/virtualmachine_controller_intg_test.go b/controllers/virtualmachine/v1alpha1/virtualmachine_controller_intg_test.go index d4b9826be..0ddb61cab 100644 --- a/controllers/virtualmachine/v1alpha1/virtualmachine_controller_intg_test.go +++ b/controllers/virtualmachine/v1alpha1/virtualmachine_controller_intg_test.go @@ -12,7 +12,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + resourcev1 "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -28,11 +31,36 @@ func intgTests() { vm *vmopv1.VirtualMachine vmKey types.NamespacedName + + storageClass *storagev1.StorageClass + resourceQuota *corev1.ResourceQuota ) BeforeEach(func() { ctx = suite.NewIntegrationTestContext() + // The validation webhook expects there to be a storage class associated + // with the namespace where the VM is located. + storageClass = &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "dummy-storage-class-", + }, + Provisioner: "dummy-provisioner", + } + Expect(ctx.Client.Create(ctx, storageClass)).To(Succeed()) + resourceQuota = &corev1.ResourceQuota{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "dummy-resource-quota-", + Namespace: ctx.Namespace, + }, + Spec: corev1.ResourceQuotaSpec{ + Hard: corev1.ResourceList{ + corev1.ResourceName(storageClass.Name + ".storageclass.storage.k8s.io/dummy"): resourcev1.MustParse("0"), + }, + }, + } + Expect(ctx.Client.Create(ctx, resourceQuota)).To(Succeed()) + vm = &vmopv1.VirtualMachine{ ObjectMeta: metav1.ObjectMeta{ Namespace: ctx.Namespace, @@ -42,7 +70,7 @@ func intgTests() { ImageName: "dummy-image", ClassName: "dummy-class", PowerState: vmopv1.VirtualMachinePoweredOn, - StorageClass: "dummy-storageclass", + StorageClass: storageClass.Name, VmMetadata: &vmopv1.VirtualMachineMetadata{ Transport: vmopv1.VirtualMachineMetadataOvfEnvTransport, ConfigMapName: "dummy-configmap", @@ -53,6 +81,11 @@ func intgTests() { }) AfterEach(func() { + Expect(ctx.Client.Delete(ctx, resourceQuota)).To(Succeed()) + resourceQuota = nil + Expect(ctx.Client.Delete(ctx, storageClass)).To(Succeed()) + storageClass = nil + ctx.AfterEach() ctx = nil intgFakeVMProvider.Reset() diff --git a/controllers/virtualmachine/v1alpha1/virtualmachine_controller_suite_test.go b/controllers/virtualmachine/v1alpha1/virtualmachine_controller_suite_test.go index 9a5cea317..30ceaf636 100644 --- a/controllers/virtualmachine/v1alpha1/virtualmachine_controller_suite_test.go +++ b/controllers/virtualmachine/v1alpha1/virtualmachine_controller_suite_test.go @@ -12,19 +12,35 @@ import ( virtualmachine "github.com/vmware-tanzu/vm-operator/controllers/virtualmachine/v1alpha1" ctrlContext "github.com/vmware-tanzu/vm-operator/pkg/context" + pkgmgr "github.com/vmware-tanzu/vm-operator/pkg/manager" providerfake "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/fake" "github.com/vmware-tanzu/vm-operator/test/builder" + mutationv1a1 "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha1/mutation" + validationv1a1 "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha1/validation" ) var intgFakeVMProvider = providerfake.NewVMProvider() -var suite = builder.NewTestSuiteForController( - virtualmachine.AddToManager, - func(ctx *ctrlContext.ControllerManagerContext, _ ctrlmgr.Manager) error { - ctx.VMProvider = intgFakeVMProvider - return nil - }, -) +var suite = builder.NewTestSuiteWithOptions( + builder.TestSuiteOptions{ + InitProviderFn: func(ctx *ctrlContext.ControllerManagerContext, _ ctrlmgr.Manager) error { + ctx.VMProvider = intgFakeVMProvider + return nil + }, + Controllers: []pkgmgr.AddToManagerFunc{virtualmachine.AddToManager}, + MutationWebhooks: []builder.TestSuiteMutationWebhookOptions{ + { + Name: "default.mutating.virtualmachine.v1alpha1.vmoperator.vmware.com", + AddToManagerFn: mutationv1a1.AddToManager, + }, + }, + ValidationWebhooks: []builder.TestSuiteValidationWebhookOptions{ + { + Name: "default.validating.virtualmachine.v1alpha1.vmoperator.vmware.com", + AddToManagerFn: validationv1a1.AddToManager, + }, + }, + }) func TestVirtualMachine(t *testing.T) { suite.Register(t, "VirtualMachine controller suite", intgTests, unitTests) diff --git a/controllers/virtualmachine/v1alpha2/virtualmachine_controller_intg_test.go b/controllers/virtualmachine/v1alpha2/virtualmachine_controller_intg_test.go index 13be6367e..07d744bc9 100644 --- a/controllers/virtualmachine/v1alpha2/virtualmachine_controller_intg_test.go +++ b/controllers/virtualmachine/v1alpha2/virtualmachine_controller_intg_test.go @@ -11,105 +11,135 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/client" + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + resourcev1 "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + vmopv1a2 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" "github.com/vmware-tanzu/vm-operator/test/builder" ) func intgTests() { - var ( - ctx *builder.IntegrationTestContext + const dummyInstanceUUID = "instanceUUID1234" - vm *vmopv1.VirtualMachine - vmKey types.NamespacedName + var ( + ctx *builder.IntegrationTestContext + storageClass *storagev1.StorageClass + resourceQuota *corev1.ResourceQuota + obj client.Object + objKey types.NamespacedName + newObjFn func() client.Object + getInstanceUUIDFn func(client.Object) string + pauseAnnotationLabel string ) BeforeEach(func() { ctx = suite.NewIntegrationTestContext() - vm = &vmopv1.VirtualMachine{ + objKey = types.NamespacedName{Name: "dummy-vm", Namespace: ctx.Namespace} + + // The validation webhook expects there to be a storage class associated + // with the namespace where the VM is located. + storageClass = &storagev1.StorageClass{ ObjectMeta: metav1.ObjectMeta{ - Namespace: ctx.Namespace, - Name: "dummy-vm", + GenerateName: "dummy-storage-class-", }, - Spec: vmopv1.VirtualMachineSpec{ - ImageName: "dummy-image", - ClassName: "dummy-class", - PowerState: vmopv1.VirtualMachinePowerStateOn, + Provisioner: "dummy-provisioner", + } + Expect(ctx.Client.Create(ctx, storageClass)).To(Succeed()) + resourceQuota = &corev1.ResourceQuota{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "dummy-resource-quota-", + Namespace: ctx.Namespace, + }, + Spec: corev1.ResourceQuotaSpec{ + Hard: corev1.ResourceList{ + corev1.ResourceName(storageClass.Name + ".storageclass.storage.k8s.io/dummy"): resourcev1.MustParse("0"), + }, }, } - vmKey = types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace} + Expect(ctx.Client.Create(ctx, resourceQuota)).To(Succeed()) + + intgFakeVMProvider.Lock() + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1a2.VirtualMachine) error { + // Used below just to check for something in the Status is updated. + vm.Status.InstanceUUID = dummyInstanceUUID + return nil + } + intgFakeVMProvider.Unlock() }) AfterEach(func() { + By("Delete VirtualMachine", func() { + if err := ctx.Client.Delete(ctx, obj); err == nil { + obj := newObjFn() + // If VM is still around because of finalizer, try to cleanup for next test. + if err := ctx.Client.Get(ctx, objKey, obj); err == nil && len(obj.GetFinalizers()) > 0 { + obj.SetFinalizers(nil) + _ = ctx.Client.Update(ctx, obj) + } + } else { + Expect(k8serrors.IsNotFound(err)).To(BeTrue()) + } + }) + + Expect(ctx.Client.Delete(ctx, resourceQuota)).To(Succeed()) + resourceQuota = nil + Expect(ctx.Client.Delete(ctx, storageClass)).To(Succeed()) + storageClass = nil + + obj = nil + newObjFn = nil + getInstanceUUIDFn = nil + pauseAnnotationLabel = "" + ctx.AfterEach() ctx = nil intgFakeVMProvider.Reset() }) - getVirtualMachine := func(ctx *builder.IntegrationTestContext, objKey types.NamespacedName) *vmopv1.VirtualMachine { - vm := &vmopv1.VirtualMachine{} - if err := ctx.Client.Get(ctx, objKey, vm); err != nil { + getObject := func( + ctx *builder.IntegrationTestContext, + objKey types.NamespacedName, + obj client.Object) client.Object { + + if err := ctx.Client.Get(ctx, objKey, obj); err != nil { return nil } - return vm + return obj } - waitForVirtualMachineFinalizer := func(ctx *builder.IntegrationTestContext, objKey types.NamespacedName) { + waitForVirtualMachineFinalizer := func( + ctx *builder.IntegrationTestContext, + objKey types.NamespacedName, + obj client.Object) { + Eventually(func() []string { - if vm := getVirtualMachine(ctx, objKey); vm != nil { - return vm.GetFinalizers() + if obj := getObject(ctx, objKey, obj); obj != nil { + return obj.GetFinalizers() } return nil }).Should(ContainElement(finalizer), "waiting for VirtualMachine finalizer") } - Context("Reconcile", func() { - dummyInstanceUUID := "instanceUUID1234" - - BeforeEach(func() { - intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { - // Used below just to check for something in the Status is updated. - vm.Status.InstanceUUID = dummyInstanceUUID - return nil - } - intgFakeVMProvider.Unlock() - }) - - AfterEach(func() { - By("Delete VirtualMachine", func() { - if err := ctx.Client.Delete(ctx, vm); err == nil { - vm := &vmopv1.VirtualMachine{} - // If VM is still around because of finalizer, try to cleanup for next test. - if err := ctx.Client.Get(ctx, vmKey, vm); err == nil && len(vm.Finalizers) > 0 { - vm.Finalizers = nil - _ = ctx.Client.Update(ctx, vm) - } - } else { - Expect(k8serrors.IsNotFound(err)).To(BeTrue()) - } - }) - }) - + reconcile := func() { When("the pause annotation is set", func() { It("Reconcile returns early and the finalizer never gets added", func() { - // Set the Pause annotation on the VM - vm.Annotations = map[string]string{ - vmopv1.PauseAnnotation: "", - } - - Expect(ctx.Client.Create(ctx, vm)).To(Succeed()) - + obj.SetAnnotations(map[string]string{ + pauseAnnotationLabel: "", + }) + Expect(ctx.Client.Create(ctx, obj)).To(Succeed()) Consistently(func() []string { - if vm := getVirtualMachine(ctx, vmKey); vm != nil { - return vm.GetFinalizers() + if obj := getObject(ctx, objKey, newObjFn()); obj != nil { + return obj.GetFinalizers() } return nil }).ShouldNot(ContainElement(finalizer), "waiting for VirtualMachine finalizer") @@ -117,34 +147,34 @@ func intgTests() { }) It("Reconciles after VirtualMachine creation", func() { - Expect(ctx.Client.Create(ctx, vm)).To(Succeed()) + Expect(ctx.Client.Create(ctx, obj)).To(Succeed()) By("VirtualMachine should have finalizer added", func() { - waitForVirtualMachineFinalizer(ctx, vmKey) + waitForVirtualMachineFinalizer(ctx, objKey, newObjFn()) }) By("VirtualMachine should reflect VMProvider updates", func() { Eventually(func() string { - if vm := getVirtualMachine(ctx, vmKey); vm != nil { - return vm.Status.InstanceUUID + if obj := getObject(ctx, objKey, newObjFn()); obj != nil { + return getInstanceUUIDFn(obj) } return "" }).Should(Equal(dummyInstanceUUID), "waiting for expected InstanceUUID") }) By("VirtualMachine should not be updated in steady-state", func() { - vm := getVirtualMachine(ctx, vmKey) - Expect(vm).ToNot(BeNil()) - rv := vm.GetResourceVersion() + obj := getObject(ctx, objKey, newObjFn()) + Expect(obj).ToNot(BeNil()) + rv := obj.GetResourceVersion() Expect(rv).ToNot(BeEmpty()) - expected := fmt.Sprintf("%s :: %d", rv, vm.GetGeneration()) + expected := fmt.Sprintf("%s :: %d", rv, obj.GetGeneration()) // The resync period is 1 second, so balance between giving enough time vs a slow test. // Note: the kube-apiserver we test against (obtained from kubebuilder) is old and // appears to behavior differently than newer versions (like used in the SV) in that noop // Status subresource updates don't increment the ResourceVersion. Consistently(func() string { - if vm := getVirtualMachine(ctx, vmKey); vm != nil { - return fmt.Sprintf("%s :: %d", vm.GetResourceVersion(), vm.GetGeneration()) + if obj := getObject(ctx, objKey, newObjFn()); obj != nil { + return fmt.Sprintf("%s :: %d", obj.GetResourceVersion(), obj.GetGeneration()) } return "" }, 4*time.Second).Should(Equal(expected)) @@ -156,7 +186,7 @@ func intgTests() { BeforeEach(func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1a2.VirtualMachine) error { vm.Status.BiosUUID = "dummy-bios-uuid" return errors.New(errMsg) } @@ -164,22 +194,21 @@ func intgTests() { }) It("VirtualMachine is in Creating Phase", func() { - Expect(ctx.Client.Create(ctx, vm)).To(Succeed()) + Expect(ctx.Client.Create(ctx, obj)).To(Succeed()) // Wait for initial reconcile. - waitForVirtualMachineFinalizer(ctx, vmKey) + waitForVirtualMachineFinalizer(ctx, objKey, newObjFn()) }) }) It("Reconciles after VirtualMachine deletion", func() { - Expect(ctx.Client.Create(ctx, vm)).To(Succeed()) + Expect(ctx.Client.Create(ctx, obj)).To(Succeed()) // Wait for initial reconcile. - waitForVirtualMachineFinalizer(ctx, vmKey) - - Expect(ctx.Client.Delete(ctx, vm)).To(Succeed()) + waitForVirtualMachineFinalizer(ctx, objKey, newObjFn()) + Expect(ctx.Client.Delete(ctx, obj)).To(Succeed()) By("Finalizer should be removed after deletion", func() { Eventually(func() []string { - if vm := getVirtualMachine(ctx, vmKey); vm != nil { - return vm.GetFinalizers() + if obj := getObject(ctx, objKey, newObjFn()); obj != nil { + return obj.GetFinalizers() } return nil }).ShouldNot(ContainElement(finalizer)) @@ -191,25 +220,81 @@ func intgTests() { BeforeEach(func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.DeleteVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.DeleteVirtualMachineFn = func(ctx context.Context, vm *vmopv1a2.VirtualMachine) error { return errors.New(errMsg) } intgFakeVMProvider.Unlock() }) It("VirtualMachine is in Deleting Phase", func() { - Expect(ctx.Client.Create(ctx, vm)).To(Succeed()) + Expect(ctx.Client.Create(ctx, obj)).To(Succeed()) // Wait for initial reconcile. - waitForVirtualMachineFinalizer(ctx, vmKey) - - Expect(ctx.Client.Delete(ctx, vm)).To(Succeed()) - + waitForVirtualMachineFinalizer(ctx, objKey, newObjFn()) + Expect(ctx.Client.Delete(ctx, obj)).To(Succeed()) By("Finalizer should still be present", func() { - vm := getVirtualMachine(ctx, vmKey) - Expect(vm).ToNot(BeNil()) - Expect(vm.GetFinalizers()).To(ContainElement(finalizer)) + obj := getObject(ctx, objKey, newObjFn()) + Expect(obj).ToNot(BeNil()) + Expect(obj.GetFinalizers()).To(ContainElement(finalizer)) }) }) }) + } + + Context("v1alpha2", func() { + BeforeEach(func() { + pauseAnnotationLabel = vmopv1a2.PauseAnnotation + + newObjFn = func() client.Object { + return &vmopv1a2.VirtualMachine{} + } + + getInstanceUUIDFn = func(obj client.Object) string { + return obj.(*vmopv1a2.VirtualMachine).Status.InstanceUUID + } + + obj = &vmopv1a2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: objKey.Namespace, + Name: objKey.Name, + }, + Spec: vmopv1a2.VirtualMachineSpec{ + ImageName: "dummy-image", + ClassName: "dummy-class", + StorageClass: storageClass.Name, + PowerState: vmopv1a2.VirtualMachinePowerStateOn, + }, + } + }) + + Context("Reconcile", reconcile) + }) + + Context("v1alpha1", func() { + BeforeEach(func() { + pauseAnnotationLabel = vmopv1.PauseAnnotation + + newObjFn = func() client.Object { + return &vmopv1.VirtualMachine{} + } + + getInstanceUUIDFn = func(obj client.Object) string { + return obj.(*vmopv1.VirtualMachine).Status.InstanceUUID + } + + obj = &vmopv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: objKey.Namespace, + Name: objKey.Name, + }, + Spec: vmopv1.VirtualMachineSpec{ + ImageName: "dummy-image", + ClassName: "dummy-class", + StorageClass: storageClass.Name, + PowerState: vmopv1.VirtualMachinePoweredOn, + }, + } + }) + + Context("Reconcile", reconcile) }) } diff --git a/controllers/virtualmachine/v1alpha2/virtualmachine_controller_suite_test.go b/controllers/virtualmachine/v1alpha2/virtualmachine_controller_suite_test.go index b22524ee6..5282949c1 100644 --- a/controllers/virtualmachine/v1alpha2/virtualmachine_controller_suite_test.go +++ b/controllers/virtualmachine/v1alpha2/virtualmachine_controller_suite_test.go @@ -10,22 +10,60 @@ import ( ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + vmopv1a2 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" virtualmachine "github.com/vmware-tanzu/vm-operator/controllers/virtualmachine/v1alpha2" ctrlContext "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/lib" + pkgmgr "github.com/vmware-tanzu/vm-operator/pkg/manager" providerfake "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/fake" "github.com/vmware-tanzu/vm-operator/test/builder" + mutationv1a1 "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha1/mutation" + validationv1a1 "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha1/validation" + mutationv1a2 "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha2/mutation" + validationv1a2 "github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/v1alpha2/validation" ) var intgFakeVMProvider = providerfake.NewVMProviderA2() -var suite = builder.NewTestSuiteForControllerWithFSS( - virtualmachine.AddToManager, - func(ctx *ctrlContext.ControllerManagerContext, _ ctrlmgr.Manager) error { - ctx.VMProviderA2 = intgFakeVMProvider - return nil - }, - map[string]bool{lib.VMServiceV1Alpha2FSS: true}) +var suite = builder.NewTestSuiteWithOptions( + builder.TestSuiteOptions{ + InitProviderFn: func(ctx *ctrlContext.ControllerManagerContext, _ ctrlmgr.Manager) error { + ctx.VMProviderA2 = intgFakeVMProvider + return nil + }, + FeatureStates: map[string]bool{lib.VMServiceV1Alpha2FSS: true}, + Controllers: []pkgmgr.AddToManagerFunc{virtualmachine.AddToManager}, + ConversionWebhooks: []builder.TestSuiteConversionWebhookOptions{ + { + Name: "virtualmachines.vmoperator.vmware.com", + AddToManagerFn: []func(ctrlmgr.Manager) error{ + (&vmopv1.VirtualMachine{}).SetupWebhookWithManager, + (&vmopv1a2.VirtualMachine{}).SetupWebhookWithManager, + }, + }, + }, + MutationWebhooks: []builder.TestSuiteMutationWebhookOptions{ + { + Name: "default.mutating.virtualmachine.v1alpha1.vmoperator.vmware.com", + AddToManagerFn: mutationv1a1.AddToManager, + }, + { + Name: "default.mutating.virtualmachine.v1alpha2.vmoperator.vmware.com", + AddToManagerFn: mutationv1a2.AddToManager, + }, + }, + ValidationWebhooks: []builder.TestSuiteValidationWebhookOptions{ + { + Name: "default.validating.virtualmachine.v1alpha1.vmoperator.vmware.com", + AddToManagerFn: validationv1a1.AddToManager, + }, + { + Name: "default.validating.virtualmachine.v1alpha2.vmoperator.vmware.com", + AddToManagerFn: validationv1a2.AddToManager, + }, + }, + }) func TestVirtualMachine(t *testing.T) { suite.Register(t, "VirtualMachine controller suite", intgTests, unitTests) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 374763b4a..f7135a6cd 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" @@ -46,6 +47,7 @@ func New(opts Options) (Manager, error) { opts.defaults() _ = clientgoscheme.AddToScheme(opts.Scheme) + _ = apiextensionsv1.AddToScheme(opts.Scheme) _ = vmopv1.AddToScheme(opts.Scheme) _ = ncpv1alpha1.AddToScheme(opts.Scheme) _ = cnsv1alpha1.AddToScheme(opts.Scheme) diff --git a/pkg/manager/options.go b/pkg/manager/options.go index 7f02fca82..e9496e605 100644 --- a/pkg/manager/options.go +++ b/pkg/manager/options.go @@ -37,7 +37,7 @@ var InitializeProvidersNoopFn InitializeProvidersFunc = func(_ *context.Controll return nil } -// Options describes the options used to create a new GCM manager. +// Options describes the options used to create a new manager. type Options struct { // LeaderElectionEnabled is a flag that enables leader election. LeaderElectionEnabled bool @@ -46,8 +46,8 @@ type Options struct { // locking resource when configuring leader election. LeaderElectionID string - // HealthProbeBindAddress is the TCP address that the controller should bind to - // for serving health probes + // HealthProbeBindAddress is the TCP address to which the controller should + // bind for serving health probes. HealthProbeBindAddress string // SyncPeriod is the amount of time to wait between syncing the local diff --git a/test/builder/test_suite.go b/test/builder/test_suite.go index 8480f8f1a..2f14f5e9e 100644 --- a/test/builder/test_suite.go +++ b/test/builder/test_suite.go @@ -33,7 +33,8 @@ import ( "k8s.io/client-go/rest" "k8s.io/klog/v2" "k8s.io/klog/v2/klogr" - "sigs.k8s.io/controller-runtime/pkg/client" + ctrl "sigs.k8s.io/controller-runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -73,34 +74,63 @@ type TestSuite struct { flags testFlags envTest envtest.Environment config *rest.Config - integrationTestClient client.Client + integrationTestClient ctrlclient.Client + integrationTest bool + initProvidersFn pkgmgr.InitializeProvidersFunc + + // Controller manager specific fields. + manager pkgmgr.Manager + managerRunning bool + managerRunningMutex sync.Mutex // Cancel function that will be called to close the Done channel of the // Context, which will then stop the manager. cancelFuncMutex sync.Mutex cancelFunc context.CancelFunc - // Controller specific fields - addToManagerFn pkgmgr.AddToManagerFunc - initProvidersFn pkgmgr.InitializeProvidersFunc - integrationTest bool - manager pkgmgr.Manager - managerRunning bool - managerRunningMutex sync.Mutex + // Controller specific fields. + controllers []pkgmgr.AddToManagerFunc - // Webhook specific fields - webhookName string - certDir string - validatorFn builder.ValidatorFunc - mutatorFn builder.MutatorFunc - pki pkiToolchain - webhookYaml []byte + // Webhook specific fields. + webhook testSuiteWebhookConfig + // Feature state switches. fssMap map[string]bool } +type testSuiteWebhookConfig struct { + certDir string + pki pkiToolchain + conversionOpts []TestSuiteConversionWebhookOptions + conversionName []string + mutationOpts []TestSuiteMutationWebhookOptions + mutationYAML []byte + validationOpts []TestSuiteValidationWebhookOptions + validationYAML []byte +} + +func (s *TestSuite) isControllerTest() bool { + return len(s.controllers) > 0 +} + func (s *TestSuite) isWebhookTest() bool { - return s.webhookName != "" + return s.isAdmissionWebhookTest() || s.isConversionWebhookTest() +} + +func (s *TestSuite) isAdmissionWebhookTest() bool { + return s.isMutationWebhookTest() || s.isValidationWebhookTest() +} + +func (s *TestSuite) isConversionWebhookTest() bool { + return len(s.webhook.conversionOpts) > 0 +} + +func (s *TestSuite) isMutationWebhookTest() bool { + return len(s.webhook.mutationOpts) > 0 +} + +func (s *TestSuite) isValidationWebhookTest() bool { + return len(s.webhook.validationOpts) > 0 } func (s *TestSuite) GetEnvTestConfig() *rest.Config { @@ -113,46 +143,137 @@ func (s *TestSuite) GetLogger() logr.Logger { // NewTestSuite returns a new test suite used for unit and/or integration test. func NewTestSuite() *TestSuite { - return NewTestSuiteForController( - pkgmgr.AddToManagerNoopFn, - pkgmgr.InitializeProvidersNoopFn, - ) + return NewTestSuiteWithOptions( + TestSuiteOptions{ + InitProviderFn: pkgmgr.InitializeProvidersNoopFn, + Controllers: []pkgmgr.AddToManagerFunc{pkgmgr.AddToManagerNoopFn}, + }) } // NewFunctionalTestSuite returns a new test suite used for functional tests. // The functional test starts all the controllers, and creates all the providers // so it is a more fully functioning env than an integration test with a single // controller running. -func NewFunctionalTestSuite(addToManagerFunc func(ctx *ctrlCtx.ControllerManagerContext, mgr manager.Manager) error) *TestSuite { - return NewTestSuiteForController( - addToManagerFunc, - pkgmgr.InitializeProviders, - ) +func NewFunctionalTestSuite(addToManagerFn pkgmgr.AddToManagerFunc) *TestSuite { + return NewTestSuiteWithOptions( + TestSuiteOptions{ + InitProviderFn: pkgmgr.InitializeProviders, + Controllers: []pkgmgr.AddToManagerFunc{addToManagerFn}, + }) } -// NewTestSuiteForController returns a new test suite used for controller integration test. -func NewTestSuiteForController(addToManagerFn pkgmgr.AddToManagerFunc, initProvidersFn pkgmgr.InitializeProvidersFunc) *TestSuite { - return NewTestSuiteForControllerWithFSS(addToManagerFn, initProvidersFn, map[string]bool{}) +// NewTestSuiteForController returns a new test suite used for controller +// integration test. +func NewTestSuiteForController( + addToManagerFn pkgmgr.AddToManagerFunc, + initProvidersFn pkgmgr.InitializeProvidersFunc) *TestSuite { + + return NewTestSuiteWithOptions( + TestSuiteOptions{ + InitProviderFn: initProvidersFn, + Controllers: []pkgmgr.AddToManagerFunc{addToManagerFn}, + FeatureStates: map[string]bool{}, + }) } -// NewTestSuiteForControllerWithFSS returns a new test suite used for controller integration test with FSS set. -func NewTestSuiteForControllerWithFSS(addToManagerFn pkgmgr.AddToManagerFunc, - initProvidersFn pkgmgr.InitializeProvidersFunc, fssMap map[string]bool) *TestSuite { +// NewTestSuiteForControllerWithFSS returns a new test suite used for controller +// integration test with FSS set. +func NewTestSuiteForControllerWithFSS( + addToManagerFn pkgmgr.AddToManagerFunc, + initProvidersFn pkgmgr.InitializeProvidersFunc, + fssMap map[string]bool) *TestSuite { - if addToManagerFn == nil { - panic("addToManagerFn is nil") + return NewTestSuiteWithOptions( + TestSuiteOptions{ + InitProviderFn: initProvidersFn, + Controllers: []pkgmgr.AddToManagerFunc{addToManagerFn}, + FeatureStates: fssMap, + }) +} + +type TestSuiteOptions struct { + InitProviderFn pkgmgr.InitializeProvidersFunc + FeatureStates map[string]bool + Controllers []pkgmgr.AddToManagerFunc + ValidationWebhooks []TestSuiteValidationWebhookOptions + MutationWebhooks []TestSuiteMutationWebhookOptions + ConversionWebhooks []TestSuiteConversionWebhookOptions +} + +type TestSuiteValidationWebhookOptions struct { + // Name is the unique ID of the validation webhook, ex. + // default.validating.virtualmachine.v1alpha1.vmoperator.vmware.com. + Name string + + // AddToManagerFn is the function that adds the webhook to the controller + // manager. + AddToManagerFn pkgmgr.AddToManagerFunc + + // ValidatorFn is used to unit testing. + ValidatorFn builder.ValidatorFunc +} + +type TestSuiteMutationWebhookOptions struct { + // Name is the unique ID of the mutation webhook, ex. + // default.mutating.virtualmachine.v1alpha1.vmoperator.vmware.com. + Name string + + // AddToManagerFn is the function that adds the webhook to the controller + // manager. + AddToManagerFn pkgmgr.AddToManagerFunc + + // MutatorFn is used to unit testing. + MutatorFn builder.MutatorFunc +} + +type TestSuiteConversionWebhookOptions struct { + // Name is the resource to which the conversion webhook applies. For + // example, the name of the VirtualMachine resource is + // virtualmachines.vmoperator.vmware.com. + Name string + + // AddToManagerFn is a list of the functions that add the webhook(s) to the + // controller manager. There is a distinct function for each version of the + // resource specified by Name. + AddToManagerFn []func(ctrl.Manager) error +} + +// NewTestSuiteWithOptions returns a new test suite used for controller integration test with FSS set. +func NewTestSuiteWithOptions(opts TestSuiteOptions) *TestSuite { + + if len(opts.Controllers) == 0 && + len(opts.ValidationWebhooks) == 0 && + len(opts.MutationWebhooks) == 0 && + len(opts.ConversionWebhooks) == 0 { + + panic("there are no addToManager functions") } - if initProvidersFn == nil { + if opts.InitProviderFn == nil { panic("initProvidersFn is nil") } testSuite := &TestSuite{ Context: context.Background(), integrationTest: true, - addToManagerFn: addToManagerFn, - initProvidersFn: initProvidersFn, - fssMap: fssMap, + controllers: opts.Controllers, + initProvidersFn: opts.InitProviderFn, + fssMap: opts.FeatureStates, + webhook: testSuiteWebhookConfig{ + conversionOpts: opts.ConversionWebhooks, + mutationOpts: opts.MutationWebhooks, + validationOpts: opts.ValidationWebhooks, + }, + } + + if testSuite.isWebhookTest() { + // Create a temp directory for the certs needed for testing webhooks. + certDir, err := os.MkdirTemp(os.TempDir(), "") + if err != nil { + panic(errors.Wrap(err, "failed to create temp dir for certs")) + } + testSuite.webhook.certDir = certDir } + testSuite.init() return testSuite @@ -211,32 +332,31 @@ func newTestSuiteForWebhook( webhookName string, fssMap map[string]bool) *TestSuite { - testSuite := &TestSuite{ - Context: context.Background(), - integrationTest: true, - addToManagerFn: addToManagerFn, - initProvidersFn: pkgmgr.InitializeProvidersNoopFn, - webhookName: webhookName, - fssMap: fssMap, + opts := TestSuiteOptions{ + InitProviderFn: pkgmgr.InitializeProvidersNoopFn, + FeatureStates: fssMap, } - if newValidatorFn != nil { - testSuite.validatorFn = newValidatorFn - } if newMutatorFn != nil { - testSuite.mutatorFn = newMutatorFn + opts.MutationWebhooks = []TestSuiteMutationWebhookOptions{ + { + Name: webhookName, + MutatorFn: newMutatorFn, + AddToManagerFn: addToManagerFn, + }, + } } - - // Create a temp directory for the certs needed for testing webhooks. - certDir, err := os.MkdirTemp(os.TempDir(), "") - if err != nil { - panic(errors.Wrap(err, "failed to create temp dir for certs")) + if newValidatorFn != nil { + opts.ValidationWebhooks = []TestSuiteValidationWebhookOptions{ + { + Name: webhookName, + ValidatorFn: newValidatorFn, + AddToManagerFn: addToManagerFn, + }, + } } - testSuite.certDir = certDir - - testSuite.init() - return testSuite + return NewTestSuiteWithOptions(opts) } func (s *TestSuite) init() { @@ -298,7 +418,7 @@ func (s *TestSuite) Register(t *testing.T, name string, runIntegrationTestsFn, r // suite's reconciler. // // Returns nil if unit testing is disabled. -func (s *TestSuite) NewUnitTestContextForController(initObjects ...client.Object) *UnitTestContextForController { +func (s *TestSuite) NewUnitTestContextForController(initObjects ...ctrlclient.Object) *UnitTestContextForController { if s.flags.UnitTestsEnabled { ctx := NewUnitTestContextForController(initObjects) return ctx @@ -312,10 +432,10 @@ func (s *TestSuite) NewUnitTestContextForController(initObjects ...client.Object // Returns nil if unit testing is disabled. func (s *TestSuite) NewUnitTestContextForValidatingWebhook( obj, oldObj *unstructured.Unstructured, - initObjects ...client.Object) *UnitTestContextForValidatingWebhook { + initObjects ...ctrlclient.Object) *UnitTestContextForValidatingWebhook { if s.flags.UnitTestsEnabled { - ctx := NewUnitTestContextForValidatingWebhook(s.validatorFn, obj, oldObj, initObjects...) + ctx := NewUnitTestContextForValidatingWebhook(s.webhook.validationOpts[0].ValidatorFn, obj, oldObj, initObjects...) return ctx } return nil @@ -327,7 +447,7 @@ func (s *TestSuite) NewUnitTestContextForValidatingWebhook( // Returns nil if unit testing is disabled. func (s *TestSuite) NewUnitTestContextForMutatingWebhook(obj *unstructured.Unstructured) *UnitTestContextForMutatingWebhook { if s.flags.UnitTestsEnabled { - ctx := NewUnitTestContextForMutatingWebhook(s.mutatorFn, obj) + ctx := NewUnitTestContextForMutatingWebhook(s.webhook.mutationOpts[0].MutatorFn, obj) return ctx } return nil @@ -354,8 +474,51 @@ func (s *TestSuite) createManager() { opts := pkgmgr.Options{ KubeConfig: s.config, MetricsAddr: "0", - AddToManager: s.addToManagerFn, InitializeProviders: s.initProvidersFn, + AddToManager: func( + ctx *ctrlCtx.ControllerManagerContext, + m manager.Manager) error { + + if s.isControllerTest() { + By("registering controllers") + for i := range s.controllers { + if err := s.controllers[i](ctx, m); err != nil { + return err + } + } + } + + if s.isConversionWebhookTest() { + By("registering conversion webhooks") + for i := range s.webhook.conversionOpts { + for j := range s.webhook.conversionOpts[i].AddToManagerFn { + if err := s.webhook.conversionOpts[i].AddToManagerFn[j](m); err != nil { + return err + } + } + } + } + + if s.isMutationWebhookTest() { + By("registering mutation webhooks") + for i := range s.webhook.mutationOpts { + if err := s.webhook.mutationOpts[i].AddToManagerFn(ctx, m); err != nil { + return err + } + } + } + + if s.isValidationWebhookTest() { + By("registering validation webhooks") + for i := range s.webhook.validationOpts { + if err := s.webhook.validationOpts[i].AddToManagerFn(ctx, m); err != nil { + return err + } + } + } + + return nil + }, } if enabled, ok := s.fssMap[lib.VMServiceV1Alpha2FSS]; ok && enabled { @@ -376,7 +539,7 @@ func (s *TestSuite) initializeManager() { svr := s.manager.GetWebhookServer().(*webhook.DefaultServer) svr.Options.Host = "127.0.0.1" svr.Options.Port = randomTCPPort() - svr.Options.CertDir = s.certDir + svr.Options.CertDir = s.webhook.certDir }) } } @@ -418,32 +581,42 @@ func (s *TestSuite) postConfigureManager() { // webhooks are being tested. Go ahead and install the webhooks and wait // for the webhook server to come online. if s.isWebhookTest() { - By("installing the webhook(s)", func() { - // ASSERT that the file for validating webhook file exists. - validatingWebhookFile := path.Join(testutil.GetRootDirOrDie(), "config", "webhook", "manifests.yaml") - Expect(validatingWebhookFile).Should(BeAnExistingFile()) - - // UNMARSHAL the contents of the validating webhook file into MutatingWebhookConfiguration and - // ValidatingWebhookConfiguration. - mutatingWebhookConfig, validatingWebhookConfig := parseWebhookConfig(validatingWebhookFile) - - // MARSHAL the webhook config back to YAML. - if s.mutatorFn != nil { - By("installing the mutating webhook(s)") - svr := s.manager.GetWebhookServer().(*webhook.DefaultServer) - s.webhookYaml = updateMutatingWebhookConfig(mutatingWebhookConfig, s.webhookName, svr.Options.Host, svr.Options.Port, s.pki.publicKeyPEM) - } else { - By("installing the validating webhook(s)") - svr := s.manager.GetWebhookServer().(*webhook.DefaultServer) - s.webhookYaml = updateValidatingWebhookConfig(validatingWebhookConfig, s.webhookName, svr.Options.Host, svr.Options.Port, s.pki.publicKeyPEM) - } + svr := s.manager.GetWebhookServer().(*webhook.DefaultServer) + + if s.isConversionWebhookTest() { + updateConversionWebhookConfig( + s, s.integrationTestClient, + s.webhook.conversionOpts, + svr.Options.Host, svr.Options.Port, + s.webhook.pki.publicKeyPEM, + &s.webhook.conversionName) + } - // ASSERT that eventually the webhook config gets successfully - // applied to the API server. - Eventually(func() error { - return remote.ApplyYAML(s, s.integrationTestClient, s.webhookYaml) - }).Should(Succeed()) - }) + if s.isAdmissionWebhookTest() { + // ASSERT the admission webhook manifest file exists. + admissionWebhookManifestFilePath := path.Join( + testutil.GetRootDirOrDie(), + "config", "webhook", "manifests.yaml") + Expect(admissionWebhookManifestFilePath).Should(BeAnExistingFile()) + + // UNMARSHAL the contents of the admission webhook manifest file. + mutationWebhookConfig, validationWebhookConfig := parseAdmissionWebhookManifestFile( + admissionWebhookManifestFilePath) + + updateMutationWebhookConfig( + s, s.integrationTestClient, + s.webhook.mutationOpts, mutationWebhookConfig, + svr.Options.Host, svr.Options.Port, + s.webhook.pki.publicKeyPEM, + &s.webhook.mutationYAML) + + updateValidationWebhookConfig( + s, s.integrationTestClient, + s.webhook.validationOpts, validationWebhookConfig, + svr.Options.Host, svr.Options.Port, + s.webhook.pki.publicKeyPEM, + &s.webhook.validationYAML) + } // It can take a few seconds for the webhook server to come online. // This step blocks until the webserver can be successfully accessed. @@ -492,11 +665,13 @@ func (s *TestSuite) beforeSuiteForIntegrationTesting() { // PKI toolchain to use with the webhook server. if s.isWebhookTest() { By("generating the pki toolchain", func() { - s.pki, err = generatePKIToolchain() + s.webhook.pki, err = generatePKIToolchain() Expect(err).ToNot(HaveOccurred()) // Write the CA pub key and cert pub and private keys to the cert dir. - Expect(os.WriteFile(path.Join(s.certDir, "tls.crt"), s.pki.publicKeyPEM, 0400)).To(Succeed()) - Expect(os.WriteFile(path.Join(s.certDir, "tls.key"), s.pki.privateKeyPEM, 0400)).To(Succeed()) + tlsCrtPath := path.Join(s.webhook.certDir, "tls.crt") + tlsKeyPath := path.Join(s.webhook.certDir, "tls.key") + Expect(os.WriteFile(tlsCrtPath, s.webhook.pki.publicKeyPEM, 0400)).To(Succeed()) + Expect(os.WriteFile(tlsKeyPath, s.webhook.pki.privateKeyPEM, 0400)).To(Succeed()) }) } @@ -506,7 +681,7 @@ func (s *TestSuite) beforeSuiteForIntegrationTesting() { s.initializeManager() }) - s.integrationTestClient, err = client.New(s.manager.GetConfig(), client.Options{Scheme: s.manager.GetScheme()}) + s.integrationTestClient, err = ctrlclient.New(s.manager.GetConfig(), ctrlclient.Options{Scheme: s.manager.GetScheme()}) Expect(err).NotTo(HaveOccurred()) By("create pod namespace", func() { @@ -543,9 +718,41 @@ func (s *TestSuite) afterSuiteForIntegrationTesting() { Eventually(s.getManagerRunning).Should(BeFalse()) - if s.webhookYaml != nil { + if data := s.webhook.conversionName; len(data) > 0 { + By("tearing down conversion webhooks") + for i := range data { + name := data[i] + crd := apiextensionsv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } + Expect(s.integrationTestClient.Get(s, ctrlclient.ObjectKey{Name: name}, &crd)).To(Succeed()) + Expect(crd.Spec.Conversion).ToNot(BeNil()) + crd.Spec.Conversion = nil + Expect(s.integrationTestClient.Update(s, &crd)).To(Succeed()) + Expect(s.integrationTestClient.Get(s, ctrlclient.ObjectKey{Name: name}, &crd)).To(Succeed()) + Expect(crd.Spec.Conversion).ToNot(BeNil()) + Expect(crd.Spec.Conversion.Strategy).To(Equal(apiextensionsv1.NoneConverter)) + Expect(crd.Spec.Conversion.Webhook).To(BeNil()) + } + } + + if data := s.webhook.mutationYAML; len(data) > 0 { + By("tearing down mutation webhooks") Eventually(func() error { - return remote.DeleteYAML(s, s.integrationTestClient, s.webhookYaml) + return remote.DeleteYAML(s, s.integrationTestClient, data) + }).Should(Succeed()) + } + + if data := s.webhook.validationYAML; len(data) > 0 { + By("tearing down validation webhooks") + Eventually(func() error { + return remote.DeleteYAML(s, s.integrationTestClient, data) }).Should(Succeed()) } }) @@ -604,7 +811,7 @@ func (s *TestSuite) UpdateCRDScope(oldCrd *apiextensionsv1.CustomResourceDefinit s.envTest.CRDs = append(s.envTest.CRDs, crds...) } -func parseWebhookConfig(path string) ( +func parseAdmissionWebhookManifestFile(path string) ( mutatingWebhookConfig admissionregv1.MutatingWebhookConfiguration, validatingWebhookConfig admissionregv1.ValidatingWebhookConfiguration) { @@ -629,44 +836,164 @@ func parseWebhookConfig(path string) ( return mutatingWebhookConfig, validatingWebhookConfig } -func updateValidatingWebhookConfig(webhookConfig admissionregv1.ValidatingWebhookConfiguration, webhookName, host string, port int, key []byte) []byte { +func updateConversionWebhookConfig( + ctx context.Context, + client ctrlclient.Client, + webhookOption []TestSuiteConversionWebhookOptions, + host string, + port int, + key []byte, + addrOfName *[]string) { + + if len(webhookOption) == 0 { + return + } + + By("installing conversion webhooks") + + for _, in := range webhookOption { + crd := apiextensionsv1.CustomResourceDefinition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.k8s.io/v1", + Kind: "CustomResourceDefinition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: in.Name, + }, + } + Expect(client.Get(ctx, ctrlclient.ObjectKey{Name: in.Name}, &crd)).To(Succeed()) + + crd.Spec.Conversion = &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ConversionReviewVersions: []string{"v1", "v1beta1"}, + ClientConfig: &apiextensionsv1.WebhookClientConfig{}, + }, + } + updateAPIExtensionWebhookConfig( + host, port, "/convert", + key, crd.Spec.Conversion.Webhook.ClientConfig) + + Expect(client.Update(ctx, &crd)).To(Succeed()) + + *addrOfName = append(*addrOfName, in.Name) + } +} + +func updateMutationWebhookConfig( + ctx context.Context, + client ctrlclient.Client, + webhookOption []TestSuiteMutationWebhookOptions, + webhookConfig admissionregv1.MutatingWebhookConfiguration, + host string, + port int, + key []byte, + addrOfYAML *[]byte) { + + if len(webhookOption) == 0 { + return + } + + By("installing mutation webhooks") + webhookConfigToInstall := webhookConfig.DeepCopy() - // ITERATE over all of the defined webhooks and find the webhook - // the test suite is testing and update its client config to point - // to the test webhook server. - // 1. Use the test CA - // 2. Use the test webhook endpoint - for _, webhook := range webhookConfig.Webhooks { - if webhook.Name == webhookName { - url := fmt.Sprintf("https://%s:%d%s", host, port, *webhook.ClientConfig.Service.Path) - webhook.ClientConfig.CABundle = key - webhook.ClientConfig.Service = nil - webhook.ClientConfig.URL = &url - webhookConfigToInstall.Webhooks = []admissionregv1.ValidatingWebhook{webhook} + webhookConfigToInstall.Webhooks = nil + + for _, in := range webhookOption { + for _, out := range webhookConfig.Webhooks { + if in.Name == out.Name { + updateAdmissionWebhookConfig( + host, port, *out.ClientConfig.Service.Path, key, + &out.ClientConfig) + webhookConfigToInstall.Webhooks = append( + webhookConfigToInstall.Webhooks, + out) + } } } - result, err := yaml.Marshal(webhookConfigToInstall) + + if len(webhookConfigToInstall.Webhooks) == 0 { + return + } + + By(fmt.Sprintf("installing %d mutation webhooks", + len(webhookConfigToInstall.Webhooks))) + + data, err := yaml.Marshal(webhookConfigToInstall) Expect(err).ShouldNot(HaveOccurred()) - return result -} + *addrOfYAML = data + + // ASSERT that eventually the webhook config gets successfully + // applied to the API server. + Eventually(func() error { + return remote.ApplyYAML(ctx, client, data) + }).Should(Succeed()) +} + +func updateValidationWebhookConfig( + ctx context.Context, + client ctrlclient.Client, + webhookOption []TestSuiteValidationWebhookOptions, + webhookConfig admissionregv1.ValidatingWebhookConfiguration, + host string, + port int, + key []byte, + addrOfYAML *[]byte) { + + if len(webhookOption) == 0 { + return + } + + By("installing validation webhooks") -func updateMutatingWebhookConfig(webhookConfig admissionregv1.MutatingWebhookConfiguration, webhookName, host string, port int, key []byte) []byte { webhookConfigToInstall := webhookConfig.DeepCopy() - // ITERATE over all of the defined webhooks and find the webhook - // the test suite is testing and update its client config to point - // to the test webhook server. - // 1. Use the test CA - // 2. Use the test webhook endpoint - for _, webhook := range webhookConfig.Webhooks { - if webhook.Name == webhookName { - url := fmt.Sprintf("https://%s:%d%s", host, port, *webhook.ClientConfig.Service.Path) - webhook.ClientConfig.CABundle = key - webhook.ClientConfig.Service = nil - webhook.ClientConfig.URL = &url - webhookConfigToInstall.Webhooks = []admissionregv1.MutatingWebhook{webhook} + webhookConfigToInstall.Webhooks = nil + + for _, in := range webhookOption { + for _, out := range webhookConfig.Webhooks { + if in.Name == out.Name { + updateAdmissionWebhookConfig( + host, port, *out.ClientConfig.Service.Path, key, + &out.ClientConfig) + webhookConfigToInstall.Webhooks = append( + webhookConfigToInstall.Webhooks, + out) + } } } - result, err := yaml.Marshal(webhookConfigToInstall) + + if len(webhookConfigToInstall.Webhooks) == 0 { + return + } + + By(fmt.Sprintf("installing %d validation webhooks", + len(webhookConfigToInstall.Webhooks))) + + data, err := yaml.Marshal(webhookConfigToInstall) Expect(err).ShouldNot(HaveOccurred()) - return result + *addrOfYAML = data + + // ASSERT that eventually the webhook config gets successfully + // applied to the API server. + Eventually(func() error { + return remote.ApplyYAML(ctx, client, data) + }).Should(Succeed()) +} + +func updateAdmissionWebhookConfig( + host string, port int, path string, key []byte, + webhookConfig *admissionregv1.WebhookClientConfig) { + + webhookConfig.CABundle = key + webhookConfig.Service = nil + webhookConfig.URL = addrOf(fmt.Sprintf("https://%s:%d%s", host, port, path)) +} + +func updateAPIExtensionWebhookConfig( + host string, port int, path string, key []byte, + webhookConfig *apiextensionsv1.WebhookClientConfig) { + + webhookConfig.CABundle = key + webhookConfig.Service = nil + webhookConfig.URL = addrOf(fmt.Sprintf("https://%s:%d%s", host, port, path)) } diff --git a/test/builder/util.go b/test/builder/util.go index 60cdcf9c3..480665585 100644 --- a/test/builder/util.go +++ b/test/builder/util.go @@ -662,3 +662,7 @@ func readDocuments(fp string) ([][]byte, error) { } return docs, nil } + +func addrOf[T any](t T) *T { + return &t +} diff --git a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go index b179281bc..b36a41aec 100644 --- a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go @@ -51,7 +51,7 @@ const ( 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" + storageClassNotFoundFmt = "Storage policy is not found" invalidVolumeSpecified = "only one of persistentVolumeClaim or vsphereVolume must be specified" vSphereVolumeSizeNotMBMultiple = "value must be a multiple of MB" eagerZeroedAndThinProvisionedNotSupported = "Volume provisioning cannot have EagerZeroed and ThinProvisioning set. Eager zeroing requires thick provisioning" @@ -276,8 +276,7 @@ func (v validator) validateStorageClass(ctx *context.WebhookRequestContext, vm * 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, namespace))) + return append(allErrs, field.Invalid(scPath, scName, storageClassNotFoundFmt)) } resourceQuotas := &corev1.ResourceQuotaList{} diff --git a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_unit_test.go b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_unit_test.go index 71de9334d..4225775e0 100644 --- a/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_unit_test.go +++ b/webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_unit_test.go @@ -488,7 +488,7 @@ func unitTestsValidateCreate() { field.Forbidden(field.NewPath("spec", "advancedOptions", "defaultVolumeProvisioningOptions"), "Volume provisioning cannot have EagerZeroed and ThinProvisioning set. Eager zeroing requires thick provisioning").Error(), nil), Entry("should deny a storage class 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), + field.Invalid(specPath.Child("storageClass"), builder.DummyStorageClassName, "Storage policy is not found").Error(), nil), Entry("should deny a storage class 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 empty vmMetadata resource Names", createArgs{emptyMetadataResource: true}, true, nil, nil), diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go index 36b8f288b..3dab7d378 100644 --- a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go @@ -50,7 +50,7 @@ const ( 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" + storageClassNotFoundFmt = "Storage policy is not found" 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" @@ -352,7 +352,7 @@ func (v validator) validateStorageClass(ctx *context.WebhookRequestContext, vm * 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))) + return append(allErrs, field.Invalid(scPath, scName, storageClassNotFoundFmt)) } resourceQuotas := &corev1.ResourceQuotaList{} diff --git a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go index 9d37930af..c3148d5ee 100644 --- a/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go +++ b/webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go @@ -289,7 +289,7 @@ func unitTestsValidateCreate() { 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), + field.Invalid(specPath.Child("storageClass"), builder.DummyStorageClassName, "Storage policy is not found").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),