Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Move machinepool and CRS feature gate checks to webhooks #6348

Merged
merged 2 commits into from
Mar 30, 2022
Merged

✨ Move machinepool and CRS feature gate checks to webhooks #6348

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e622cbb
Move machinepool and CRS feature gate checks to webhooks
Karthik-K-N Mar 30, 2022
d8f9646
Updated import order to fix golint
Karthik-K-N Mar 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion exp/addons/api/v1beta1/clusterresourceset_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion exp/api/v1alpha3/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Karthik-K-N marked this conversation as resolved.
Show resolved Hide resolved
)

func TestMachinePoolConversion(t *testing.T) {
// NOTE: MachinePool feature flag is disabled by default, thus preventing to create or update MachinePool.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I think it's fine to add this as part of this PR, but we should open an issue to update how the conversion tests/webhook in the MachinePool is done. I'm not sure what our testing area is exactly if we have this test in v1alpha3, but nothing in v1alpha4.

// 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())
Expand Down
9 changes: 9 additions & 0 deletions exp/api/v1beta1/machinepool_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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(
Expand Down
21 changes: 20 additions & 1 deletion exp/api/v1beta1/machinepool_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Karthik-K-N marked this conversation as resolved.
Show resolved Hide resolved
)

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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 8 additions & 10 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Karthik-K-N marked this conversation as resolved.
Show resolved Hide resolved
setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSet")
os.Exit(1)
}

if err := (&clusterv1.MachineHealthCheck{}).SetupWebhookWithManager(mgr); err != nil {
Expand Down