From f1600b141fe7dd0bf1bd78ef045f1a312b9ced27 Mon Sep 17 00:00:00 2001 From: willie-yao Date: Wed, 18 Oct 2023 20:47:40 +0000 Subject: [PATCH] Cleanup --- .../azuremanagedclustertemplate_webhook.go | 26 ++++++++++------- ...zuremanagedcontrolplanetemplate_webhook.go | 29 +++++++++---------- ...azuremanagedmachinepooltemplate_webhook.go | 23 +++++++-------- main.go | 7 ++++- 4 files changed, 45 insertions(+), 40 deletions(-) diff --git a/api/v1beta1/azuremanagedclustertemplate_webhook.go b/api/v1beta1/azuremanagedclustertemplate_webhook.go index 51b4b8055a38..3cb304e83cf2 100644 --- a/api/v1beta1/azuremanagedclustertemplate_webhook.go +++ b/api/v1beta1/azuremanagedclustertemplate_webhook.go @@ -17,21 +17,20 @@ limitations under the License. package v1beta1 import ( + "fmt" "reflect" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/cluster-api-provider-azure/feature" + "sigs.k8s.io/cluster-api-provider-azure/util/maps" capifeature "sigs.k8s.io/cluster-api/feature" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// AzureManagedClusterTemplateImmutableMsg is the message used for errors on fields that are immutable. -const AzureManagedClusterTemplateImmutableMsg = "AzureManagedClusterTemplate spec.template.spec field is immutable. Please create new resource instead. ref doc: https://cluster-api.sigs.k8s.io/tasks/experimental-features/cluster-class/change-clusterclass.html" - // SetupWebhookWithManager sets up and registers the webhook with the manager. func (r *AzureManagedClusterTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). @@ -58,18 +57,25 @@ func (r *AzureManagedClusterTemplate) ValidateCreate() (admission.Warnings, erro // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. func (r *AzureManagedClusterTemplate) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) { - var allErrs field.ErrorList old := oldRaw.(*AzureManagedClusterTemplate) - if !reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) { + var allErrs field.ErrorList + + // custom headers are immutable + oldCustomHeaders := maps.FilterByKeyPrefix(old.ObjectMeta.Annotations, CustomHeaderPrefix) + newCustomHeaders := maps.FilterByKeyPrefix(r.ObjectMeta.Annotations, CustomHeaderPrefix) + if !reflect.DeepEqual(oldCustomHeaders, newCustomHeaders) { allErrs = append(allErrs, - field.Invalid(field.NewPath("AzureManagedClusterTemplate", "spec", "template", "spec"), rScanInterval, AzureManagedClusterTemplateImmutableMsg), - ) + field.Invalid( + field.NewPath("metadata", "annotations"), + r.ObjectMeta.Annotations, + fmt.Sprintf("annotations with '%s' prefix are immutable", CustomHeaderPrefix))) } - if len(allErrs) == 0 { - return nil, nil + if len(allErrs) != 0 { + return nil, apierrors.NewInvalid(GroupVersion.WithKind("AzureManagedClusterTemplate").GroupKind(), r.Name, allErrs) } - return nil, apierrors.NewInvalid(GroupVersion.WithKind("AzureManagedClusterTemplate").GroupKind(), r.Name, allErrs) + + return nil, nil } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. diff --git a/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go b/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go index 2a5873f1ab92..4f9d6ba6ad0c 100644 --- a/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go +++ b/api/v1beta1/azuremanagedcontrolplanetemplate_webhook.go @@ -31,11 +31,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// AzureManagedControlPlaneTemplateImmutableMsg is the message used for errors on fields that are immutable. -const AzureManagedControlPlaneTemplateImmutableMsg = "AzureManagedControlPlaneTemplate spec.template.spec field is immutable. Please create new resource instead. ref doc: https://cluster-api.sigs.k8s.io/tasks/experimental-features/cluster-class/change-clusterclass.html" - -// SetupAzureManagedControlPlaneTemplateWithManager will set up the webhook to be managed by the specified manager. -func SetupAzureManagedControlPlaneTemplateWithManager(mgr ctrl.Manager) error { +// SetupAzureManagedControlPlaneTemplateWebhookWithManager will set up the webhook to be managed by the specified manager. +func SetupAzureManagedControlPlaneTemplateWebhookWithManager(mgr ctrl.Manager) error { mcpw := &azureManagedControlPlaneTemplateWebhook{Client: mgr.GetClient()} return ctrl.NewWebhookManagedBy(mgr). For(&AzureManagedControlPlaneTemplate{}). @@ -67,7 +64,7 @@ func (mcpw *azureManagedControlPlaneTemplateWebhook) ValidateCreate(ctx context. if !ok { return nil, apierrors.NewBadRequest("expected an AzureManagedControlPlaneTemplate") } - // NOTE: AzureManagedControlPlane relies upon MachinePools, which is behind a feature gate flag. + // NOTE: AzureManagedControlPlaneTemplate relies upon MachinePools, which is behind a feature gate flag. // The webhook must prevent creating new objects in case the feature flag is disabled. if !feature.Gates.Enabled(capifeature.MachinePool) { return nil, field.Forbidden( @@ -91,42 +88,42 @@ func (mcpw *azureManagedControlPlaneTemplateWebhook) ValidateUpdate(ctx context. return nil, apierrors.NewBadRequest("expected an AzureManagedControlPlaneTemplate") } if err := webhookutils.ValidateImmutable( - field.NewPath("Spec", "SubscriptionID"), + field.NewPath("Spec", "Template", "Spec", "SubscriptionID"), old.Spec.Template.Spec.SubscriptionID, mcp.Spec.Template.Spec.SubscriptionID); err != nil { allErrs = append(allErrs, err) } if err := webhookutils.ValidateImmutable( - field.NewPath("Spec", "Location"), + field.NewPath("Spec", "Template", "Spec", "Location"), old.Spec.Template.Spec.Location, mcp.Spec.Template.Spec.Location); err != nil { allErrs = append(allErrs, err) } if err := webhookutils.ValidateImmutable( - field.NewPath("Spec", "DNSServiceIP"), + field.NewPath("Spec", "Template", "Spec", "DNSServiceIP"), old.Spec.Template.Spec.DNSServiceIP, mcp.Spec.Template.Spec.DNSServiceIP); err != nil { allErrs = append(allErrs, err) } if err := webhookutils.ValidateImmutable( - field.NewPath("Spec", "NetworkPlugin"), + field.NewPath("Spec", "Template", "Spec", "NetworkPlugin"), old.Spec.Template.Spec.NetworkPlugin, mcp.Spec.Template.Spec.NetworkPlugin); err != nil { allErrs = append(allErrs, err) } if err := webhookutils.ValidateImmutable( - field.NewPath("Spec", "NetworkPolicy"), + field.NewPath("Spec", "Template", "Spec", "NetworkPolicy"), old.Spec.Template.Spec.NetworkPolicy, mcp.Spec.Template.Spec.NetworkPolicy); err != nil { allErrs = append(allErrs, err) } if err := webhookutils.ValidateImmutable( - field.NewPath("Spec", "LoadBalancerSKU"), + field.NewPath("Spec", "Template", "Spec", "LoadBalancerSKU"), old.Spec.Template.Spec.LoadBalancerSKU, mcp.Spec.Template.Spec.LoadBalancerSKU); err != nil { allErrs = append(allErrs, err) @@ -136,21 +133,21 @@ func (mcpw *azureManagedControlPlaneTemplateWebhook) ValidateUpdate(ctx context. if mcp.Spec.Template.Spec.AADProfile == nil { allErrs = append(allErrs, field.Invalid( - field.NewPath("Spec", "AADProfile"), + field.NewPath("Spec", "Template", "Spec", "AADProfile"), mcp.Spec.Template.Spec.AADProfile, "field cannot be nil, cannot disable AADProfile")) } else { if !mcp.Spec.Template.Spec.AADProfile.Managed && old.Spec.Template.Spec.AADProfile.Managed { allErrs = append(allErrs, field.Invalid( - field.NewPath("Spec", "AADProfile.Managed"), + field.NewPath("Spec", "Template", "Spec", "AADProfile.Managed"), mcp.Spec.Template.Spec.AADProfile.Managed, "cannot set AADProfile.Managed to false")) } if len(mcp.Spec.Template.Spec.AADProfile.AdminGroupObjectIDs) == 0 { allErrs = append(allErrs, field.Invalid( - field.NewPath("Spec", "AADProfile.AdminGroupObjectIDs"), + field.NewPath("Spec", "Template", "Spec", "AADProfile.AdminGroupObjectIDs"), mcp.Spec.Template.Spec.AADProfile.AdminGroupObjectIDs, "length of AADProfile.AdminGroupObjectIDs cannot be zero")) } @@ -161,7 +158,7 @@ func (mcpw *azureManagedControlPlaneTemplateWebhook) ValidateUpdate(ctx context. // Updating outboundType after cluster creation (PREVIEW) // https://learn.microsoft.com/en-us/azure/aks/egress-outboundtype#updating-outboundtype-after-cluster-creation-preview if err := webhookutils.ValidateImmutable( - field.NewPath("Spec", "OutboundType"), + field.NewPath("Spec", "Template", "Spec", "OutboundType"), old.Spec.Template.Spec.OutboundType, mcp.Spec.Template.Spec.OutboundType); err != nil { allErrs = append(allErrs, err) diff --git a/api/v1beta1/azuremanagedmachinepooltemplate_webhook.go b/api/v1beta1/azuremanagedmachinepooltemplate_webhook.go index 979432d9e7ef..2995d7007d7f 100644 --- a/api/v1beta1/azuremanagedmachinepooltemplate_webhook.go +++ b/api/v1beta1/azuremanagedmachinepooltemplate_webhook.go @@ -36,11 +36,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// AzureManagedMachinePoolTemplateImmutableMsg is the message used for errors on fields that are immutable. -const AzureManagedMachinePoolTemplateImmutableMsg = "AzureManagedMachinePoolTemplate spec.template.spec field is immutable. Please create new resource instead. ref doc: https://cluster-api.sigs.k8s.io/tasks/experimental-features/cluster-class/change-clusterclass.html" - -// SetupAzureManagedMachinePoolTemplateWithManager will set up the webhook to be managed by the specified manager. -func SetupAzureManagedMachinePoolTemplateWithManager(mgr ctrl.Manager) error { +// SetupAzureManagedMachinePoolTemplateWebhookWithManager will set up the webhook to be managed by the specified manager. +func SetupAzureManagedMachinePoolTemplateWebhookWithManager(mgr ctrl.Manager) error { mpw := &AzureManagedMachinePoolTemplateWebhook{Client: mgr.GetClient()} return ctrl.NewWebhookManagedBy(mgr). For(&AzureManagedMachinePoolTemplate{}). @@ -96,39 +93,39 @@ func (mpw *AzureManagedMachinePoolTemplateWebhook) ValidateCreate(ctx context.Co errs = append(errs, validateMaxPods( mp.Spec.Template.Spec.MaxPods, - field.NewPath("Spec", "MaxPods"))) + field.NewPath("Spec", "Template", "Spec", "MaxPods"))) errs = append(errs, validateOSType( mp.Spec.Template.Spec.Mode, mp.Spec.Template.Spec.OSType, - field.NewPath("Spec", "OSType"))) + field.NewPath("Spec", "Template", "Spec", "OSType"))) errs = append(errs, validateAgentPoolName( mp.Spec.Template.Spec.OSType, mp.Spec.Template.Spec.Name, - field.NewPath("Spec", "Name"))) + field.NewPath("Spec", "Template", "Spec", "Name"))) errs = append(errs, validateNodeLabels( mp.Spec.Template.Spec.NodeLabels, - field.NewPath("Spec", "NodeLabels"))) + field.NewPath("Spec", "Template", "Spec", "NodeLabels"))) errs = append(errs, validateNodePublicIPPrefixID( mp.Spec.Template.Spec.NodePublicIPPrefixID, - field.NewPath("Spec", "NodePublicIPPrefixID"))) + field.NewPath("Spec", "Template", "Spec", "NodePublicIPPrefixID"))) errs = append(errs, validateEnableNodePublicIP( mp.Spec.Template.Spec.EnableNodePublicIP, mp.Spec.Template.Spec.NodePublicIPPrefixID, - field.NewPath("Spec", "EnableNodePublicIP"))) + field.NewPath("Spec", "Template", "Spec", "EnableNodePublicIP"))) errs = append(errs, validateKubeletConfig( mp.Spec.Template.Spec.KubeletConfig, - field.NewPath("Spec", "KubeletConfig"))) + field.NewPath("Spec", "Template", "Spec", "KubeletConfig"))) errs = append(errs, validateLinuxOSConfig( mp.Spec.Template.Spec.LinuxOSConfig, mp.Spec.Template.Spec.KubeletConfig, - field.NewPath("Spec", "LinuxOSConfig"))) + field.NewPath("Spec", "Template", "Spec", "LinuxOSConfig"))) return nil, kerrors.NewAggregate(errs) } diff --git a/main.go b/main.go index 46132b5597aa..50b408985c25 100644 --- a/main.go +++ b/main.go @@ -537,12 +537,17 @@ func registerWebhooks(mgr manager.Manager) { os.Exit(1) } + if err := infrav1.SetupAzureManagedMachinePoolTemplateWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "AzureManagedMachinePoolTemplate") + os.Exit(1) + } + if err := infrav1.SetupAzureManagedControlPlaneWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "AzureManagedControlPlane") os.Exit(1) } - if err := infrav1.SetupAzureManagedControlPlaneTemplateWithManager(mgr); err != nil { + if err := infrav1.SetupAzureManagedControlPlaneTemplateWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "AzureManagedControlPlaneTemplate") os.Exit(1) }