Merge pull request #7243 from sbueringer/pr-relax-md-var-validation

🌱 ClusterClass: relax validation of MD variables overrides of opt variables
kubernetes-sigs · Sep 27, 2022 · 1b7490a · 1b7490a
2 parents 80c4450 + ebf2161
commit 1b7490a
Show file tree

Hide file tree

Showing 4 changed files with 2 additions and 151 deletions.
diff --git a/internal/topology/variables/cluster_variable_validation.go b/internal/topology/variables/cluster_variable_validation.go
@@ -102,23 +102,6 @@ func validateClusterVariablesDefined(clusterVariables []clusterv1.ClusterVariabl
 	return allErrs
 }
 
-// ValidateTopLevelClusterVariablesExist validates that all overrides have a corresponding top-level variable.
-func ValidateTopLevelClusterVariablesExist(clusterVariablesOverrides []clusterv1.ClusterVariable, clusterVariables []clusterv1.ClusterVariable, fldPath *field.Path) field.ErrorList {
-	var allErrs field.ErrorList
-
-	// Build map for easier and faster access.
-	clusterVariablesMap := getClusterVariablesMap(clusterVariables)
-
-	for i, clusterVariableOverride := range clusterVariablesOverrides {
-		if _, ok := clusterVariablesMap[clusterVariableOverride.Name]; !ok {
-			return field.ErrorList{field.Invalid(fldPath.Index(i).Child("name"), clusterVariableOverride.Name,
-				fmt.Sprintf("variable override with name %q is missing a corresponding top-level variable", clusterVariableOverride.Name))}
-		}
-	}
-
-	return allErrs
-}
-
 // ValidateClusterVariable validates a clusterVariable.
 func ValidateClusterVariable(clusterVariable *clusterv1.ClusterVariable, clusterClassVariable *clusterv1.ClusterClassVariable, fldPath *field.Path) field.ErrorList {
 	// Parse JSON value.

diff --git a/internal/topology/variables/cluster_variable_validation_test.go b/internal/topology/variables/cluster_variable_validation_test.go
@@ -236,135 +236,6 @@ func Test_ValidateClusterVariables(t *testing.T) {
 	}
 }
 
-func Test_ValidateTopLevelClusterVariablesExist(t *testing.T) {
-	tests := []struct {
-		name                      string
-		clusterVariablesOverrides []clusterv1.ClusterVariable
-		clusterVariables          []clusterv1.ClusterVariable
-		wantErr                   bool
-	}{
-		{
-			name: "Pass if all variable overrides have corresponding top-level variables.",
-			clusterVariablesOverrides: []clusterv1.ClusterVariable{
-				{
-					Name: "cpu",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`1`),
-					},
-				},
-				{
-					Name: "zone",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`"longerThanOneCharacter"`),
-					},
-				},
-			},
-			clusterVariables: []clusterv1.ClusterVariable{
-				{
-					Name: "cpu",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`1`),
-					},
-				},
-				{
-					Name: "zone",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`"longerThanOneCharacter"`),
-					},
-				},
-			},
-		},
-		{
-			name:                      "Pass if there are no variable overrides and no top-level variables.",
-			clusterVariablesOverrides: []clusterv1.ClusterVariable{},
-			clusterVariables:          []clusterv1.ClusterVariable{},
-		},
-		{
-			name:                      "Pass if there are no variable overrides.",
-			clusterVariablesOverrides: []clusterv1.ClusterVariable{},
-			clusterVariables: []clusterv1.ClusterVariable{
-				{
-					Name: "cpu",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`1`),
-					},
-				},
-				{
-					Name: "zone",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`"longerThanOneCharacter"`),
-					},
-				},
-			},
-		},
-		{
-			name: "Error if a variable override is missing the corresponding top-level variables.",
-			clusterVariablesOverrides: []clusterv1.ClusterVariable{
-				{
-					Name: "cpu",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`1`),
-					},
-				},
-				{
-					Name: "zone",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`"longerThanOneCharacter"`),
-					},
-				},
-			},
-			clusterVariables: []clusterv1.ClusterVariable{
-				{
-					Name: "zone",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`"longerThanOneCharacter"`),
-					},
-				},
-			},
-			wantErr: true,
-		},
-		{
-			name: "Pass if not every top-level variable has an override.",
-			clusterVariablesOverrides: []clusterv1.ClusterVariable{
-				{
-					Name: "zone",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`"longerThanOneCharacter"`),
-					},
-				},
-			},
-			clusterVariables: []clusterv1.ClusterVariable{
-				{
-					Name: "cpu",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`1`),
-					},
-				},
-				{
-					Name: "zone",
-					Value: apiextensionsv1.JSON{
-						Raw: []byte(`"longerThanOneCharacter"`),
-					},
-				},
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			g := NewWithT(t)
-
-			errList := ValidateTopLevelClusterVariablesExist(tt.clusterVariablesOverrides, tt.clusterVariables,
-				field.NewPath("spec", "topology", "workers", "machineDeployments").Index(0).Child("variables", "overrides"))
-
-			if tt.wantErr {
-				g.Expect(errList).NotTo(BeEmpty())
-				return
-			}
-			g.Expect(errList).To(BeEmpty())
-		})
-	}
-}
-
 func Test_ValidateClusterVariable(t *testing.T) {
 	tests := []struct {
 		name                 string

diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go
@@ -259,9 +259,6 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu
 				continue
 			}
 
-			allErrs = append(allErrs, variables.ValidateTopLevelClusterVariablesExist(md.Variables.Overrides, newCluster.Spec.Topology.Variables,
-				fldPath.Child("workers", "machineDeployments").Index(i).Child("variables", "overrides"))...)
-
 			allErrs = append(allErrs, variables.ValidateMachineDeploymentVariables(md.Variables.Overrides, clusterClass.Spec.Variables,
 				fldPath.Child("workers", "machineDeployments").Index(i).Child("variables", "overrides"))...)
 		}

diff --git a/internal/webhooks/cluster_test.go b/internal/webhooks/cluster_test.go
@@ -757,7 +757,7 @@ func TestClusterTopologyValidation(t *testing.T) {
 			expectErr: true,
 		},
 		{
-			name: "should fail when variable override is missing the corresponding top-level variable",
+			name: "should pass even when variable override is missing the corresponding top-level variable",
 			clusterClassVariables: []clusterv1.ClusterClassVariable{
 				{
 					Name:     "cpu",
@@ -782,7 +782,7 @@ func TestClusterTopologyValidation(t *testing.T) {
 						Build()).
 					Build()).
 				Build(),
-			expectErr: true,
+			expectErr: false,
 		},
 	}