fix: Correctly handle variable overrides

api/go.mod

@@ -25,6 +25,7 @@ require (
 )
 
 require (
+	dario.cat/mergo v1.0.1 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect

api/go.sum

@@ -1,3 +1,5 @@
+dario.cat/mergo v1.0.1 h1:Ra4+bf83h2ztPIQYNP99R6m+Y7KfnARDfID+a+vLl4s=
+dario.cat/mergo v1.0.1/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk=
 github.com/aws/aws-sdk-go v1.55.8 h1:JRmEUbU52aJQZ2AjX4q4Wu7t4uZjOu71uyNmaWlUkJQ=
 github.com/aws/aws-sdk-go v1.55.8/go.mod h1:ZkViS9AqA6otK+JBBNH2++sx1sgxrPKcSzPPvQkUtXk=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=

common/go.mod

@@ -8,6 +8,7 @@ go 1.23.0
 toolchain go1.24.5
 
 require (
+	dario.cat/mergo v1.0.1
 	github.com/evanphx/json-patch/v5 v5.9.11
 	github.com/go-logr/logr v1.4.3
 	github.com/onsi/ginkgo/v2 v2.23.4

common/go.sum

@@ -1,5 +1,7 @@
 cel.dev/expr v0.18.0 h1:CJ6drgk+Hf96lkLikr4rFf19WrU0BOWEihyZnI2TAzo=
 cel.dev/expr v0.18.0/go.mod h1:MrpN08Q+lEBs+bGYdLxxHkZoUSsCp0nSKTs0nTymJgw=
+dario.cat/mergo v1.0.1 h1:Ra4+bf83h2ztPIQYNP99R6m+Y7KfnARDfID+a+vLl4s=
+dario.cat/mergo v1.0.1/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk=
 github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
 github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
 github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so=

common/pkg/capi/clustertopology/handlers/mutation/meta.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"sync"
 
+	"github.com/samber/lo"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -18,6 +19,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/handlers"
+	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/common/pkg/capi/clustertopology/variables"
 )
 
 type MutateFunc func(
@@ -89,6 +91,21 @@ func (mgp metaGeneratePatches) GeneratePatches(
 ) {
 	clusterKey := handlers.ClusterKeyFromReq(req)
 	clusterGetter := mgp.CreateClusterGetter(clusterKey)
+
+	// Create a map of variables from the request that can be overridden by machine deployment or control plane
+	// configuration.
+	// Filter out the "builtin" variable, which is already present in the vars map and should not be overridden by
+	// the global variables.
+	globalVars := lo.FilterSliceToMap(
+		req.Variables,
+		func(v runtimehooksv1.Variable) (string, apiextensionsv1.JSON, bool) {
+			if v.Name == "builtin" {
+				return "", apiextensionsv1.JSON{}, false
+			}
+			return v.Name, v.Value, true
+		},
+	)
+
 	topologymutation.WalkTemplates(
 		ctx,
 		unstructured.UnstructuredJSONScheme,
@@ -108,16 +125,33 @@ func (mgp metaGeneratePatches) GeneratePatches(
 
 			log.V(3).Info("Starting mutation pipeline", "handlerCount", len(mgp.mutators))
 
-			for i, h := range mgp.mutators {
-				handlerName := fmt.Sprintf("%T", h)
-				log.V(5).Info("Running mutator", "index", i, "handler", handlerName)
+			// Merge the global variables to the current resource vars. This allows the handlers to access
+			// the variables defined in the cluster class or cluster configuration and use these correctly when
+			// overrides are specified on machine deployment or control plane configuration.
+			mergedVars, err := variables.MergeVariableOverridesWithGlobal(vars, globalVars)
+			if err != nil {
+				log.Error(err, "Failed to merge global variables")
+				return err
+			}
 
-				if err := h.Mutate(ctx, obj.(*unstructured.Unstructured), vars, holderRef, clusterKey, clusterGetter); err != nil {
-					log.Error(err, "Mutator failed", "index", i, "handler", handlerName)
+			for i, h := range mgp.mutators {
+				mutatorType := fmt.Sprintf("%T", h)
+				log.V(5).
+					Info("Running mutator", "index", i, "handler", mutatorType, "vars", mergedVars)
+
+				if err := h.Mutate(
+					ctx,
+					obj.(*unstructured.Unstructured),
+					mergedVars,
+					holderRef,
+					clusterKey,
+					clusterGetter,
+				); err != nil {
+					log.Error(err, "Mutator failed", "index", i, "handler", mutatorType)
 					return err
 				}
 
-				log.V(5).Info("Mutator completed successfully", "index", i, "handler", handlerName)
+				log.V(5).Info("Mutator completed successfully", "index", i, "handler", mutatorType)
 			}
 
 			log.V(3).Info("Mutation pipeline completed successfully", "handlerCount", len(mgp.mutators))

common/pkg/capi/clustertopology/handlers/mutation/meta_test.go

@@ -34,7 +34,7 @@ var _ MetaMutator = &testHandler{}
 func (h *testHandler) Mutate(
 	_ context.Context,
 	obj *unstructured.Unstructured,
-	_ map[string]apiextensionsv1.JSON,
+	vars map[string]apiextensionsv1.JSON,
 	holderRef runtimehooksv1.HolderReference,
 	_ client.ObjectKey,
 	_ ClusterGetter,
@@ -43,6 +43,31 @@ func (h *testHandler) Mutate(
 		return fmt.Errorf("This is a failure")
 	}
 
+	varAVal, ok := vars["varA"]
+	if !ok {
+		return fmt.Errorf("varA not found in vars")
+	}
+	if string(varAVal.Raw) != `{"a":1,"b":2}` {
+		return fmt.Errorf("varA value mismatch: %s", string(varAVal.Raw))
+	}
+
+	varBVal, ok := vars["varB"]
+	if !ok {
+		return fmt.Errorf("varB not found in vars")
+	}
+	switch obj.GetKind() {
+	case "KubeadmControlPlaneTemplate":
+		if string(varBVal.Raw) != `{"c":3,"d":{"e":4,"f":5}}` {
+			return fmt.Errorf("varB value mismatch: %s", string(varBVal.Raw))
+		}
+	case "KubeadmConfigTemplate":
+		if string(varBVal.Raw) != `{"c":3,"d":{"e":5,"f":5}}` {
+			return fmt.Errorf("varB value mismatch: %s", string(varBVal.Raw))
+		}
+	default:
+		return fmt.Errorf("unexpected object kind: %s", obj.GetKind())
+	}
+
 	if h.mutateControlPlane {
 		return patches.MutateIfApplicable(
 			obj, nil, &holderRef, selectors.ControlPlane(), logr.Discard(),
@@ -61,7 +86,7 @@ func (h *testHandler) Mutate(
 
 	return patches.MutateIfApplicable(
 		obj,
-		machineVars(),
+		vars,
 		&holderRef,
 		selectors.WorkersKubeadmConfigTemplateSelector(),
 		logr.Discard(),
@@ -78,10 +103,32 @@ func (h *testHandler) Mutate(
 	)
 }
 
-func machineVars() map[string]apiextensionsv1.JSON {
-	return map[string]apiextensionsv1.JSON{
-		"builtin": {Raw: []byte(`{"machineDeployment": {"class": "a-worker"}}`)},
-	}
+func globalVars() []runtimehooksv1.Variable {
+	return []runtimehooksv1.Variable{{
+		Name: "varA",
+		Value: apiextensionsv1.JSON{
+			Raw: []byte(`{"a": 1, "b": 2}`),
+		},
+	}, {
+		Name: "varB",
+		Value: apiextensionsv1.JSON{
+			Raw: []byte(`{"c": 3, "d": {"e": 4, "f": 5}}`),
+		},
+	}}
+}
+
+func overrideVars() []runtimehooksv1.Variable {
+	return []runtimehooksv1.Variable{{
+		Name: "builtin",
+		Value: apiextensionsv1.JSON{
+			Raw: []byte(`{"machineDeployment": {"class": "a-worker"}}`),
+		},
+	}, {
+		Name: "varB",
+		Value: apiextensionsv1.JSON{
+			Raw: []byte(`{"d": {"e": 5}}`),
+		},
+	}}
 }
 
 func TestMetaGeneratePatches(t *testing.T) {
@@ -221,9 +268,12 @@ func TestMetaGeneratePatches(t *testing.T) {
 			h := NewMetaGeneratePatchesHandler("", nil, tt.mutaters...).(GeneratePatches)
 
 			resp := &runtimehooksv1.GeneratePatchesResponse{}
+			kctReq := request.NewKubeadmConfigTemplateRequestItem("kubeadm-config")
+			kctReq.Variables = overrideVars()
 			h.GeneratePatches(context.Background(), &runtimehooksv1.GeneratePatchesRequest{
+				Variables: globalVars(),
 				Items: []runtimehooksv1.GeneratePatchesRequestItem{
-					request.NewKubeadmConfigTemplateRequestItem("kubeadm-config"),
+					kctReq,
 					request.NewKubeadmControlPlaneTemplateRequestItem("kubeadm-control-plane"),
 				},
 			}, resp)

common/pkg/capi/clustertopology/variables/merge.go

@@ -0,0 +1,66 @@
+// Copyright 2023 Nutanix. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package variables
+
+import (
+	"encoding/json"
+	"fmt"
+	"maps"
+
+	"dario.cat/mergo"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+)
+
+// MergeVariableOverridesWithGlobal merges the provided variable overrides with the global variables.
+// It performs a deep merge, ensuring that if a variable exists in both maps, the value from the overrides is used.
+func MergeVariableOverridesWithGlobal(
+	overrideVars, globalVars map[string]apiextensionsv1.JSON,
+) (map[string]apiextensionsv1.JSON, error) {
+	mergedVars := maps.Clone(overrideVars)
+
+	for k, v := range globalVars {
+		// If the value of v is nil, skip it.
+		if v.Raw == nil {
+			continue
+		}
+
+		existingValue, exists := mergedVars[k]
+
+		// If the variable does not exist in the mergedVars or the value is nil, add it and continue.
+		if !exists || existingValue.Raw == nil {
+			mergedVars[k] = v
+			continue
+		}
+
+		// Wrap the value in a temporary key to ensure we can unmarshal to a map.
+		// This is necessary because the values could be scalars.
+		tempValJSON := fmt.Sprintf(`{"value": %s}`, string(existingValue.Raw))
+		tempGlobalValJSON := fmt.Sprintf(`{"value": %s}`, string(v.Raw))
+
+		// Unmarshal the existing value and the global value into maps.
+		var val, globalVal map[string]interface{}
+		if err := json.Unmarshal([]byte(tempValJSON), &val); err != nil {
+			return nil, fmt.Errorf("failed to unmarshal existing value for key %q: %w", k, err)
+		}
+
+		if err := json.Unmarshal([]byte(tempGlobalValJSON), &globalVal); err != nil {
+			return nil, fmt.Errorf("failed to unmarshal global value for key %q: %w", k, err)
+		}
+
+		// Now use mergo to perform a deep merge of the values, retaining the values in `val` if present.
+		if err := mergo.Merge(&val, globalVal); err != nil {
+			return nil, fmt.Errorf("failed to merge values for key %q: %w", k, err)
+		}
+
+		// Marshal the merged value back to JSON.
+		mergedVal, err := json.Marshal(val["value"])
+		if err != nil {
+			return nil, fmt.Errorf("failed to marshal merged value for key %q: %w", k, err)
+		}
+
+		mergedVars[k] = apiextensionsv1.JSON{Raw: mergedVal}
+	}
+
+	return mergedVars, nil
+}