Cleanup

kubernetes-sigs · Oct 18, 2023 · e3bc4cd · e3bc4cd
1 parent c52e29f
commit e3bc4cd
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 48 deletions.
diff --git 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
@@ -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
@@ -36,28 +36,24 @@ 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 {
-	mpw := &AzureManagedMachinePoolTemplateWebhook{Client: mgr.GetClient()}
+// 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{}).
 		WithDefaulter(mpw).
 		WithValidator(mpw).
 		Complete()
 }
 
-// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepooltemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azuremanagedmachinepooltemplates,versions=v1beta1,name=validation.azuremanagedmachinepooltemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
-// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepooltemplate,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azuremanagedmachinepooltemplates,versions=v1beta1,name=default.azuremanagedmachinepooltemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
+//+kubebuilder:webhook:path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepooltemplate,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azuremanagedmachinepooltemplates,verbs=create;update,versions=v1beta1,name=default.azuremanagedmachinepooltemplates.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
 
-type AzureManagedMachinePoolTemplateWebhook struct {
+type azureManagedMachinePoolTemplateWebhook struct {
 	Client client.Client
 }
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (mpw *AzureManagedMachinePoolTemplateWebhook) Default(ctx context.Context, obj runtime.Object) error {
+func (mpw *azureManagedMachinePoolTemplateWebhook) Default(ctx context.Context, obj runtime.Object) error {
 	mp, ok := obj.(*AzureManagedMachinePoolTemplate)
 	if !ok {
 		return apierrors.NewBadRequest("expected an AzureManagedMachinePoolTemplate")
@@ -78,8 +74,10 @@ func (mpw *AzureManagedMachinePoolTemplateWebhook) Default(ctx context.Context,
 	return nil
 }
 
+//+kubebuilder:webhook:verbs=create;update;delete,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepooltemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=azuremanagedmachinepooltemplates,versions=v1beta1,name=validation.azuremanagedmachinepooltemplates.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
+
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (mpw *AzureManagedMachinePoolTemplateWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+func (mpw *azureManagedMachinePoolTemplateWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
 	mp, ok := obj.(*AzureManagedMachinePoolTemplate)
 	if !ok {
 		return nil, apierrors.NewBadRequest("expected an AzureManagedMachinePoolTemplate")
@@ -96,45 +94,45 @@ 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)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (mpw *AzureManagedMachinePoolTemplateWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+func (mpw *azureManagedMachinePoolTemplateWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
 	var allErrs field.ErrorList
 	old, ok := oldObj.(*AzureManagedMachinePoolTemplate)
 	if !ok {
@@ -291,7 +289,7 @@ func (mpw *AzureManagedMachinePoolTemplateWebhook) ValidateUpdate(ctx context.Co
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (mpw *AzureManagedMachinePoolTemplateWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+func (mpw *azureManagedMachinePoolTemplateWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
 	mp, ok := obj.(*AzureManagedMachinePoolTemplate)
 	if !ok {
 		return nil, apierrors.NewBadRequest("expected an AzureManagedMachinePoolTemplate")

diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -158,6 +158,28 @@ webhooks:
     resources:
     - azuremanagedmachinepools
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepooltemplate
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: default.azuremanagedmachinepooltemplates.infrastructure.cluster.x-k8s.io
+  rules:
+  - apiGroups:
+    - infrastructure.cluster.x-k8s.io
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - azuremanagedmachinepooltemplates
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   - v1beta1
@@ -403,6 +425,29 @@ webhooks:
     resources:
     - azuremanagedmachinepools
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-infrastructure-cluster-x-k8s-io-v1beta1-azuremanagedmachinepooltemplate
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: validation.azuremanagedmachinepooltemplates.infrastructure.cluster.x-k8s.io
+  rules:
+  - apiGroups:
+    - infrastructure.cluster.x-k8s.io
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    - UPDATE
+    - DELETE
+    resources:
+    - azuremanagedmachinepooltemplates
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   - v1beta1

diff --git a/main.go b/main.go
@@ -539,12 +539,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)
 	}