From c95e17bfffb4e7be9cad281d7ff7e6640897072b Mon Sep 17 00:00:00 2001 From: Prajyot-Parab Date: Fri, 10 Jun 2022 23:46:11 +0530 Subject: [PATCH] Move machinepool and AKS feature gate checks to webhooks Signed-off-by: Prajyot-Parab --- exp/api/v1beta1/azuremachinepool_test.go | 6 ++++ exp/api/v1beta1/azuremachinepool_webhook.go | 11 +++++++ .../v1beta1/azuremachinepool_webhook_test.go | 15 ++++++++++ .../azuremachinepoolmachine_webhook.go | 12 ++++++++ .../v1beta1/azuremanagedcluster_webhook.go | 10 +++++++ .../azuremanagedcluster_webhook_test.go | 6 ++++ main.go | 29 +++++++++---------- 7 files changed, 74 insertions(+), 15 deletions(-) diff --git a/exp/api/v1beta1/azuremachinepool_test.go b/exp/api/v1beta1/azuremachinepool_test.go index 6d206da12bc..8ead3958e31 100644 --- a/exp/api/v1beta1/azuremachinepool_test.go +++ b/exp/api/v1beta1/azuremachinepool_test.go @@ -21,8 +21,11 @@ import ( "github.com/Azure/go-autorest/autorest/to" "github.com/onsi/gomega" + utilfeature "k8s.io/component-base/featuregate/testing" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/feature" + capifeature "sigs.k8s.io/cluster-api/feature" ) func TestAzureMachinePool_Validate(t *testing.T) { @@ -132,6 +135,9 @@ func TestAzureMachinePool_Validate(t *testing.T) { c := c t.Run(c.Name, func(t *testing.T) { t.Parallel() + // NOTE: AzureMachinePool is behind MachinePool feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, capifeature.MachinePool, true)() g := gomega.NewGomegaWithT(t) amp := c.Factory(g) actualErr := amp.Validate(nil) diff --git a/exp/api/v1beta1/azuremachinepool_webhook.go b/exp/api/v1beta1/azuremachinepool_webhook.go index 0b08850c19e..368012a1c75 100644 --- a/exp/api/v1beta1/azuremachinepool_webhook.go +++ b/exp/api/v1beta1/azuremachinepool_webhook.go @@ -26,6 +26,8 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation/field" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/feature" + capifeature "sigs.k8s.io/cluster-api/feature" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" ) @@ -70,6 +72,15 @@ 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/azuremachinepool_webhook_test.go b/exp/api/v1beta1/azuremachinepool_webhook_test.go index b157fb2e7cf..002e1ee0e55 100644 --- a/exp/api/v1beta1/azuremachinepool_webhook_test.go +++ b/exp/api/v1beta1/azuremachinepool_webhook_test.go @@ -28,7 +28,10 @@ import ( "golang.org/x/crypto/ssh" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/uuid" + utilfeature "k8s.io/component-base/featuregate/testing" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/feature" + capifeature "sigs.k8s.io/cluster-api/feature" ) var ( @@ -36,6 +39,10 @@ var ( ) func TestAzureMachinePool_ValidateCreate(t *testing.T) { + // NOTE: AzureMachinePool is behind MachinePool feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, capifeature.MachinePool, true)() + g := NewWithT(t) var ( @@ -149,6 +156,10 @@ func TestAzureMachinePool_ValidateCreate(t *testing.T) { } func TestAzureMachinePool_ValidateUpdate(t *testing.T) { + // NOTE: AzureMachinePool is behind MachinePool feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, capifeature.MachinePool, true)() + g := NewWithT(t) var ( @@ -224,6 +235,10 @@ func TestAzureMachinePool_ValidateUpdate(t *testing.T) { } func TestAzureMachinePool_Default(t *testing.T) { + // NOTE: AzureMachinePool is behind MachinePool feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, capifeature.MachinePool, true)() + g := NewWithT(t) type test struct { diff --git a/exp/api/v1beta1/azuremachinepoolmachine_webhook.go b/exp/api/v1beta1/azuremachinepoolmachine_webhook.go index bb77cf595e9..701d3d425f7 100644 --- a/exp/api/v1beta1/azuremachinepoolmachine_webhook.go +++ b/exp/api/v1beta1/azuremachinepoolmachine_webhook.go @@ -19,6 +19,9 @@ package v1beta1 import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/cluster-api-provider-azure/feature" + capifeature "sigs.k8s.io/cluster-api/feature" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" ) @@ -41,6 +44,15 @@ func (ampm *AzureMachinePoolMachine) ValidateCreate() error { // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. func (ampm *AzureMachinePoolMachine) ValidateUpdate(old runtime.Object) error { + // NOTE: AzureMachinePoolMachine 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", + ) + } + oldMachine, ok := old.(*AzureMachinePoolMachine) if !ok { return errors.New("expected and AzureMachinePoolMachine") diff --git a/exp/api/v1beta1/azuremanagedcluster_webhook.go b/exp/api/v1beta1/azuremanagedcluster_webhook.go index f4c2e5da372..e8e8cd8a826 100644 --- a/exp/api/v1beta1/azuremanagedcluster_webhook.go +++ b/exp/api/v1beta1/azuremanagedcluster_webhook.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/feature" "sigs.k8s.io/cluster-api-provider-azure/util/maps" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -47,6 +48,15 @@ func (r *AzureManagedCluster) ValidateCreate() error { // 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. + if !feature.Gates.Enabled(feature.AKS) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the AKS feature flag is enabled", + ) + } + old := oldRaw.(*AzureManagedCluster) var allErrs field.ErrorList diff --git a/exp/api/v1beta1/azuremanagedcluster_webhook_test.go b/exp/api/v1beta1/azuremanagedcluster_webhook_test.go index 1ceb2045b1b..73a8e66b882 100644 --- a/exp/api/v1beta1/azuremanagedcluster_webhook_test.go +++ b/exp/api/v1beta1/azuremanagedcluster_webhook_test.go @@ -21,9 +21,15 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/component-base/featuregate/testing" + "sigs.k8s.io/cluster-api-provider-azure/feature" ) func TestAzureManagedCluster_ValidateUpdate(t *testing.T) { + // NOTE: AzureManagedCluster 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 4fa6aa4d4d5..e6be45517ce 100644 --- a/main.go +++ b/main.go @@ -496,24 +496,23 @@ func registerWebhooks(mgr manager.Manager) { setupLog.Error(err, "unable to create webhook", "webhook", "AzureClusterIdentity") os.Exit(1) } - // just use CAPI MachinePool feature flag rather than create a new one - if feature.Gates.Enabled(capifeature.MachinePool) { - if err := (&infrav1beta1exp.AzureMachinePool{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "AzureMachinePool") - os.Exit(1) - } + // NOTE: AzureMachinePool is behind MachinePool feature gate flag; the webhook + // is going to prevent creating or updating new objects in case the feature flag is disabled + if err := (&infrav1beta1exp.AzureMachinePool{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "AzureMachinePool") + os.Exit(1) + } - if err := (&infrav1beta1exp.AzureMachinePoolMachine{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "AzureMachinePoolMachine") - os.Exit(1) - } + if err := (&infrav1beta1exp.AzureMachinePoolMachine{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "AzureMachinePoolMachine") + os.Exit(1) } - if feature.Gates.Enabled(feature.AKS) { - if err := (&infrav1beta1exp.AzureManagedCluster{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "AzureManagedCluster") - os.Exit(1) - } + // NOTE: AzureManagedCluster is behind AKS feature gate flag; the webhook + // is going to prevent creating or updating new objects in case the feature flag is disabled + if err := (&infrav1beta1exp.AzureManagedCluster{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "AzureManagedCluster") + os.Exit(1) } if feature.Gates.Enabled(feature.AKS) {