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

jimmidyson · jimmidyson · commit c3876bd45a82 · 2025-05-22T19:31:46.000+01:00
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.
diff --git a/charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/aws-cluster-class.yaml b/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:
diff --git a/charts/cluster-api-runtime-extensions-nutanix/defaultclusterclasses/nutanix-cluster-class.yaml b/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:
diff --git a/common/pkg/capi/clustertopology/variables/variable.go b/common/pkg/capi/clustertopology/variables/variable.go
@@ -33,6 +33,10 @@ func IsNotFoundError(err error) bool {
 	return topologymutation.IsNotFoundError(err) || errors.As(err, &fieldNotFoundError{})
 }
 
+func IsFieldNotFoundError(err error) bool {
+	return errors.As(err, &fieldNotFoundError{})
+}
+
 // Get finds and parses variable to given type.
 func Get[T any](
 	variables map[string]apiextensionsv1.JSON,
diff --git a/common/pkg/testutils/capitest/patches.go b/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]
diff --git a/common/pkg/testutils/capitest/request/items.go b/common/pkg/testutils/capitest/request/items.go
@@ -4,11 +4,16 @@
 package request
 
 import (
+	"encoding/json"
+	"maps"
+
 	corev1 "k8s.io/api/core/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/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 +93,9 @@ func NewKubeadmConfigTemplateRequest(
 }
 
 type KubeadmControlPlaneTemplateRequestItemBuilder struct {
-	files []bootstrapv1.File
+	files              []bootstrapv1.File
+	version            *string
+	apiServerExtraArgs map[string]string
 }
 
 func (b *KubeadmControlPlaneTemplateRequestItemBuilder) WithFiles(
@@ -98,43 +105,68 @@ 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.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,
+		)
+	}
+
+	requestItem := NewRequestItem(
+		cpTemplate,
 		&runtimehooksv1.HolderReference{
 			APIVersion: clusterv1.GroupVersion.String(),
 			Kind:       "Cluster",
@@ -144,6 +176,22 @@ func (b *KubeadmControlPlaneTemplateRequestItemBuilder) NewRequest(
 		},
 		uid,
 	)
+
+	if b.version != nil {
+		marshaledBuiltin, _ := json.Marshal( //nolint:errchkjson // Marshalling is guaranteed to succeed.
+			map[string]interface{}{
+				"controlPlane": map[string]interface{}{
+					"version": *b.version,
+				},
+			},
+		)
+		requestItem.Variables = append(requestItem.Variables, runtimehooksv1.Variable{
+			Name:  runtimehooksv1.BuiltinsName,
+			Value: apiextensionsv1.JSON{Raw: marshaledBuiltin},
+		})
+	}
+
+	return requestItem
 }
 
 func NewKubeadmControlPlaneTemplateRequestItem(
diff --git a/hack/examples/bases/aws/clusterclass/kustomization.yaml.tmpl b/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:
diff --git a/hack/examples/bases/nutanix/clusterclass/kustomization.yaml.tmpl b/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:
diff --git a/pkg/handlers/generic/mutation/externalcloudprovider/inject.go b/pkg/handlers/generic/mutation/externalcloudprovider/inject.go
@@ -0,0 +1,97 @@
+// Copyright 2023 Nutanix. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+package externalcloudprovider
+
+import (
+	"context"
+	"fmt"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
+	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"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/variables"
+)
+
+var (
+	versionGreatOrEqualTo133Range = semver.MustParseRange(">=1.33.0-0")
+)
+
+type externalCloudProviderPatchHandler struct{}
+
+func NewControlPlanePatch() *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,
+	)
+
+	cpVersion, err := variables.Get[string](vars, runtimehooksv1.BuiltinsName, "controlPlane", "version")
+	if err != nil {
+		// This builtin variable is guaranteed to be provided for control plane component patch requests so if it is not
+		// found then we can safely skip this patch for this request item.
+		if variables.IsFieldNotFoundError(err) {
+			log.V(5).
+				WithValues("variables", vars).
+				Info(
+					"skipping external cloud-provider flag to control plane because CP Kubernetes version is not found",
+				)
+			return nil
+		}
+
+		// This is a fatal error, we can't proceed without the control plane version.
+		log.WithValues("variables", vars).Error(err, "failed to get control plane Kubernetes version from builtin variable")
+		return fmt.Errorf("failed to get control plane Kubernetes version from builtin variable: %w", err)
+	}
+
+	cpK8sVersion, err := semver.ParseTolerant(cpVersion)
+	if err != nil {
+		log.WithValues(
+			"kubernetesVersion",
+			cpVersion,
+		).Error(err, "failed to parse control plane Kubernetes version")
+		return fmt.Errorf("failed to parse control plane Kubernetes version: %w", err)
+	}
+
+	if versionGreatOrEqualTo133Range(cpK8sVersion) {
+		log.V(5).Info(
+			"skipping external cloud-provider flag to control plane kubeadm config template because Kubernetes < 1.33.0",
+		)
+		return nil
+	}
+
+	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 = &bootstrapv1.ClusterConfiguration{}
+			}
+			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
+}
diff --git a/pkg/handlers/generic/mutation/externalcloudprovider/inject_test.go b/pkg/handlers/generic/mutation/externalcloudprovider/inject_test.go
diff --git a/pkg/handlers/generic/mutation/handlers.go b/pkg/handlers/generic/mutation/handlers.go
