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 index 4c5e5b850..04be532d9 100644 --- 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 index e81b4e74c..2cf268f5b 100644 --- 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 index 0af85ad4e..0091204e0 100644 --- 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 index a7998ba18..77b8eae5a 100644 --- a/common/pkg/testutils/capitest/patches.go +++ b/common/pkg/testutils/capitest/patches.go @@ -63,17 +63,16 @@ 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(expectedStatus), fmt.Sprintf("Message: %s", resp.Message)) - - if len(tt.ExpectedPatchMatchers) == 0 { + g.Expect(resp.Status). + To(gomega.Equal(runtimehooksv1.ResponseStatusFailure), fmt.Sprintf("Message: %s", resp.Message)) g.Expect(resp.Items).To(gomega.BeEmpty()) return } + + g.Expect(resp.Status). + To(gomega.Equal(runtimehooksv1.ResponseStatusSuccess), fmt.Sprintf("Message: %s", resp.Message)) + g.Expect(resp.Items).To(containPatches(&tt.RequestItem, tt.ExpectedPatchMatchers...)) if len(tt.UnexpectedPatchMatchers) > 0 { @@ -111,6 +110,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 index 6b4b2a116..7bb4b0a81 100644 --- 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 index ef475a75d..d26955365 100644 --- 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 index fda261b99..e7167dd96 100644 --- 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 new file mode 100644 index 000000000..2c312508c --- /dev/null +++ b/pkg/handlers/generic/mutation/externalcloudprovider/inject.go @@ -0,0 +1,99 @@ +// 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 ( + versionGreaterOrEqualTo133Range = 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) + } + + kubernetesVersion, 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 versionGreaterOrEqualTo133Range(kubernetesVersion) { + 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) + } + if _, ok := obj.Spec.Template.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer.ExtraArgs["cloud-provider"]; !ok { + 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 new file mode 100644 index 000000000..2dc007022 --- /dev/null +++ b/pkg/handlers/generic/mutation/externalcloudprovider/inject_test.go @@ -0,0 +1,113 @@ +// Copyright 2023 Nutanix. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package externalcloudprovider + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + + "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/testutils/capitest" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/testutils/capitest/request" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/test/helpers" +) + +func TestExternalCloudProviderPatch(t *testing.T) { + gomega.RegisterFailHandler(Fail) + RunSpecs(t, "External cloud provider mutator suite") +} + +var _ = Describe("Generate external cloud provider patches", func() { + // only add aws region patch + patchGenerator := func() mutation.GeneratePatches { + return mutation.NewMetaGeneratePatchesHandler("", helpers.TestEnv.Client, NewControlPlanePatch()).(mutation.GeneratePatches) + } + + testDefs := []capitest.PatchTestDef{ + { + Name: "no kubernetes version available", + RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""), + ExpectedFailure: true, + }, + { + Name: "explicit empty kubernetes version specified", + RequestItem: (&request.KubeadmControlPlaneTemplateRequestItemBuilder{}).WithKubernetesVersion("").NewRequest(""), + ExpectedFailure: true, + }, + { + Name: "non-semver kubernetes version specified", + RequestItem: (&request.KubeadmControlPlaneTemplateRequestItemBuilder{}). + WithKubernetesVersion("this is not semver").NewRequest(""), + ExpectedFailure: true, + }, + { + Name: "add API server flag for pre-1.33.0 control plane", + RequestItem: (&request.KubeadmControlPlaneTemplateRequestItemBuilder{}). + WithKubernetesVersion("1.32.29").NewRequest(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/clusterConfiguration", + ValueMatcher: gomega.HaveKeyWithValue( + "apiServer", + gomega.Equal(map[string]interface{}{ + "extraArgs": map[string]interface{}{ + "cloud-provider": "external", + }, + }), + ), + }, + }, + }, + { + Name: "add API server flag for pre-1.33.0 control plane with pre-existing extra args", + RequestItem: (&request.KubeadmControlPlaneTemplateRequestItemBuilder{}). + WithKubernetesVersion("1.32.29"). + WithAPIServerExtraArgs(map[string]string{"foo": "bar"}). + NewRequest(""), + ExpectedPatchMatchers: []capitest.JSONPatchMatcher{ + { + Operation: "add", + Path: "/spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/extraArgs/cloud-provider", + ValueMatcher: gomega.Equal("external"), + }, + }, + }, + { + Name: "no patches added for >= 1.33.0 control plane", + RequestItem: (&request.KubeadmControlPlaneTemplateRequestItemBuilder{}). + WithKubernetesVersion("1.33.0"). + WithAPIServerExtraArgs(map[string]string{"foo": "bar"}). + NewRequest(""), + }, + { + Name: "no patches added for >= 1.33.0 control plane take 2", + RequestItem: (&request.KubeadmControlPlaneTemplateRequestItemBuilder{}). + WithKubernetesVersion("2.72.0"). + WithAPIServerExtraArgs(map[string]string{"foo": "bar"}). + NewRequest(""), + }, + { + Name: "no patches added for 1.33.0 pre-release control plane", + RequestItem: (&request.KubeadmControlPlaneTemplateRequestItemBuilder{}). + WithKubernetesVersion("1.33.0-alpha.1"). + WithAPIServerExtraArgs(map[string]string{"foo": "bar"}). + NewRequest(""), + }, + } + + // create test node for each case + for testIdx := range testDefs { + tt := testDefs[testIdx] + It(tt.Name, func() { + capitest.AssertGeneratePatches( + GinkgoT(), + patchGenerator, + &tt, + ) + }) + } +}) diff --git a/pkg/handlers/generic/mutation/handlers.go b/pkg/handlers/generic/mutation/handlers.go index b6576829c..21c28f5e2 100644 --- a/pkg/handlers/generic/mutation/handlers.go +++ b/pkg/handlers/generic/mutation/handlers.go @@ -16,6 +16,7 @@ import ( "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/coredns" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/encryptionatrest" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/etcd" + "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/externalcloudprovider" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/extraapiservercertsans" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/httpproxy" "github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/handlers/generic/mutation/imageregistries/credentials" @@ -61,6 +62,7 @@ func ControlPlaneMetaMutators() []mutation.MetaMutator { return []mutation.MetaMutator{ taints.NewControlPlanePatch(), noderegistration.NewControlPlanePatch(), + externalcloudprovider.NewControlPlanePatch(), } }