From 7315412b4745b56bbaadc1c7c8324337703c2194 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Fri, 14 Oct 2022 11:01:05 -0700 Subject: [PATCH] standardize AzureManagedCluster webhooks --- exp/api/v1beta1/azuremachinepool_webhook.go | 17 ++++++----- .../v1beta1/azuremanagedcluster_webhook.go | 11 ++++---- .../azuremanagedmachinepool_webhook.go | 17 +++++++++++ .../azuremanagedmachinepool_webhook_test.go | 5 ++++ main.go | 28 +++++++++---------- 5 files changed, 48 insertions(+), 30 deletions(-) diff --git a/exp/api/v1beta1/azuremachinepool_webhook.go b/exp/api/v1beta1/azuremachinepool_webhook.go index 368012a1c755..1dcb692432ce 100644 --- a/exp/api/v1beta1/azuremachinepool_webhook.go +++ b/exp/api/v1beta1/azuremachinepool_webhook.go @@ -57,6 +57,14 @@ var _ webhook.Validator = &AzureMachinePool{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. func (amp *AzureMachinePool) ValidateCreate() error { + // NOTE: AzureMachinePool is behind MachinePool feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + if !feature.Gates.Enabled(capifeature.MachinePool) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the MachinePool feature flag is enabled", + ) + } return amp.Validate(nil) } @@ -72,15 +80,6 @@ func (amp *AzureMachinePool) ValidateDelete() error { // Validate the Azure Machine Pool and return an aggregate error. func (amp *AzureMachinePool) Validate(old runtime.Object) error { - // NOTE: AzureMachinePool is behind MachinePool feature gate flag; the web hook - // must prevent creating new objects new case the feature flag is disabled. - if !feature.Gates.Enabled(capifeature.MachinePool) { - return field.Forbidden( - field.NewPath("spec"), - "can be set only if the MachinePool feature flag is enabled", - ) - } - validators := []func() error{ amp.ValidateImage, amp.ValidateTerminateNotificationTimeout, diff --git a/exp/api/v1beta1/azuremanagedcluster_webhook.go b/exp/api/v1beta1/azuremanagedcluster_webhook.go index e8e8cd8a826d..40e901468572 100644 --- a/exp/api/v1beta1/azuremanagedcluster_webhook.go +++ b/exp/api/v1beta1/azuremanagedcluster_webhook.go @@ -43,20 +43,19 @@ var _ webhook.Validator = &AzureManagedCluster{} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. func (r *AzureManagedCluster) ValidateCreate() error { - return nil -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (r *AzureManagedCluster) ValidateUpdate(oldRaw runtime.Object) error { // NOTE: AzureManagedCluster is behind AKS feature gate flag; the web hook - // must prevent creating new objects new case the feature flag is disabled. + // must prevent creating new objects in case the feature flag is disabled. if !feature.Gates.Enabled(feature.AKS) { return field.Forbidden( field.NewPath("spec"), "can be set only if the AKS feature flag is enabled", ) } + return nil +} +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. +func (r *AzureManagedCluster) ValidateUpdate(oldRaw runtime.Object) error { old := oldRaw.(*AzureManagedCluster) var allErrs field.ErrorList diff --git a/exp/api/v1beta1/azuremanagedmachinepool_webhook.go b/exp/api/v1beta1/azuremanagedmachinepool_webhook.go index 38cca1a246ab..9c99d19afc9c 100644 --- a/exp/api/v1beta1/azuremanagedmachinepool_webhook.go +++ b/exp/api/v1beta1/azuremanagedmachinepool_webhook.go @@ -28,12 +28,21 @@ import ( kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/feature" azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure" "sigs.k8s.io/cluster-api-provider-azure/util/maps" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) +// SetupWebhookWithManager sets up and registers the webhook with the manager. +func (m *AzureManagedMachinePool) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(m). + Complete() +} + //+kubebuilder:webhook:path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepool,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azuremanagedmachinepools,verbs=create;update,versions=v1beta1,name=default.azuremanagedmachinepools.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 // Default implements webhook.Defaulter so a webhook will be registered for the type. @@ -56,6 +65,14 @@ func (m *AzureManagedMachinePool) Default(client client.Client) { // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. func (m *AzureManagedMachinePool) ValidateCreate(client client.Client) error { + // NOTE: AzureManagedMachinePool is behind AKS feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + if !feature.Gates.Enabled(feature.AKS) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the AKS feature flag is enabled", + ) + } validators := []func() error{ m.validateMaxPods, m.validateOSType, diff --git a/exp/api/v1beta1/azuremanagedmachinepool_webhook_test.go b/exp/api/v1beta1/azuremanagedmachinepool_webhook_test.go index b449b84a6f63..b7b7ad6ed06a 100644 --- a/exp/api/v1beta1/azuremanagedmachinepool_webhook_test.go +++ b/exp/api/v1beta1/azuremanagedmachinepool_webhook_test.go @@ -24,7 +24,9 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/component-base/featuregate/testing" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/feature" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -568,6 +570,9 @@ func TestValidateBoolPtrImmutable(t *testing.T) { } func TestAzureManagedMachinePool_ValidateCreate(t *testing.T) { + // NOTE: AzureManagedMachinePool is behind AKS feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.AKS, true)() g := NewWithT(t) tests := []struct { diff --git a/main.go b/main.go index 494c86fdb28d..ab35e46925bf 100644 --- a/main.go +++ b/main.go @@ -523,21 +523,19 @@ func registerWebhooks(mgr manager.Manager) { os.Exit(1) } - if feature.Gates.Enabled(feature.AKS) { - hookServer := mgr.GetWebhookServer() - hookServer.Register("/mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepool", webhook.NewMutatingWebhook( - &infrav1exp.AzureManagedMachinePool{}, mgr.GetClient(), - )) - hookServer.Register("/validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepool", webhook.NewValidatingWebhook( - &infrav1exp.AzureManagedMachinePool{}, mgr.GetClient(), - )) - hookServer.Register("/mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedcontrolplane", webhook.NewMutatingWebhook( - &infrav1exp.AzureManagedControlPlane{}, mgr.GetClient(), - )) - hookServer.Register("/validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedcontrolplane", webhook.NewValidatingWebhook( - &infrav1exp.AzureManagedControlPlane{}, mgr.GetClient(), - )) - } + hookServer := mgr.GetWebhookServer() + hookServer.Register("/mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepool", webhook.NewMutatingWebhook( + &infrav1exp.AzureManagedMachinePool{}, mgr.GetClient(), + )) + hookServer.Register("/validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepool", webhook.NewValidatingWebhook( + &infrav1exp.AzureManagedMachinePool{}, mgr.GetClient(), + )) + hookServer.Register("/mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedcontrolplane", webhook.NewMutatingWebhook( + &infrav1exp.AzureManagedControlPlane{}, mgr.GetClient(), + )) + hookServer.Register("/validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedcontrolplane", webhook.NewValidatingWebhook( + &infrav1exp.AzureManagedControlPlane{}, mgr.GetClient(), + )) if err := mgr.AddReadyzCheck("webhook", mgr.GetWebhookServer().StartedChecker()); err != nil { setupLog.Error(err, "unable to create ready check")