Skip to content

Commit

Permalink
standardize AzureManagedCluster webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
jackfrancis committed Oct 15, 2022
1 parent f9a6449 commit 81f657a
Show file tree
Hide file tree
Showing 9 changed files with 241 additions and 58 deletions.
17 changes: 8 additions & 9 deletions exp/api/v1beta1/azuremachinepool_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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,
Expand Down
71 changes: 66 additions & 5 deletions exp/api/v1beta1/azuremachinepool_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import (

var (
validSSHPublicKey = generateSSHPublicKey(true)
zero = intstr.FromInt(0)
one = intstr.FromInt(1)
)

func TestAzureMachinePool_ValidateCreate(t *testing.T) {
Expand All @@ -45,16 +47,16 @@ func TestAzureMachinePool_ValidateCreate(t *testing.T) {

g := NewWithT(t)

var (
zero = intstr.FromInt(0)
one = intstr.FromInt(1)
)

tests := []struct {
name string
amp *AzureMachinePool
wantErr bool
}{
{
name: "valid",
amp: getKnownValidAzureMachinePool(),
wantErr: false,
},
{
name: "azuremachinepool with marketplace image - full",
amp: createMachinePoolWithMarketPlaceImage("PUB1234", "OFFER1234", "SKU1234", "1.0.0", to.IntPtr(10)),
Expand Down Expand Up @@ -378,3 +380,62 @@ func createMachinePoolWithStrategy(strategy AzureMachinePoolDeploymentStrategy)
},
}
}

func TestAzureMachinePool_ValidateCreateFailure(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
amp *AzureMachinePool
deferFunc func()
}{
{
name: "feature gate explicitly disabled",
amp: getKnownValidAzureMachinePool(),
deferFunc: utilfeature.SetFeatureGateDuringTest(t, feature.Gates, capifeature.MachinePool, false),
},
{
name: "feature gate implicitly disabled",
amp: getKnownValidAzureMachinePool(),
deferFunc: func() {},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
defer tc.deferFunc()
err := tc.amp.ValidateCreate()
g.Expect(err).To(HaveOccurred())
})
}
}

func getKnownValidAzureMachinePool() *AzureMachinePool {
image := infrav1.Image{
Marketplace: &infrav1.AzureMarketplaceImage{
ImagePlan: infrav1.ImagePlan{
Publisher: "PUB1234",
Offer: "OFFER1234",
SKU: "SKU1234",
},
Version: "1.0.0",
},
}
return &AzureMachinePool{
Spec: AzureMachinePoolSpec{
Template: AzureMachinePoolMachineTemplate{
Image: &image,
SSHPublicKey: validSSHPublicKey,
TerminateNotificationTimeout: to.IntPtr(10),
},
Identity: infrav1.VMIdentitySystemAssigned,
RoleAssignmentName: string(uuid.NewUUID()),
Strategy: AzureMachinePoolDeploymentStrategy{
Type: RollingUpdateAzureMachinePoolDeploymentStrategyType,
RollingUpdate: &MachineRollingUpdateDeployment{
MaxSurge: &zero,
MaxUnavailable: &one,
},
},
},
}
}
11 changes: 5 additions & 6 deletions exp/api/v1beta1/azuremanagedcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
32 changes: 32 additions & 0 deletions exp/api/v1beta1/azuremanagedcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,35 @@ func TestAzureManagedCluster_ValidateUpdate(t *testing.T) {
})
}
}

func TestAzureManagedCluster_ValidateCreateFailure(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
amc *AzureManagedCluster
deferFunc func()
}{
{
name: "feature gate explicitly disabled",
amc: getKnownValidAzureManagedCluster(),
deferFunc: utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.AKS, false),
},
{
name: "feature gate implicitly disabled",
amc: getKnownValidAzureManagedCluster(),
deferFunc: func() {},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
defer tc.deferFunc()
err := tc.amc.ValidateCreate()
g.Expect(err).To(HaveOccurred())
})
}
}

func getKnownValidAzureManagedCluster() *AzureManagedCluster {
return &AzureManagedCluster{}
}
9 changes: 9 additions & 0 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
kerrors "k8s.io/apimachinery/pkg/util/errors"
"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"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -80,6 +81,14 @@ func (m *AzureManagedControlPlane) Default(_ client.Client) {

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
func (m *AzureManagedControlPlane) ValidateCreate(client client.Client) error {
// NOTE: AzureManagedControlPlane 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",
)
}
return m.Validate(client)
}

Expand Down
71 changes: 55 additions & 16 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"github.com/Azure/go-autorest/autorest/to"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilfeature "k8s.io/component-base/featuregate/testing"
"k8s.io/utils/pointer"
"sigs.k8s.io/cluster-api-provider-azure/feature"
)

func TestDefaultingWebhook(t *testing.T) {
Expand Down Expand Up @@ -77,6 +79,10 @@ func TestDefaultingWebhook(t *testing.T) {
}

func TestValidatingWebhook(t *testing.T) {
// NOTE: AzureManageControlPlane 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 {
name string
amcp AzureManagedControlPlane
Expand Down Expand Up @@ -239,8 +245,6 @@ func TestValidatingWebhook(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
t.Parallel()
if tt.expectErr {
g.Expect(tt.amcp.ValidateCreate(nil)).NotTo(Succeed())
} else {
Expand All @@ -251,6 +255,9 @@ func TestValidatingWebhook(t *testing.T) {
}

func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
// NOTE: AzureManageControlPlane 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 {
Expand All @@ -260,20 +267,8 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
errorLen int
}{
{
name: "all valid",
amcp: &AzureManagedControlPlane{
Spec: AzureManagedControlPlaneSpec{
DNSServiceIP: to.StringPtr("192.168.0.0"),
Version: "v1.18.0",
SSHPublicKey: generateSSHPublicKey(true),
AADProfile: &AADProfile{
Managed: true,
AdminGroupObjectIDs: []string{
"616077a8-5db7-4c98-b856-b34619afg75h",
},
},
},
},
name: "all valid",
amcp: getKnownValidAzureManagedControlPlane(),
wantErr: false,
},
{
Expand Down Expand Up @@ -344,6 +339,34 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
}
}

func TestAzureManagedControlPlane_ValidateCreateFailure(t *testing.T) {
g := NewWithT(t)

tests := []struct {
name string
amcp *AzureManagedControlPlane
deferFunc func()
}{
{
name: "feature gate explicitly disabled",
amcp: getKnownValidAzureManagedControlPlane(),
deferFunc: utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.AKS, false),
},
{
name: "feature gate implicitly disabled",
amcp: getKnownValidAzureManagedControlPlane(),
deferFunc: func() {},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
defer tc.deferFunc()
err := tc.amcp.ValidateCreate(nil)
g.Expect(err).To(HaveOccurred())
})
}
}

func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) {
g := NewWithT(t)

Expand Down Expand Up @@ -901,3 +924,19 @@ func createAzureManagedControlPlane(serviceIP, version, sshKey string) *AzureMan
},
}
}

func getKnownValidAzureManagedControlPlane() *AzureManagedControlPlane {
return &AzureManagedControlPlane{
Spec: AzureManagedControlPlaneSpec{
DNSServiceIP: to.StringPtr("192.168.0.0"),
Version: "v1.18.0",
SSHPublicKey: generateSSHPublicKey(true),
AADProfile: &AADProfile{
Managed: true,
AdminGroupObjectIDs: []string{
"616077a8-5db7-4c98-b856-b34619afg75h",
},
},
},
}
}
9 changes: 9 additions & 0 deletions exp/api/v1beta1/azuremanagedmachinepool_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ 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"
Expand Down Expand Up @@ -59,6 +60,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,
Expand Down
Loading

0 comments on commit 81f657a

Please sign in to comment.