feat: Add k8s version logic for external cloud-provider flag

This flag has been removed from kube-apiserver in k8s v1.33 so need to
use version specific logic to add it for pre-1.33 clusters.

Author: jimmidyson

charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/aws-cluster-class.yaml

@@ -84,7 +84,6 @@ spec:
         clusterConfiguration:
           apiServer:
             extraArgs:
-              cloud-provider: external
               profiling: "false"
           controllerManager:
             extraArgs:

charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/nutanix-cluster-class.yaml

@@ -123,7 +123,6 @@ spec:
         clusterConfiguration:
           apiServer:
             extraArgs:
-              cloud-provider: external
               profiling: "false"
               tls-cipher-suites: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
           controllerManager:

common/pkg/testutils/capitest/patches.go

@@ -63,17 +63,15 @@ func AssertGeneratePatches[T mutation.GeneratePatches](
 	}
 	resp := &runtimehooksv1.GeneratePatchesResponse{}
 	h.GeneratePatches(context.Background(), req, resp)
-	expectedStatus := runtimehooksv1.ResponseStatusSuccess
 	if tt.ExpectedFailure {
-		expectedStatus = runtimehooksv1.ResponseStatusFailure
+		g.Expect(resp.Status).
+			To(gomega.Equal(runtimehooksv1.ResponseStatusFailure), fmt.Sprintf("Message: %s", resp.Message))
+		return
 	}
+
 	g.Expect(resp.Status).
-		To(gomega.Equal(expectedStatus), fmt.Sprintf("Message: %s", resp.Message))
+		To(gomega.Equal(runtimehooksv1.ResponseStatusSuccess), fmt.Sprintf("Message: %s", resp.Message))
 
-	if len(tt.ExpectedPatchMatchers) == 0 {
-		g.Expect(resp.Items).To(gomega.BeEmpty())
-		return
-	}
 	g.Expect(resp.Items).To(containPatches(&tt.RequestItem, tt.ExpectedPatchMatchers...))
 
 	if len(tt.UnexpectedPatchMatchers) > 0 {
@@ -111,6 +109,17 @@ func containPatches(
 	requestItem *runtimehooksv1.GeneratePatchesRequestItem,
 	jsonMatchers ...JSONPatchMatcher,
 ) gomega.OmegaMatcher {
+	if len(jsonMatchers) == 0 {
+		return gomega.SatisfyAny(
+			gomega.BeEmpty(),
+			gomega.ContainElement(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
+				"UID":       gomega.Equal(requestItem.UID),
+				"PatchType": gomega.Equal(runtimehooksv1.JSONPatchType),
+				"Patch":     gomega.Equal([]byte("[]")),
+			})),
+		)
+	}
+
 	patchMatchers := make([]interface{}, 0, len(jsonMatchers))
 	for patchIdx := range jsonMatchers {
 		unexpectedPatch := jsonMatchers[patchIdx]

common/pkg/testutils/capitest/request/items.go

@@ -4,11 +4,14 @@
 package request
 
 import (
+	"maps"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/utils/ptr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
@@ -88,7 +91,9 @@ func NewKubeadmConfigTemplateRequest(
 }
 
 type KubeadmControlPlaneTemplateRequestItemBuilder struct {
-	files []bootstrapv1.File
+	files              []bootstrapv1.File
+	version            *string
+	apiServerExtraArgs map[string]string
 }
 
 func (b *KubeadmControlPlaneTemplateRequestItemBuilder) WithFiles(
@@ -98,43 +103,75 @@ func (b *KubeadmControlPlaneTemplateRequestItemBuilder) WithFiles(
 	return b
 }
 
+func (b *KubeadmControlPlaneTemplateRequestItemBuilder) WithKubernetesVersion(
+	version string,
+) *KubeadmControlPlaneTemplateRequestItemBuilder {
+	b.version = ptr.To(version)
+	return b
+}
+
+func (b *KubeadmControlPlaneTemplateRequestItemBuilder) WithAPIServerExtraArgs(
+	extraArgs map[string]string,
+) *KubeadmControlPlaneTemplateRequestItemBuilder {
+	b.apiServerExtraArgs = extraArgs
+	return b
+}
+
 func (b *KubeadmControlPlaneTemplateRequestItemBuilder) NewRequest(
 	uid types.UID,
 ) runtimehooksv1.GeneratePatchesRequestItem {
-	return NewRequestItem(
-		&controlplanev1.KubeadmControlPlaneTemplate{
-			TypeMeta: metav1.TypeMeta{
-				APIVersion: controlplanev1.GroupVersion.String(),
-				Kind:       "KubeadmControlPlaneTemplate",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      kubeadmControlPlaneTemplateRequestObjectName,
-				Namespace: Namespace,
-			},
-			Spec: controlplanev1.KubeadmControlPlaneTemplateSpec{
-				Template: controlplanev1.KubeadmControlPlaneTemplateResource{
-					Spec: controlplanev1.KubeadmControlPlaneTemplateResourceSpec{
-						KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{
-							InitConfiguration: &bootstrapv1.InitConfiguration{
-								NodeRegistration: bootstrapv1.NodeRegistrationOptions{
-									KubeletExtraArgs: map[string]string{
-										"cloud-provider": "external",
-									},
+	cpTemplate := &controlplanev1.KubeadmControlPlaneTemplate{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: controlplanev1.GroupVersion.String(),
+			Kind:       "KubeadmControlPlaneTemplate",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      kubeadmControlPlaneTemplateRequestObjectName,
+			Namespace: Namespace,
+		},
+		Spec: controlplanev1.KubeadmControlPlaneTemplateSpec{
+			Template: controlplanev1.KubeadmControlPlaneTemplateResource{
+				Spec: controlplanev1.KubeadmControlPlaneTemplateResourceSpec{
+					KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{
+						InitConfiguration: &bootstrapv1.InitConfiguration{
+							NodeRegistration: bootstrapv1.NodeRegistrationOptions{
+								KubeletExtraArgs: map[string]string{
+									"cloud-provider": "external",
 								},
 							},
-							JoinConfiguration: &bootstrapv1.JoinConfiguration{
-								NodeRegistration: bootstrapv1.NodeRegistrationOptions{
-									KubeletExtraArgs: map[string]string{
-										"cloud-provider": "external",
-									},
+						},
+						JoinConfiguration: &bootstrapv1.JoinConfiguration{
+							NodeRegistration: bootstrapv1.NodeRegistrationOptions{
+								KubeletExtraArgs: map[string]string{
+									"cloud-provider": "external",
 								},
 							},
-							Files: b.files,
 						},
+						Files: b.files,
 					},
 				},
 			},
 		},
+	}
+
+	if b.version != nil {
+		if cpTemplate.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration == nil {
+			cpTemplate.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration = &bootstrapv1.ClusterConfiguration{}
+		}
+		cpTemplate.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.KubernetesVersion = *b.version
+	}
+
+	if b.apiServerExtraArgs != nil {
+		if cpTemplate.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration == nil {
+			cpTemplate.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration = &bootstrapv1.ClusterConfiguration{}
+		}
+		cpTemplate.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs = maps.Clone(
+			b.apiServerExtraArgs,
+		)
+	}
+
+	return NewRequestItem(
+		cpTemplate,
 		&runtimehooksv1.HolderReference{
 			APIVersion: clusterv1.GroupVersion.String(),
 			Kind:       "Cluster",

hack/examples/bases/aws/clusterclass/kustomization.yaml.tmpl

@@ -34,9 +34,6 @@ patches:
 - target:
     kind: KubeadmControlPlaneTemplate
   patch: |-
-    - op: "replace"
-      path: "/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/extraArgs/cloud-provider"
-      value: "external"
     - op: "replace"
       path: "/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/controllerManager/extraArgs/cloud-provider"
       value: "external"
@@ -52,6 +49,13 @@ patches:
     - op: "replace"
       path: "/spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/cloud-provider"
       value: "external"
+# Delete the API server cloud-provider flag from the template.
+# They will be added by the handler for k8s < 1.33.
+- target:
+    kind: KubeadmControlPlaneTemplate
+  patch: |-
+    - op: "remove"
+      path: "/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/extraArgs/cloud-provider"
 
 # Delete the cluster-specific resources.
 - target:

hack/examples/bases/nutanix/clusterclass/kustomization.yaml.tmpl

@@ -46,6 +46,16 @@ patches:
     - op: "remove"
       path: "/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs"
 
+# TODO: Remove once https://github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pull/519 is
+# merged and released.
+# Delete the API server cloud-provider flag from the template.
+# They will be added by the handler for k8s < 1.33.
+- target:
+    kind: KubeadmControlPlaneTemplate
+  patch: |-
+    - op: "remove"
+      path: "/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/extraArgs/cloud-provider"
+
 # Template the kube-vip file.
 # The handler will set the variables if needed, or remove it.
 - target:

pkg/handlers/generic/mutation/externalcloudprovider/inject.go

@@ -0,0 +1,86 @@
+// Copyright 2023 Nutanix. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+package externalcloudprovider
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
+	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
+	ctrl "sigs.k8s.io/controller-runtime"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/blang/semver/v4"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers/mutation"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/patches/selectors"
+)
+
+var (
+	k8sLessThan133Range = semver.MustParseRange("<1.33.0-0")
+)
+
+type externalCloudProviderPatchHandler struct{}
+
+func NewPatch() *externalCloudProviderPatchHandler {
+	return &externalCloudProviderPatchHandler{}
+}
+
+func (h *externalCloudProviderPatchHandler) Mutate(
+	ctx context.Context,
+	obj *unstructured.Unstructured,
+	vars map[string]apiextensionsv1.JSON,
+	holderRef runtimehooksv1.HolderReference,
+	_ ctrlclient.ObjectKey,
+	_ mutation.ClusterGetter,
+) error {
+	log := ctrl.LoggerFrom(ctx).WithValues(
+		"holderRef", holderRef,
+	)
+
+	if err := patches.MutateIfApplicable(
+		obj, vars, &holderRef, selectors.ControlPlane(), log,
+		func(obj *controlplanev1.KubeadmControlPlaneTemplate) error {
+			if obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration == nil ||
+				obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.KubernetesVersion == "" {
+				err := errors.New("missing control plane Kubernetes version")
+				log.Error(err, "unable to add external cloud-provider flag")
+				return err
+			}
+
+			cpK8sVersion, err := semver.ParseTolerant(
+				obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.KubernetesVersion,
+			)
+			if err != nil {
+				log.WithValues(
+					"kubernetesVersion",
+					obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.KubernetesVersion,
+				).Error(err, "failed to parse control plane Kubernetes version")
+				return fmt.Errorf("failed to parse control plane Kubernetes version: %w", err)
+			}
+
+			if k8sLessThan133Range(cpK8sVersion) {
+				log.WithValues(
+					"patchedObjectKind", obj.GetObjectKind().GroupVersionKind().String(),
+					"patchedObjectName", ctrlclient.ObjectKeyFromObject(obj),
+				).Info(
+					"adding external cloud-provider flag to control plane kubeadm config template because Kubernetes < 1.33.0",
+				)
+
+				if obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs == nil {
+					obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs = make(map[string]string, 1)
+				}
+				obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs["cloud-provider"] = "external"
+			}
+
+			return nil
+		}); err != nil {
+		return err
+	}
+
+	return nil
+}