Return better error messages for invalid JSON schema types in templates (#661)

## Changes
Adds a function to validate json schema types added by the author. The
default json unmarshaller does not validate that the parsed type matches
the enum defined in `jsonschema.Type`

Includes some other improvements to provide better error messages.

This PR was prompted by usability difficulties reported by @mingyu89
during mlops stack migration.

## Tests
Unit tests

libs/jsonschema/schema.go

@@ -1,5 +1,11 @@
 package jsonschema
 
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+)
+
 // defines schema for a json object
 type Schema struct {
 	// Type of the object
@@ -47,3 +53,34 @@ const (
 	ArrayType   Type = "array"
 	IntegerType Type = "integer"
 )
+
+func (schema *Schema) validate() error {
+	for _, v := range schema.Properties {
+		switch v.Type {
+		case NumberType, BooleanType, StringType, IntegerType:
+			continue
+		case "int", "int32", "int64":
+			return fmt.Errorf("type %s is not a recognized json schema type. Please use \"integer\" instead", v.Type)
+		case "float", "float32", "float64":
+			return fmt.Errorf("type %s is not a recognized json schema type. Please use \"number\" instead", v.Type)
+		case "bool":
+			return fmt.Errorf("type %s is not a recognized json schema type. Please use \"boolean\" instead", v.Type)
+		default:
+			return fmt.Errorf("type %s is not a recognized json schema type", v.Type)
+		}
+	}
+	return nil
+}
+
+func Load(path string) (*Schema, error) {
+	b, err := os.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	schema := &Schema{}
+	err = json.Unmarshal(b, schema)
+	if err != nil {
+		return nil, err
+	}
+	return schema, schema.validate()
+}

libs/jsonschema/schema_test.go

@@ -0,0 +1,44 @@
+package jsonschema
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestJsonSchemaValidate(t *testing.T) {
+	var err error
+	toSchema := func(s string) *Schema {
+		return &Schema{
+			Properties: map[string]*Schema{
+				"foo": {
+					Type: Type(s),
+				},
+			},
+		}
+	}
+
+	err = toSchema("string").validate()
+	assert.NoError(t, err)
+
+	err = toSchema("boolean").validate()
+	assert.NoError(t, err)
+
+	err = toSchema("number").validate()
+	assert.NoError(t, err)
+
+	err = toSchema("integer").validate()
+	assert.NoError(t, err)
+
+	err = toSchema("int").validate()
+	assert.EqualError(t, err, "type int is not a recognized json schema type. Please use \"integer\" instead")
+
+	err = toSchema("float").validate()
+	assert.EqualError(t, err, "type float is not a recognized json schema type. Please use \"number\" instead")
+
+	err = toSchema("bool").validate()
+	assert.EqualError(t, err, "type bool is not a recognized json schema type. Please use \"boolean\" instead")
+
+	err = toSchema("foobar").validate()
+	assert.EqualError(t, err, "type foobar is not a recognized json schema type")
+}

libs/template/config.go

@@ -18,13 +18,11 @@ type config struct {
 
 func newConfig(ctx context.Context, schemaPath string) (*config, error) {
 	// Read config schema
-	schemaBytes, err := os.ReadFile(schemaPath)
+	schema, err := jsonschema.Load(schemaPath)
 	if err != nil {
 		return nil, err
 	}
-	schema := &jsonschema.Schema{}
-	err = json.Unmarshal(schemaBytes, schema)
-	if err != nil {
+	if err := validateSchema(schema); err != nil {
 		return nil, err
 	}
 
@@ -36,6 +34,15 @@ func newConfig(ctx context.Context, schemaPath string) (*config, error) {
 	}, nil
 }
 
+func validateSchema(schema *jsonschema.Schema) error {
+	for _, v := range schema.Properties {
+		if v.Type == jsonschema.ArrayType || v.Type == jsonschema.ObjectType {
+			return fmt.Errorf("property type %s is not supported by bundle templates", v.Type)
+		}
+	}
+	return nil
+}
+
 // Reads json file at path and assigns values from the file
 func (c *config) assignValuesFromFile(path string) error {
 	// Read the config file

libs/template/config_test.go

@@ -161,3 +161,34 @@ func TestTemplateConfigValidateTypeForInvalidType(t *testing.T) {
 	err = c.validate()
 	assert.EqualError(t, err, `incorrect type for int_val. expected type integer, but value is "this-should-be-an-int"`)
 }
+
+func TestTemplateValidateSchema(t *testing.T) {
+	var err error
+	toSchema := func(s string) *jsonschema.Schema {
+		return &jsonschema.Schema{
+			Properties: map[string]*jsonschema.Schema{
+				"foo": {
+					Type: jsonschema.Type(s),
+				},
+			},
+		}
+	}
+
+	err = validateSchema(toSchema("string"))
+	assert.NoError(t, err)
+
+	err = validateSchema(toSchema("boolean"))
+	assert.NoError(t, err)
+
+	err = validateSchema(toSchema("number"))
+	assert.NoError(t, err)
+
+	err = validateSchema(toSchema("integer"))
+	assert.NoError(t, err)
+
+	err = validateSchema(toSchema("object"))
+	assert.EqualError(t, err, "property type object is not supported by bundle templates")
+
+	err = validateSchema(toSchema("array"))
+	assert.EqualError(t, err, "property type array is not supported by bundle templates")
+}

libs/template/utils.go

@@ -66,8 +66,10 @@ func toString(v any, T jsonschema.Type) (string, error) {
 			return "", err
 		}
 		return strconv.FormatInt(intVal, 10), nil
-	default:
+	case jsonschema.ArrayType, jsonschema.ObjectType:
 		return "", fmt.Errorf("cannot format object of type %s as a string. Value of object: %#v", T, v)
+	default:
+		return "", fmt.Errorf("unknown json schema type: %q", T)
 	}
 }
 
@@ -87,8 +89,10 @@ func fromString(s string, T jsonschema.Type) (any, error) {
 		v, err = strconv.ParseFloat(s, 32)
 	case jsonschema.IntegerType:
 		v, err = strconv.ParseInt(s, 10, 64)
-	default:
+	case jsonschema.ArrayType, jsonschema.ObjectType:
 		return "", fmt.Errorf("cannot parse string as object of type %s. Value of string: %q", T, s)
+	default:
+		return "", fmt.Errorf("unknown json schema type: %q", T)
 	}
 
 	// Return more readable error incase of a syntax error

libs/template/utils_test.go

@@ -80,6 +80,9 @@ func TestTemplateToString(t *testing.T) {
 
 	_, err = toString("abc", jsonschema.IntegerType)
 	assert.EqualError(t, err, "cannot convert \"abc\" to an integer")
+
+	_, err = toString("abc", "foobar")
+	assert.EqualError(t, err, "unknown json schema type: \"foobar\"")
 }
 
 func TestTemplateFromString(t *testing.T) {
@@ -112,4 +115,7 @@ func TestTemplateFromString(t *testing.T) {
 
 	_, err = fromString("1.0", jsonschema.IntegerType)
 	assert.EqualError(t, err, "could not parse \"1.0\" as a integer: strconv.ParseInt: parsing \"1.0\": invalid syntax")
+
+	_, err = fromString("1.0", "foobar")
+	assert.EqualError(t, err, "unknown json schema type: \"foobar\"")
 }