From 2320c1046375da6e60b80db546f46043cb93ae55 Mon Sep 17 00:00:00 2001
From: Phani Teja Marupaka <pmarupaka@google.com>
Date: Fri, 15 May 2020 15:44:45 -0700
Subject: [PATCH] Validate list values against openAPI

---
 cmd/config/internal/commands/cmdset_test.go | 52 +++++++++++++++++++++
 kyaml/setters2/set.go                       | 35 ++++++++++++--
 kyaml/setters2/set_test.go                  |  5 ++
 kyaml/testutil/testutil.go                  |  3 ++
 4 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/cmd/config/internal/commands/cmdset_test.go b/cmd/config/internal/commands/cmdset_test.go
index 8b1c90d74..7bf532263 100644
--- a/cmd/config/internal/commands/cmdset_test.go
+++ b/cmd/config/internal/commands/cmdset_test.go
@@ -403,6 +403,58 @@ spec:
 `,
 			errMsg: "tag in body should be at least 6 chars long",
 		},
+
+		{
+			name: "validate openAPI list values",
+			args: []string{"list", "10", "hi", "true"},
+			inputOpenAPI: `
+kind: Kptfile
+openAPI:
+  definitions:
+    io.k8s.cli.setters.list:
+      type: array
+      maxItems: 2
+      items:
+        type: integer
+      x-k8s-cli:
+        setter:
+          name: list
+          listValues:
+          - 0
+ `,
+			input: `
+apiVersion: example.com/v1beta1
+kind: Example
+spec:
+  list: # {"$ref":"#/definitions/io.k8s.cli.setters.list"}
+  - 0
+ `,
+			expectedOpenAPI: `
+kind: Kptfile
+openAPI:
+  definitions:
+    io.k8s.cli.setters.list:
+      type: array
+      maxItems: 2
+      items:
+        type: integer
+      x-k8s-cli:
+        setter:
+          name: list
+          listValues:
+          - 0
+ `,
+			expectedResources: `
+apiVersion: example.com/v1beta1
+kind: Example
+spec:
+  list: # {"$ref":"#/definitions/io.k8s.cli.setters.list"}
+  - 0
+ `,
+			errMsg: `list in body must be of type integer: "string"
+list in body must be of type integer: "boolean"
+list in body should have at most 2 items`,
+		},
 	}
 	for i := range tests {
 		test := tests[i]
diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go
index df4299da8..e7daedb6c 100644
--- a/kyaml/setters2/set.go
+++ b/kyaml/setters2/set.go
@@ -58,6 +58,11 @@ func (s *Set) visitSequence(object *yaml.RNode, p string, schema *openapi.Resour
 
 	// set the values on the sequences
 	var elements []*yaml.Node
+	if len(ext.Setter.ListValues) > 0 {
+		if err := validateAgainstSchema(ext, schema.Schema); err != nil {
+			return err
+		}
+	}
 	for i := range ext.Setter.ListValues {
 		v := ext.Setter.ListValues[i]
 		n := yaml.NewScalarRNode(v).YNode()
@@ -200,15 +205,22 @@ func validateAgainstSchema(ext *cliExtension, sch *spec.Schema) error {
 	sc.Properties[ext.Setter.Name] = *sch
 
 	input := map[string]interface{}{}
-	format := `{"%s" : "%s"}`
-	if yaml.IsValueNonString(ext.Setter.Value) {
-		format = `{"%s" : %s}`
+	var inputJSON 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}`
+		}
+		inputJSON = fmt.Sprintf(format, 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
-	inputJSON := fmt.Sprintf(format, ext.Setter.Name, ext.Setter.Value)
 	err := json.Unmarshal([]byte(inputJSON), &input)
 	if err != nil {
 		return err
@@ -220,6 +232,21 @@ 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
diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go
index 2992fee36..bd56b2127 100644
--- a/kyaml/setters2/set_test.go
+++ b/kyaml/setters2/set_test.go
@@ -1293,3 +1293,8 @@ 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))
+}
diff --git a/kyaml/testutil/testutil.go b/kyaml/testutil/testutil.go
index cce2f2e8f..aa4a3e866 100644
--- a/kyaml/testutil/testutil.go
+++ b/kyaml/testutil/testutil.go
@@ -1,3 +1,6 @@
+// Copyright 2019 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
 package testutil_test
 
 import (