Some more improvements (#4)

sbueringer · Jun 24, 2024 · 8834caa · 8834caa
1 parent c47d989
commit 8834caa
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 44 deletions.
diff --git a/internal/topology/variables/cluster_variable_validation.go b/internal/topology/variables/cluster_variable_validation.go
@@ -251,7 +251,7 @@ func ValidateClusterVariable(ctx context.Context, value, oldValue *clusterv1.Clu
 			// Set correct value in the field error. ValidateCustomResource sets the type instead of the value.
 			validationError.BadValue = string(value.Value.Raw)
 			// Drop "variableValue" from the path.
-			validationError.Field = strings.TrimSuffix(validationError.Field, ".variableValue")
+			validationError.Field = strings.Replace(validationError.Field, "value.variableValue", "value", 1)
 			allErrs = append(allErrs, validationError)
 		}
 		return allErrs

diff --git a/internal/topology/variables/cluster_variable_validation_test.go b/internal/topology/variables/cluster_variable_validation_test.go
@@ -37,6 +37,7 @@ func Test_ValidateClusterVariables(t *testing.T) {
 		validateRequired bool
 		wantErrs         []validationMatch
 	}{
+		// Basic cases
 		{
 			name: "Pass for a number of valid values.",
 			definitions: []clusterv1.ClusterClassStatusVariable{
@@ -670,6 +671,7 @@ func Test_ValidateClusterVariable(t *testing.T) {
 		clusterVariable      *clusterv1.ClusterVariable
 		wantErrs             []validationMatch
 	}{
+		// Scalars
 		{
 			name: "Valid integer",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -735,7 +737,6 @@ func Test_ValidateClusterVariable(t *testing.T) {
 				},
 			},
 		},
-
 		{
 			name: "Fails, expected integer got string",
 			wantErrs: []validationMatch{
@@ -922,6 +923,7 @@ func Test_ValidateClusterVariable(t *testing.T) {
 				},
 			},
 		},
+		// Objects
 		{
 			name: "Valid object",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -1180,6 +1182,7 @@ func Test_ValidateClusterVariable(t *testing.T) {
 				},
 			},
 		},
+		// Maps
 		{
 			name: "Valid map",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -1272,6 +1275,7 @@ func Test_ValidateClusterVariable(t *testing.T) {
 				},
 			},
 		},
+		// Arrays
 		{
 			name: "Valid array",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -1458,6 +1462,7 @@ func Test_ValidateClusterVariable(t *testing.T) {
 				},
 			},
 		},
+		// x-kubernetes-preserve-unknown-fields
 		{
 			name: "Valid object with x-kubernetes-preserve-unknown-fields",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -1687,7 +1692,7 @@ func Test_ValidateClusterVariable(t *testing.T) {
 				},
 			},
 		},
-
+		// CEL
 		{
 			name: "Valid CEL expression: scalar: using self",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -1984,6 +1989,43 @@ func Test_ValidateClusterVariable(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Invalid CEL expression: objects (nested)",
+			wantErrs: []validationMatch{
+				invalid("Invalid value: \"{\\\"objectField\\\": {\\\"field\\\": 2}}\": failed rule: self.field <= 1",
+					"spec.topology.variables[cpu].value.objectField"),
+			},
+			clusterClassVariable: &clusterv1.ClusterClassVariable{
+				Name:     "cpu",
+				Required: true,
+				Schema: clusterv1.VariableSchema{
+					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+						Type:                   "object",
+						XPreserveUnknownFields: true,
+						Properties: map[string]clusterv1.JSONSchemaProps{
+							"objectField": {
+								Type:                   "object",
+								XPreserveUnknownFields: true,
+								XValidations: []clusterv1.ValidationRule{{
+									Rule: "self.field <= 1",
+								}},
+								Properties: map[string]clusterv1.JSONSchemaProps{
+									"field": {
+										Type: "integer",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			clusterVariable: &clusterv1.ClusterVariable{
+				Name: "cpu",
+				Value: apiextensionsv1.JSON{
+					Raw: []byte(`{"objectField": {"field": 2}}`),
+				},
+			},
+		},
 		{
 			name: "Valid CEL expression: objects: defined field can be accessed",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{
@@ -2263,6 +2305,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
 		oldValues   []clusterv1.ClusterVariable
 		wantErrs    []validationMatch
 	}{
+		// Basic cases
 		{
 			name: "Pass when no value for required definition (required variables are not required for overrides)",
 			definitions: []clusterv1.ClusterClassStatusVariable{
@@ -2405,6 +2448,7 @@ func Test_ValidateMachineVariables(t *testing.T) {
 				},
 			},
 		},
+		// CEL
 		{
 			name: "Valid integer with CEL expression",
 			definitions: []clusterv1.ClusterClassStatusVariable{

diff --git a/internal/topology/variables/clusterclass_variable_validation.go b/internal/topology/variables/clusterclass_variable_validation.go
@@ -250,11 +250,11 @@ func validateRootSchema(ctx context.Context, oldClusterClassVariables, clusterCl
 	if celContext != nil && celContext.TotalCost != nil {
 		if celContext.TotalCost.Total > StaticEstimatedCRDCostLimit {
 			for _, expensive := range celContext.TotalCost.MostExpensive {
-				costErrorMsg := "contributed to estimated cost total exceeding cost limit for entire OpenAPIv3 schema"
+				costErrorMsg := "contributed to estimated rule & messageExpression cost total exceeding cost limit for entire OpenAPIv3 schema"
 				allErrs = append(allErrs, field.Forbidden(expensive.Path, costErrorMsg))
 			}
 
-			costErrorMsg := getCostErrorMessage("x-kubernetes-validations estimated cost total for entire OpenAPIv3 schema", celContext.TotalCost.Total, StaticEstimatedCRDCostLimit)
+			costErrorMsg := getCostErrorMessage("x-kubernetes-validations estimated rule & messageExpression cost total for entire OpenAPIv3 schema", celContext.TotalCost.Total, StaticEstimatedCRDCostLimit)
 			allErrs = append(allErrs, field.Forbidden(fldPath, costErrorMsg))
 		}
 	}