Fix validation logic to use yaml parsing instead of json

kyaml/setters2/set.go

@@ -4,13 +4,14 @@
 package setters2
 
 import (
-	"encoding/json"
 	"fmt"
 	"strings"
+	"text/template"
 
 	"github.com/go-openapi/spec"
 	"github.com/go-openapi/strfmt"
 	"github.com/go-openapi/validate"
+	goyaml "gopkg.in/yaml.v3"
 	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/fieldmeta"
 	"sigs.k8s.io/kustomize/kyaml/kio"
@@ -241,24 +242,28 @@ func validateAgainstSchema(ext *CliExtension, sch *spec.Schema) error {
 	sc.Properties = map[string]spec.Schema{}
 	sc.Properties[ext.Setter.Name] = *sch
 
-	input := map[string]interface{}{}
-	var inputJSON string
-
+	var inputYAML string
 	if len(ext.Setter.ListValues) > 0 {
-		format := `{"%s" : [%s]}`
-		inputJSON = fmt.Sprintf(format, ext.Setter.Name, JoinCompositeStrings(ext.Setter.ListValues))
-	} else {
-		format := `{"%s" : "%s"}`
-		if yaml.IsValueNonString(ext.Setter.Value) {
-			format = `{"%s" : %s}`
+		tmpl, err := template.New("validator").
+			Parse(`{{.key}}:{{block "list" .values}}{{"\n"}}{{range .}}{{println "-" .}}{{end}}{{end}}`)
+		if err != nil {
+			return err
 		}
-		inputJSON = fmt.Sprintf(format, ext.Setter.Name, ext.Setter.Value)
+		var builder strings.Builder
+		err = tmpl.Execute(&builder, map[string]interface{}{
+			"key":    ext.Setter.Name,
+			"values": ext.Setter.ListValues,
+		})
+		if err != nil {
+			return err
+		}
+		inputYAML = builder.String()
+	} else {
+		inputYAML = fmt.Sprintf("%s: %s", ext.Setter.Name, ext.Setter.Value)
 	}
 
-	// leverage json.Unmarshal to parse the value type
-	// Ex: `{"setter" : "true"}` parses the value as string whereas
-	// `{"setter" : true}` parses as boolean
-	err := json.Unmarshal([]byte(inputJSON), &input)
+	input := map[string]interface{}{}
+	err := goyaml.Unmarshal([]byte(inputYAML), &input)
 	if err != nil {
 		return err
 	}
@@ -269,21 +274,6 @@ func validateAgainstSchema(ext *CliExtension, sch *spec.Schema) error {
 	return nil
 }
 
-// JoinCompositeStrings takes in strings whose values can be of different types and returns
-// joined string with quotes for only string type values
-// ex: ["10", "true",  "hi", "1.1"] returns 10,true,"hi",1.1
-func JoinCompositeStrings(listValues []string) string {
-	res := ""
-	for _, val := range listValues {
-		if yaml.IsValueNonString(val) {
-			res += fmt.Sprintf(`%s,`, val)
-		} else {
-			res += fmt.Sprintf(`"%s",`, val)
-		}
-	}
-	return strings.TrimSuffix(res, ",")
-}
-
 // SetOpenAPI updates a setter value
 type SetOpenAPI struct {
 	// Name is the name of the setter to add

kyaml/setters2/set_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/go-openapi/spec"
 	"github.com/stretchr/testify/assert"
 	"sigs.k8s.io/kustomize/kyaml/openapi"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
@@ -1294,7 +1295,130 @@ openAPI:
 	}
 }
 
-func TestJoinCompositeStrings(t *testing.T) {
-	input := []string{"10", "true", "hi", "1.1", "1.8.1"}
-	assert.Equal(t, `10,true,"hi",1.1,"1.8.1"`, JoinCompositeStrings(input))
+func TestValidateAgainstSchema(t *testing.T) {
+	testCases := []struct {
+		name             string
+		setter           *setter
+		schema           spec.SchemaProps
+		shouldValidate   bool
+		expectedErrorMsg string
+	}{
+		{
+			name: "simple string value",
+			setter: &setter{
+				Name:  "foo",
+				Value: "bar",
+			},
+			schema: spec.SchemaProps{
+				Type: []string{"string"},
+			},
+			shouldValidate: true,
+		},
+		{
+			name: "simple bool value",
+			setter: &setter{
+				Name:  "foo",
+				Value: "false",
+			},
+			schema: spec.SchemaProps{
+				Type: []string{"boolean"},
+			},
+			shouldValidate: true,
+		},
+		{
+			name: "simple null value",
+			setter: &setter{
+				Name:  "foo",
+				Value: "null",
+			},
+			schema: spec.SchemaProps{
+				Type: []string{"null"},
+			},
+			shouldValidate: true,
+		},
+		{
+			name: "bool value in yaml but not openapi",
+			setter: &setter{
+				Name:  "foo",
+				Value: "yes",
+			},
+			schema: spec.SchemaProps{
+				Type: []string{"string"},
+			},
+			shouldValidate: true,
+		},
+		{
+			name: "number value should not be accepted as string",
+			setter: &setter{
+				Name:  "foo",
+				Value: "45",
+			},
+			schema: spec.SchemaProps{
+				Type: []string{"integer"},
+			},
+			shouldValidate: true,
+		},
+		{
+			name: "list with int values",
+			setter: &setter{
+				Name:       "foo",
+				ListValues: []string{"123", "456"},
+			},
+			schema: spec.SchemaProps{
+				Type: []string{"array"},
+				Items: &spec.SchemaOrArray{
+					Schema: &spec.Schema{
+						SchemaProps: spec.SchemaProps{
+							Type: []string{"integer"},
+						},
+					},
+				},
+			},
+			shouldValidate: true,
+		},
+		{
+			name: "list expecting int values, but with a string",
+			setter: &setter{
+				Name:       "foo",
+				ListValues: []string{"123", "456", "abc"},
+			},
+			schema: spec.SchemaProps{
+				Type: []string{"array"},
+				Items: &spec.SchemaOrArray{
+					Schema: &spec.Schema{
+						SchemaProps: spec.SchemaProps{
+							Type: []string{"integer"},
+						},
+					},
+				},
+			},
+			shouldValidate:   false,
+			expectedErrorMsg: "foo in body must be of type integer",
+		},
+	}
+
+	for i := range testCases {
+		test := testCases[i]
+		t.Run(test.name, func(t *testing.T) {
+			ext := &CliExtension{
+				Setter: test.setter,
+			}
+
+			schema := &spec.Schema{
+				SchemaProps: test.schema,
+			}
+
+			err := validateAgainstSchema(ext, schema)
+
+			if test.shouldValidate {
+				assert.NoError(t, err)
+				return
+			}
+
+			if !assert.Error(t, err) {
+				t.FailNow()
+			}
+			assert.Contains(t, err.Error(), test.expectedErrorMsg)
+		})
+	}
 }