From e622cbb1d20f9e3f62bee0100f8a82d4b518f373 Mon Sep 17 00:00:00 2001 From: Karthik K N Date: Wed, 30 Mar 2022 12:30:46 +0530 Subject: [PATCH 1/2] Move machinepool and CRS feature gate checks to webhooks --- .../api/v1beta1/clusterresourceset_webhook.go | 11 +++++++++- exp/api/v1alpha3/webhook_test.go | 8 ++++++- exp/api/v1beta1/machinepool_webhook.go | 9 ++++++++ exp/api/v1beta1/machinepool_webhook_test.go | 21 ++++++++++++++++++- main.go | 18 +++++++--------- 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/exp/addons/api/v1beta1/clusterresourceset_webhook.go b/exp/addons/api/v1beta1/clusterresourceset_webhook.go index 5c56cc777aeb..e33cba7faadd 100644 --- a/exp/addons/api/v1beta1/clusterresourceset_webhook.go +++ b/exp/addons/api/v1beta1/clusterresourceset_webhook.go @@ -26,6 +26,8 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" + + "sigs.k8s.io/cluster-api/feature" ) func (m *ClusterResourceSet) SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -68,8 +70,15 @@ func (m *ClusterResourceSet) ValidateDelete() error { } func (m *ClusterResourceSet) validate(old *ClusterResourceSet) error { + // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the web hook + // must prevent creating new objects new case the feature flag is disabled. + if !feature.Gates.Enabled(feature.ClusterResourceSet) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the ClusterResourceSet feature flag is enabled", + ) + } var allErrs field.ErrorList - // Validate selector parses as Selector selector, err := metav1.LabelSelectorAsSelector(&m.Spec.ClusterSelector) if err != nil { diff --git a/exp/api/v1alpha3/webhook_test.go b/exp/api/v1alpha3/webhook_test.go index 10f1b1e8d784..139d5eb419e1 100644 --- a/exp/api/v1alpha3/webhook_test.go +++ b/exp/api/v1alpha3/webhook_test.go @@ -20,17 +20,23 @@ import ( "fmt" "testing" - . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" clusterv1alpha3 "sigs.k8s.io/cluster-api/api/v1alpha3" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util" + + . "github.com/onsi/gomega" ) func TestMachinePoolConversion(t *testing.T) { + // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. + // Enabling the feature flag temporarily for this test. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)() g := NewWithT(t) ns, err := env.CreateNamespace(ctx, fmt.Sprintf("conversion-webhook-%s", util.RandomString(5))) g.Expect(err).ToNot(HaveOccurred()) diff --git a/exp/api/v1beta1/machinepool_webhook.go b/exp/api/v1beta1/machinepool_webhook.go index 73cb9bc4854c..1532bcaf42ab 100644 --- a/exp/api/v1beta1/machinepool_webhook.go +++ b/exp/api/v1beta1/machinepool_webhook.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util/version" ) @@ -93,6 +94,14 @@ func (m *MachinePool) ValidateDelete() error { } func (m *MachinePool) validate(old *MachinePool) error { + // NOTE: MachinePool 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(feature.MachinePool) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the MachinePool feature flag is enabled", + ) + } var allErrs field.ErrorList if m.Spec.Template.Spec.Bootstrap.ConfigRef == nil && m.Spec.Template.Spec.Bootstrap.DataSecretName == nil { allErrs = append( diff --git a/exp/api/v1beta1/machinepool_webhook_test.go b/exp/api/v1beta1/machinepool_webhook_test.go index 784a21dedaa8..a9bfd7fb0e6b 100644 --- a/exp/api/v1beta1/machinepool_webhook_test.go +++ b/exp/api/v1beta1/machinepool_webhook_test.go @@ -19,16 +19,23 @@ package v1beta1 import ( "testing" - . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/feature" utildefaulting "sigs.k8s.io/cluster-api/util/defaulting" + + . "github.com/onsi/gomega" ) func TestMachinePoolDefault(t *testing.T) { + // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. + // Enabling the feature flag temporarily for this test. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)() + g := NewWithT(t) m := &MachinePool{ @@ -56,6 +63,9 @@ func TestMachinePoolDefault(t *testing.T) { } func TestMachinePoolBootstrapValidation(t *testing.T) { + // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. + // Enabling the feature flag temporarily for this test. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)() tests := []struct { name string bootstrap clusterv1.Bootstrap @@ -102,6 +112,9 @@ func TestMachinePoolBootstrapValidation(t *testing.T) { } func TestMachinePoolNamespaceValidation(t *testing.T) { + // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. + // Enabling the feature flag temporarily for this test. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)() tests := []struct { name string expectErr bool @@ -167,6 +180,9 @@ func TestMachinePoolNamespaceValidation(t *testing.T) { } func TestMachinePoolClusterNameImmutable(t *testing.T) { + // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. + // Enabling the feature flag temporarily for this test. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)() tests := []struct { name string oldClusterName string @@ -223,6 +239,9 @@ func TestMachinePoolClusterNameImmutable(t *testing.T) { } func TestMachinePoolVersionValidation(t *testing.T) { + // NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool. + // Enabling the feature flag temporarily for this test. + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachinePool, true)() tests := []struct { name string expectErr bool diff --git a/main.go b/main.go index b336c5b7e2c3..577f7e983c09 100644 --- a/main.go +++ b/main.go @@ -447,18 +447,16 @@ func setupWebhooks(mgr ctrl.Manager) { os.Exit(1) } - if feature.Gates.Enabled(feature.MachinePool) { - if err := (&expv1.MachinePool{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "MachinePool") - os.Exit(1) - } + // NOTE: MachinePool 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 := (&expv1.MachinePool{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "MachinePool") + os.Exit(1) } - if feature.Gates.Enabled(feature.ClusterResourceSet) { - if err := (&addonsv1.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSet") - os.Exit(1) - } + if err := (&addonsv1.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSet") + os.Exit(1) } if err := (&clusterv1.MachineHealthCheck{}).SetupWebhookWithManager(mgr); err != nil { From d8f96462612c8cf26b59351183f46d67c9bda407 Mon Sep 17 00:00:00 2001 From: Karthik K N Date: Wed, 30 Mar 2022 17:37:07 +0530 Subject: [PATCH 2/2] Updated import order to fix golint --- exp/api/v1alpha3/webhook_test.go | 3 +-- exp/api/v1beta1/machinepool_webhook_test.go | 3 +-- main.go | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/exp/api/v1alpha3/webhook_test.go b/exp/api/v1alpha3/webhook_test.go index 139d5eb419e1..d57f29f9a6df 100644 --- a/exp/api/v1alpha3/webhook_test.go +++ b/exp/api/v1alpha3/webhook_test.go @@ -20,6 +20,7 @@ import ( "fmt" "testing" + . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilfeature "k8s.io/component-base/featuregate/testing" @@ -29,8 +30,6 @@ import ( clusterv1alpha3 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util" - - . "github.com/onsi/gomega" ) func TestMachinePoolConversion(t *testing.T) { diff --git a/exp/api/v1beta1/machinepool_webhook_test.go b/exp/api/v1beta1/machinepool_webhook_test.go index a9bfd7fb0e6b..8af1a03aee01 100644 --- a/exp/api/v1beta1/machinepool_webhook_test.go +++ b/exp/api/v1beta1/machinepool_webhook_test.go @@ -19,6 +19,7 @@ package v1beta1 import ( "testing" + . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilfeature "k8s.io/component-base/featuregate/testing" @@ -27,8 +28,6 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/feature" utildefaulting "sigs.k8s.io/cluster-api/util/defaulting" - - . "github.com/onsi/gomega" ) func TestMachinePoolDefault(t *testing.T) { diff --git a/main.go b/main.go index 577f7e983c09..356e2596dd01 100644 --- a/main.go +++ b/main.go @@ -454,6 +454,8 @@ func setupWebhooks(mgr ctrl.Manager) { os.Exit(1) } + // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the webhook + // is going to prevent creating or updating new objects in case the feature flag is disabled if err := (&addonsv1.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSet") os.Exit(1)