review fixes

api/v1beta1/cluster_types.go

@@ -157,7 +157,7 @@ type MachineDeploymentTopology struct {
 
 	// Variables can be used to customize the MachineDeployment through patches.
 	// +optional
-	Variables *ClusterVariablesOverrides `json:"variables,omitempty"`
+	Variables *MachineDeploymentVariables `json:"variables,omitempty"`
 }
 
 // ClusterVariable can be used to customize the Cluster through
@@ -177,14 +177,9 @@ type ClusterVariable struct {
 	Value apiextensionsv1.JSON `json:"value"`
 }
 
-// ClusterVariablesOverrides can be used to override top-level variables.
-// They must comply to the corresponding VariableClasses defined
-// in the ClusterClass.
-type ClusterVariablesOverrides struct {
-	// Overrides can be used to override top-level variables.
-	// They must comply to the corresponding VariableClasses defined
-	// in the ClusterClass.
-	// Only variables which are set top-level can be overridden.
+// MachineDeploymentVariables can be used to provide variables for a specific MachineDeployment.
+type MachineDeploymentVariables struct {
+	// Overrides can be used to override Cluster level variables.
 	// +optional
 	Overrides []ClusterVariable `json:"overrides,omitempty"`
 }

config/crd/bases/cluster.x-k8s.io_clusters.yaml

@@ -973,10 +973,8 @@ spec:
                                 MachineDeployment through patches.
                               properties:
                                 overrides:
-                                  description: Overrides can be used to override top-level
-                                    variables. They must comply to the corresponding
-                                    VariableClasses defined in the ClusterClass. Only
-                                    variables which are set top-level can be overridden.
+                                  description: Overrides can be used to override Cluster
+                                    level variables.
                                   items:
                                     description: ClusterVariable can be used to customize
                                       the Cluster through patches. It must comply

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

@@ -176,7 +176,7 @@ func ControlPlane(controlPlaneTopology *clusterv1.ControlPlaneTopology, controlP
 func MachineDeployment(mdTopology *clusterv1.MachineDeploymentTopology, md *clusterv1.MachineDeployment) (VariableMap, error) {
 	variables := VariableMap{}
 
-	// Add user defined variables from Cluster.spec.topology.variables.
+	// Add variables overrides for the MachineDeployment.
 	if mdTopology.Variables != nil {
 		for _, variable := range mdTopology.Variables.Overrides {
 			variables[variable.Name] = variable.Value

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

@@ -141,7 +141,7 @@ func TestMachineDeployment(t *testing.T) {
 				Replicas: pointer.Int32(3),
 				Name:     "md-topology",
 				Class:    "md-class",
-				Variables: &clusterv1.ClusterVariablesOverrides{
+				Variables: &clusterv1.MachineDeploymentVariables{
 					Overrides: []clusterv1.ClusterVariable{
 						{
 							Name:  "location",
@@ -184,7 +184,7 @@ func TestMachineDeployment(t *testing.T) {
 			mdTopology: &clusterv1.MachineDeploymentTopology{
 				Name:  "md-topology",
 				Class: "md-class",
-				Variables: &clusterv1.ClusterVariablesOverrides{
+				Variables: &clusterv1.MachineDeploymentVariables{
 					Overrides: []clusterv1.ClusterVariable{
 						{
 							Name:  "location",

internal/test/builder/builders.go

@@ -196,7 +196,7 @@ func (m *MachineDeploymentTopologyBuilder) Build() clusterv1.MachineDeploymentTo
 	}
 
 	if len(m.variables) > 0 {
-		md.Variables = &clusterv1.ClusterVariablesOverrides{
+		md.Variables = &clusterv1.MachineDeploymentVariables{
 			Overrides: m.variables,
 		}
 	}

internal/topology/variables/cluster_variable_defaulting.go

@@ -29,9 +29,19 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
-// DefaultClusterVariables defaults variables which do not exist in clusterVariable, if they
-// have a default value in the corresponding schema in clusterClassVariable.
-func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path, createVariables bool) ([]clusterv1.ClusterVariable, field.ErrorList) {
+// DefaultClusterVariables defaults ClusterVariables.
+func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) {
+	return defaultClusterVariables(clusterVariables, clusterClassVariables, true, fldPath)
+}
+
+// DefaultMachineDeploymentVariables defaults MachineDeploymentVariables.
+func DefaultMachineDeploymentVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) {
+	return defaultClusterVariables(clusterVariables, clusterClassVariables, false, fldPath)
+}
+
+// defaultClusterVariables defaults variables.
+// If they do not exist yet, they are created if createVariables is set.
+func defaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, createVariables bool, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) {
 	var allErrs field.ErrorList
 
 	// Build maps for easier and faster access.

internal/topology/variables/cluster_variable_defaulting_test.go

@@ -230,8 +230,8 @@ func Test_DefaultClusterVariables(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			vars, errList := DefaultClusterVariables(tt.clusterVariables, tt.clusterClassVariables,
-				field.NewPath("spec", "topology", "variables"), tt.createVariables)
+			vars, errList := defaultClusterVariables(tt.clusterVariables, tt.clusterClassVariables, tt.createVariables,
+				field.NewPath("spec", "topology", "variables"))
 
 			if tt.wantErr {
 				g.Expect(errList).NotTo(BeEmpty())

internal/topology/variables/cluster_variable_validation.go

@@ -27,8 +27,18 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
-// ValidateClusterVariables validates clusterVariable via the schemas in the corresponding clusterClassVariable.
-func ValidateClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path, validateRequired bool) field.ErrorList {
+// ValidateClusterVariables validates ClusterVariables.
+func ValidateClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) field.ErrorList {
+	return validateClusterVariables(clusterVariables, clusterClassVariables, true, fldPath)
+}
+
+// ValidateMachineDeploymentVariables validates ValidateMachineDeploymentVariables.
+func ValidateMachineDeploymentVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) field.ErrorList {
+	return validateClusterVariables(clusterVariables, clusterClassVariables, false, fldPath)
+}
+
+// validateClusterVariables validates variables via the schemas in the corresponding clusterClassVariable.
+func validateClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, validateRequired bool, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 
 	// Build maps for easier and faster access.

internal/topology/variables/cluster_variable_validation_test.go

@@ -224,8 +224,8 @@ func Test_ValidateClusterVariables(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			errList := ValidateClusterVariables(tt.clusterVariables, tt.clusterClassVariables,
-				field.NewPath("spec", "topology", "variables"), tt.validateRequired)
+			errList := validateClusterVariables(tt.clusterVariables, tt.clusterClassVariables,
+				tt.validateRequired, field.NewPath("spec", "topology", "variables"))
 
 			if tt.wantErr {
 				g.Expect(errList).NotTo(BeEmpty())

internal/webhooks/cluster.go

@@ -88,7 +88,7 @@ func (webhook *Cluster) Default(ctx context.Context, obj runtime.Object) error {
 		var allErrs field.ErrorList
 
 		defaultedVariables, errs := variables.DefaultClusterVariables(cluster.Spec.Topology.Variables, clusterClass.Spec.Variables,
-			field.NewPath("spec", "topology", "variables"), true)
+			field.NewPath("spec", "topology", "variables"))
 		if len(errs) > 0 {
 			allErrs = append(allErrs, errs...)
 		} else {
@@ -102,8 +102,8 @@ func (webhook *Cluster) Default(ctx context.Context, obj runtime.Object) error {
 					continue
 				}
 
-				defaultedVariables, errs := variables.DefaultClusterVariables(md.Variables.Overrides, clusterClass.Spec.Variables,
-					field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"), true)
+				defaultedVariables, errs := variables.DefaultMachineDeploymentVariables(md.Variables.Overrides, clusterClass.Spec.Variables,
+					field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"))
 				if len(errs) > 0 {
 					allErrs = append(allErrs, errs...)
 				} else {
@@ -245,7 +245,7 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 
 	// Check if the variables defined in the ClusterClass are valid.
 	allErrs = append(allErrs, variables.ValidateClusterVariables(newCluster.Spec.Topology.Variables, clusterClass.Spec.Variables,
-		field.NewPath("spec", "topology", "variables"), true)...)
+		field.NewPath("spec", "topology", "variables"))...)
 
 	if newCluster.Spec.Topology.Workers != nil {
 		for i, md := range newCluster.Spec.Topology.Workers.MachineDeployments {
@@ -256,8 +256,8 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 
 			allErrs = append(allErrs, variables.ValidateTopLevelClusterVariablesExist(md.Variables.Overrides, newCluster.Spec.Topology.Variables,
 				field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"))...)
-			allErrs = append(allErrs, variables.ValidateClusterVariables(md.Variables.Overrides, clusterClass.Spec.Variables,
-				field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"), false)...)
+			allErrs = append(allErrs, variables.ValidateMachineDeploymentVariables(md.Variables.Overrides, clusterClass.Spec.Variables,
+				field.NewPath("spec", "topology", "workers", "machineDeployments").Index(i).Child("variables", "overrides"))...)
 		}
 	}
 

internal/webhooks/cluster_test.go

@@ -248,7 +248,7 @@ func TestClusterDefaultVariables(t *testing.T) {
 						{
 							Class: "default-worker",
 							Name:  "md-1",
-							Variables: &clusterv1.ClusterVariablesOverrides{
+							Variables: &clusterv1.MachineDeploymentVariables{
 								Overrides: []clusterv1.ClusterVariable{
 									{
 										Name:  "httpProxy",
@@ -272,7 +272,7 @@ func TestClusterDefaultVariables(t *testing.T) {
 						{
 							Class: "default-worker",
 							Name:  "md-1",
-							Variables: &clusterv1.ClusterVariablesOverrides{
+							Variables: &clusterv1.MachineDeploymentVariables{
 								Overrides: []clusterv1.ClusterVariable{
 									{
 										Name: "httpProxy",