Merge pull request #5772 from sbueringer/pr-more-json-validation

🌱 ClusterClass: validate default and enum JSON, extend schema conversion test
kubernetes-sigs · Dec 2, 2021 · 29c902c · 29c902c
2 parents 6b897de + fa308e8
commit 29c902c
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 2 deletions.
diff --git a/internal/topology/variables/cluster_variable_validation.go b/internal/topology/variables/cluster_variable_validation.go
@@ -97,7 +97,7 @@ func validateClusterVariable(clusterVariable *clusterv1.ClusterVariable, cluster
 	// Note: A clusterVariable with a nil value is the result of setting the variable value to "null" via YAML.
 	if clusterVariable.Value.Raw != nil {
 		if err := json.Unmarshal(clusterVariable.Value.Raw, &variableValue); err != nil {
-			return field.ErrorList{field.Invalid(fldPath.Child("value", "raw"), string(clusterVariable.Value.Raw),
+			return field.ErrorList{field.Invalid(fldPath.Child("value"), string(clusterVariable.Value.Raw),
 				fmt.Sprintf("variable %q could not be parsed: %v", clusterVariable.Name, err))}
 		}
 	}

diff --git a/internal/topology/variables/clusterclass_variable_validation.go b/internal/topology/variables/clusterclass_variable_validation.go
@@ -17,6 +17,7 @@ limitations under the License.
 package variables
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -103,6 +104,24 @@ var validVariableTypes = sets.NewString("string", "number", "integer", "boolean"
 
 // validateSchema validates the schema.
 func validateSchema(schema *clusterv1.JSONSchemaProps, variableName string, fldPath *field.Path) field.ErrorList {
+	// Validate the JSON's in the schema
+	// NOTE: This is done here to provide a user-friendly error instead of an error which would occur during conversion.
+	var allErrs field.ErrorList
+	if schema.Default != nil && schema.Default.Raw != nil {
+		if err := json.Unmarshal(schema.Default.Raw, &struct{}{}); err != nil {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("default"), string(schema.Default.Raw),
+				"openAPIV3Schema.default is not valid JSON"))
+		}
+	}
+	for i, enum := range schema.Enum {
+		if enum.Raw != nil {
+			if err := json.Unmarshal(enum.Raw, &struct{}{}); err != nil {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("enum").Index(i), string(enum.Raw),
+					"openAPIV3Schema.enum value is not valid JSON"))
+			}
+		}
+	}
+
 	apiExtensionsSchema, err := convertToAPIExtensionsJSONSchemaProps(schema)
 	if err != nil {
 		return field.ErrorList{field.Invalid(fldPath, schema,

diff --git a/internal/topology/variables/clusterclass_variable_validation_test.go b/internal/topology/variables/clusterclass_variable_validation_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/utils/pointer"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -208,6 +209,90 @@ func Test_ValidateClusterClassVariable(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "Valid default value regular string",
+			clusterClassVariable: &clusterv1.ClusterClassVariable{
+				Name:     "var",
+				Required: true,
+				Schema: clusterv1.VariableSchema{
+					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+						Type:     "string",
+						Nullable: true,
+						Default: &apiextensionsv1.JSON{
+							Raw: []byte(`"defaultValue"`),
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "Valid default value null",
+			clusterClassVariable: &clusterv1.ClusterClassVariable{
+				Name:     "var",
+				Required: true,
+				Schema: clusterv1.VariableSchema{
+					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+						Type:     "string",
+						Nullable: true,
+						Default: &apiextensionsv1.JSON{
+							// A JSON with a nil value is the result of setting the variable value to "null" via YAML.
+							Raw: nil,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "fail on default value with invalid JSON",
+			clusterClassVariable: &clusterv1.ClusterClassVariable{
+				Name:     "var",
+				Required: true,
+				Schema: clusterv1.VariableSchema{
+					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+						Type:     "string",
+						Nullable: true,
+						Default: &apiextensionsv1.JSON{
+							Raw: []byte(`"defaultValue": "value"`), // invalid JSON
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Valid enum values",
+			clusterClassVariable: &clusterv1.ClusterClassVariable{
+				Name:     "var",
+				Required: true,
+				Schema: clusterv1.VariableSchema{
+					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+						Type:     "string",
+						Nullable: true,
+						Enum: []apiextensionsv1.JSON{
+							{Raw: []byte(`"enumValue1"`)},
+							{Raw: []byte(`"enumValue2"`)},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "fail on enum value with invalid JSON",
+			clusterClassVariable: &clusterv1.ClusterClassVariable{
+				Name:     "var",
+				Required: true,
+				Schema: clusterv1.VariableSchema{
+					OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+						Type:     "string",
+						Nullable: true,
+						Enum: []apiextensionsv1.JSON{
+							{Raw: []byte(`"defaultValue": "value"`)}, // invalid JSON
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "fail on variable type is null",
 			clusterClassVariable: &clusterv1.ClusterClassVariable{

diff --git a/internal/topology/variables/schema_test.go b/internal/topology/variables/schema_test.go
@@ -20,7 +20,9 @@ import (
 	"reflect"
 	"testing"
 
+	. "github.com/onsi/gomega"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/utils/pointer"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
@@ -86,8 +88,27 @@ func Test_convertToAPIExtensionsJSONSchemaProps(t *testing.T) {
 			}
 		})
 	}
-}
 
+	t.Run("pass for schema with default and enum", func(t *testing.T) {
+		g := NewWithT(t)
+
+		schema := &clusterv1.JSONSchemaProps{
+			Default: &apiextensionsv1.JSON{
+				Raw: []byte(`"defaultValue"`),
+			},
+			Enum: []apiextensionsv1.JSON{
+				{Raw: []byte(`"enumValue1"`)},
+				{Raw: []byte(`"enumValue2"`)},
+			},
+		}
+
+		got, err := convertToAPIExtensionsJSONSchemaProps(schema)
+		g.Expect(err).ToNot(HaveOccurred())
+
+		g.Expect(*got.Default).To(Equal(apiextensions.JSON(`defaultValue`)))
+		g.Expect(got.Enum).To(Equal([]apiextensions.JSON{`enumValue1`, `enumValue2`}))
+	})
+}
 func convertIntToFloatPointer(i int64) *float64 {
 	f := float64(i)
 	return &f