Add filter to associate variables with specific patches

internal/controllers/topology/cluster/patches/engine.go

@@ -26,6 +26,7 @@ import (
 	jsonpatch "github.com/evanphx/json-patch/v5"
 	"github.com/pkg/errors"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/klog/v2"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	runtimehooksv1 "sigs.k8s.io/cluster-api/exp/runtime/hooks/api/v1alpha1"
@@ -83,6 +84,14 @@ func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, d
 		clusterClassPatch := blueprint.ClusterClass.Spec.Patches[i]
 		ctx, log := log.WithValues("patch", clusterClassPatch.Name).Into(ctx)
 
+		definitionFrom := clusterClassPatch.Name
+		// If this isn't an external patch, use the inline patch name.
+		if clusterClassPatch.External == nil {
+			definitionFrom = clusterv1.VariableDefinitionFromInline
+		}
+		if err := addVariablesForPatch(blueprint, desired, req, definitionFrom); err != nil {
+			return errors.Wrapf(err, "failed to calculate variables for patch %q", clusterClassPatch.Name)
+		}
 		log.V(5).Infof("Applying patch to templates")
 
 		// Create patch generator for the current patch.
@@ -139,23 +148,85 @@ func (e *engine) Apply(ctx context.Context, blueprint *scope.ClusterBlueprint, d
 	return nil
 }
 
+// addVariablesForPatch adds variables for a given ClusterClassPatch to the items in the PatchRequest.
+func addVariablesForPatch(blueprint *scope.ClusterBlueprint, desired *scope.ClusterState, req *runtimehooksv1.GeneratePatchesRequest, definitionFrom string) error {
+	patchVariableDefinitions := definitionsForPatch(blueprint, definitionFrom)
+	// If there is no definitionFrom return an error.
+	if definitionFrom == "" {
+		return errors.New("failed to calculate variables: no patch name provided")
+	}
+	// Calculate global variables.
+	globalVariables, err := variables.Global(blueprint.Topology, desired.Cluster, definitionFrom, patchVariableDefinitions)
+	if err != nil {
+		return errors.Wrapf(err, "failed to calculate global variables")
+	}
+	req.Variables = globalVariables
+
+	// Calculate the Control Plane variables.
+	controlPlaneVariables, err := variables.ControlPlane(&blueprint.Topology.ControlPlane, desired.ControlPlane.Object, desired.ControlPlane.InfrastructureMachineTemplate)
+	if err != nil {
+		return errors.Wrapf(err, "failed to calculate ControlPlane variables")
+	}
+
+	mdStateIndex := map[string]*scope.MachineDeploymentState{}
+	for _, md := range desired.MachineDeployments {
+		mdStateIndex[md.Object.Name] = md
+	}
+	for i, item := range req.Items {
+		// If the item is a Control Plane add the Control Plane variables.
+		if item.HolderReference.FieldPath == "spec.controlPlaneRef" {
+			item.Variables = controlPlaneVariables
+		}
+		// If the item holder reference is a Control Plane machine add the Control Plane variables.
+		if blueprint.HasControlPlaneInfrastructureMachine() &&
+			item.HolderReference.FieldPath == strings.Join(contract.ControlPlane().MachineTemplate().InfrastructureRef().Path(), ".") {
+			item.Variables = controlPlaneVariables
+		}
+		// If the item holder reference is a MachineDeployment calculate the variables for each MachineDeploymentTopology
+		// and add them to the variables for the MachineDeployment.
+		if item.HolderReference.Kind == "MachineDeployment" {
+			md, ok := mdStateIndex[item.HolderReference.Name]
+			if !ok {
+				return errors.Errorf("could not find desired state for MachineDeployment %s", klog.KObj(md.Object))
+			}
+			mdTopology, err := getMDTopologyFromMD(blueprint, md.Object)
+			if err != nil {
+				return err
+			}
+
+			// Calculate MachineDeployment variables.
+			mdVariables, err := variables.MachineDeployment(mdTopology, md.Object, md.BootstrapTemplate, md.InfrastructureMachineTemplate, definitionFrom, patchVariableDefinitions)
+			if err != nil {
+				return errors.Wrapf(err, "failed to calculate variables for %s", klog.KObj(md.Object))
+			}
+			item.Variables = mdVariables
+		}
+		req.Items[i] = item
+	}
+	return nil
+}
+
+func getMDTopologyFromMD(blueprint *scope.ClusterBlueprint, md *clusterv1.MachineDeployment) (*clusterv1.MachineDeploymentTopology, error) {
+	topologyName, ok := md.Labels[clusterv1.ClusterTopologyMachineDeploymentNameLabel]
+	if !ok {
+		return nil, errors.Errorf("failed to get topology name for %s", klog.KObj(md))
+	}
+	mdTopology, err := lookupMDTopology(blueprint.Topology, topologyName)
+	if err != nil {
+		return nil, err
+	}
+	return mdTopology, nil
+}
+
 // createRequest creates a GeneratePatchesRequest based on the ClusterBlueprint and the desired state.
-// ClusterBlueprint supplies the templates. Desired state is used to calculate variables which are later used
-// as input for the patch generation.
 // NOTE: GenerateRequestTemplates are created for the templates of each individual MachineDeployment in the desired
 // state. This is necessary because some builtin variables are MachineDeployment specific. For example version and
 // replicas of a MachineDeployment.
 // NOTE: A single GeneratePatchesRequest object is used to carry templates state across subsequent Generate calls.
+// NOTE: This function does not add variables to items for the request, as the variables depend on the specific patch.
 func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterState) (*runtimehooksv1.GeneratePatchesRequest, error) {
 	req := &runtimehooksv1.GeneratePatchesRequest{}
 
-	// Calculate global variables.
-	globalVariables, err := variables.Global(blueprint.Topology, desired.Cluster)
-	if err != nil {
-		return nil, errors.Wrapf(err, "failed to calculate global variables")
-	}
-	req.Variables = globalVariables
-
 	// Add the InfrastructureClusterTemplate.
 	t, err := newRequestItemBuilder(blueprint.InfrastructureClusterTemplate).
 		WithHolder(desired.Cluster, "spec.infrastructureRef").
@@ -166,12 +237,6 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 	}
 	req.Items = append(req.Items, *t)
 
-	// Calculate controlPlane variables.
-	controlPlaneVariables, err := variables.ControlPlane(&blueprint.Topology.ControlPlane, desired.ControlPlane.Object, desired.ControlPlane.InfrastructureMachineTemplate)
-	if err != nil {
-		return nil, errors.Wrapf(err, "failed to calculate ControlPlane variables")
-	}
-
 	// Add the ControlPlaneTemplate.
 	t, err = newRequestItemBuilder(blueprint.ControlPlane.Template).
 		WithHolder(desired.Cluster, "spec.controlPlaneRef").
@@ -180,7 +245,6 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 		return nil, errors.Wrapf(err, "failed to prepare ControlPlane template %s for patching",
 			tlog.KObj{Obj: blueprint.ControlPlane.Template})
 	}
-	t.Variables = controlPlaneVariables
 	req.Items = append(req.Items, *t)
 
 	// If the clusterClass mandates the controlPlane has infrastructureMachines,
@@ -193,7 +257,6 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 			return nil, errors.Wrapf(err, "failed to prepare ControlPlane's machine template %s for patching",
 				tlog.KObj{Obj: blueprint.ControlPlane.InfrastructureMachineTemplate})
 		}
-		t.Variables = controlPlaneVariables
 		req.Items = append(req.Items, *t)
 	}
 
@@ -216,12 +279,6 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 			return nil, errors.Errorf("failed to lookup MachineDeployment class %q in ClusterClass", mdTopology.Class)
 		}
 
-		// Calculate MachineDeployment variables.
-		mdVariables, err := variables.MachineDeployment(mdTopology, md.Object, md.BootstrapTemplate, md.InfrastructureMachineTemplate)
-		if err != nil {
-			return nil, errors.Wrapf(err, "failed to calculate variables for %s", tlog.KObj{Obj: md.Object})
-		}
-
 		// Add the BootstrapTemplate.
 		t, err := newRequestItemBuilder(mdClass.BootstrapTemplate).
 			WithHolder(md.Object, "spec.template.spec.bootstrap.configRef").
@@ -230,7 +287,6 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 			return nil, errors.Wrapf(err, "failed to prepare BootstrapConfig template %s for MachineDeployment topology %s for patching",
 				tlog.KObj{Obj: mdClass.BootstrapTemplate}, mdTopologyName)
 		}
-		t.Variables = mdVariables
 		req.Items = append(req.Items, *t)
 
 		// Add the InfrastructureMachineTemplate.
@@ -241,7 +297,6 @@ func createRequest(blueprint *scope.ClusterBlueprint, desired *scope.ClusterStat
 			return nil, errors.Wrapf(err, "failed to prepare InfrastructureMachine template %s for MachineDeployment topology %s for patching",
 				tlog.KObj{Obj: mdClass.InfrastructureMachineTemplate}, mdTopologyName)
 		}
-		t.Variables = mdVariables
 		req.Items = append(req.Items, *t)
 	}
 
@@ -431,3 +486,16 @@ func updateDesiredState(ctx context.Context, req *runtimehooksv1.GeneratePatches
 
 	return nil
 }
+
+// definitionsForPatch returns a set which of variables in a ClusterClass defined by "definitionFrom".
+func definitionsForPatch(blueprint *scope.ClusterBlueprint, definitionFrom string) map[string]bool {
+	variableDefinitionsForPatch := make(map[string]bool)
+	for _, definitionsWithName := range blueprint.ClusterClass.Status.Variables {
+		for _, definition := range definitionsWithName.Definitions {
+			if definition.From == definitionFrom {
+				variableDefinitionsForPatch[definitionsWithName.Name] = true
+			}
+		}
+	}
+	return variableDefinitionsForPatch
+}

internal/controllers/topology/cluster/patches/engine_test.go

@@ -697,6 +697,7 @@ func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) {
 		MachineDeployments: map[string]*scope.MachineDeploymentState{
 			"default-worker-topo1": {
 				Object: builder.MachineDeployment(metav1.NamespaceDefault, "md1").
+					WithLabels(map[string]string{clusterv1.ClusterTopologyMachineDeploymentNameLabel: "default-worker-topo1"}).
 					WithVersion("v1.21.2").
 					Build(),
 				// Make sure we're using an independent instance of the template.
@@ -705,6 +706,7 @@ func setupTestObjects() (*scope.ClusterBlueprint, *scope.ClusterState) {
 			},
 			"default-worker-topo2": {
 				Object: builder.MachineDeployment(metav1.NamespaceDefault, "md2").
+					WithLabels(map[string]string{clusterv1.ClusterTopologyMachineDeploymentNameLabel: "default-worker-topo2"}).
 					WithVersion("v1.20.6").
 					WithReplicas(5).
 					Build(),

internal/controllers/topology/cluster/patches/variables/variables.go

@@ -33,6 +33,8 @@ import (
 const (
 	// BuiltinsName is the name of the builtin variable.
 	BuiltinsName = "builtin"
+	// noConflictsDefinitionFrom is the value of the variable DefinitionFrom field when a variable definition no conflicts.
+	noConflictsDefinitionFrom = ""
 )
 
 // Builtins represents builtin variables exposed through patches.
@@ -171,7 +173,7 @@ type MachineDeploymentInfrastructureRefBuiltins struct {
 
 // Global returns variables that apply to all the templates, including user provided variables
 // and builtin variables for the Cluster object.
-func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster) ([]runtimehooksv1.Variable, error) {
+func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster, definitionFrom string, patchVariableDefinitions map[string]bool) ([]runtimehooksv1.Variable, error) {
 	variables := []runtimehooksv1.Variable{}
 
 	// Add user defined variables from Cluster.spec.topology.variables.
@@ -180,11 +182,13 @@ func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster) ([]
 		if variable.Name == BuiltinsName {
 			continue
 		}
-
-		variables = append(variables, runtimehooksv1.Variable{
-			Name:  variable.Name,
-			Value: variable.Value,
-		})
+		// Add the variable if it is defined for the current patch or it is defined for all the patches.
+		if variable.DefinitionFrom == noConflictsDefinitionFrom || variable.DefinitionFrom == definitionFrom {
+			// Add the variable if it has a definition from this patch in the ClusterClass.
+			if _, ok := patchVariableDefinitions[variable.Name]; ok {
+				variables = append(variables, runtimehooksv1.Variable{Name: variable.Name, Value: variable.Value})
+			}
+		}
 	}
 
 	// Construct builtin variable.
@@ -271,16 +275,19 @@ func ControlPlane(cpTopology *clusterv1.ControlPlaneTopology, cp, cpInfrastructu
 }
 
 // MachineDeployment returns variables that apply to templates belonging to a MachineDeployment.
-func MachineDeployment(mdTopology *clusterv1.MachineDeploymentTopology, md *clusterv1.MachineDeployment, mdBootstrapTemplate, mdInfrastructureMachineTemplate *unstructured.Unstructured) ([]runtimehooksv1.Variable, error) {
+func MachineDeployment(mdTopology *clusterv1.MachineDeploymentTopology, md *clusterv1.MachineDeployment, mdBootstrapTemplate, mdInfrastructureMachineTemplate *unstructured.Unstructured, definitionFrom string, patchVariableDefinitions map[string]bool) ([]runtimehooksv1.Variable, error) {
 	variables := []runtimehooksv1.Variable{}
 
 	// Add variables overrides for the MachineDeployment.
 	if mdTopology.Variables != nil {
 		for _, variable := range mdTopology.Variables.Overrides {
-			variables = append(variables, runtimehooksv1.Variable{
-				Name:  variable.Name,
-				Value: variable.Value,
-			})
+			// Add the variable if it is defined for the current patch or it is defined for all the patches.
+			if variable.DefinitionFrom == noConflictsDefinitionFrom || variable.DefinitionFrom == definitionFrom {
+				// Add the variable if it has a definition from this patch in the ClusterClass.
+				if _, ok := patchVariableDefinitions[variable.Name]; ok {
+					variables = append(variables, runtimehooksv1.Variable{Name: variable.Name, Value: variable.Value})
+				}
+			}
 		}
 	}