Validate setters against openAPI

kyaml/setters2/set.go

@@ -4,9 +4,13 @@
 package setters2
 
 import (
+	"encoding/json"
+	"fmt"
 	"strings"
 
 	"github.com/go-openapi/spec"
+	"github.com/go-openapi/strfmt"
+	"github.com/go-openapi/validate"
 	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
@@ -77,7 +81,11 @@ func (s *Set) visitScalar(object *yaml.RNode, p string, schema *openapi.Resource
 	}
 
 	// perform a direct set of the field if it matches
-	if s.set(object, ext, schema.Schema) {
+	ok, err := s.set(object, ext, schema.Schema)
+	if err != nil {
+		return err
+	}
+	if ok {
 		s.Count++
 		return nil
 	}
@@ -155,19 +163,21 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) (
 }
 
 // set applies the value from ext to field if its name matches s.Name
-func (s *Set) set(field *yaml.RNode, ext *cliExtension, sch *spec.Schema) bool {
+func (s *Set) set(field *yaml.RNode, ext *cliExtension, sch *spec.Schema) (bool, error) {
 	// check full setter
 	if ext.Setter == nil || !s.isMatch(ext.Setter.Name) {
-		return false
+		return false, nil
 	}
 
-	// TODO(pwittrock): validate the field value
+	if err := validateAgainstSchema(ext, sch); err != nil {
+		return false, err
+	}
 
 	if val, found := ext.Setter.EnumValues[ext.Setter.Value]; found {
 		// the setter has an enum-map.  we should replace the marker with the
 		// enum value looked up from the map rather than the enum key
 		field.YNode().Value = val
-		return true
+		return true, nil
 	}
 
 	// this has a full setter, set its value
@@ -175,7 +185,35 @@ func (s *Set) set(field *yaml.RNode, ext *cliExtension, sch *spec.Schema) bool {
 
 	// format the node so it is quoted if it is a string
 	yaml.FormatNonStringStyle(field.YNode(), *sch)
-	return true
+	return true, nil
+}
+
+// validateAgainstSchema validates the input setter value against user provided
+// openAI schema
+func validateAgainstSchema(ext *cliExtension, sch *spec.Schema) error {
+	sc := spec.Schema{}
+	sc.Properties = map[string]spec.Schema{}
+	sc.Properties[ext.Setter.Name] = *sch
+
+	input := map[string]interface{}{}
+	format := `{"%s" : "%s"}`
+	if yaml.IsValueNonString(ext.Setter.Value) {
+		format = `{"%s" : %s}`
+	}
+
+	// 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
+	}
+	err = validate.AgainstSchema(&sc, input, strfmt.Default)
+	if err != nil {
+		return errors.Errorf("The input value doesn't validate against provided OpenAPI schema: %v\n", err.Error())
+	}
+	return nil
 }
 
 // SetOpenAPI updates a setter value

kyaml/setters2/set_test.go

@@ -63,7 +63,7 @@ spec:
 		},
 		{
 			name:        "set-foo-type",
-			description: "if a type is specified for a setter, ensure the field is properly quoted",
+			description: "if a type is specified for a setter, ensure the field is of provided type",
 			setter:      "foo",
 			openapi: `
 openAPI:
@@ -73,7 +73,7 @@ openAPI:
         setter:
           name: foo
           value: "4"
-      type: string
+      type: integer
  `,
 			input: `
 apiVersion: apps/v1
@@ -89,12 +89,12 @@ kind: Deployment
 metadata:
   name: nginx-deployment
   annotations:
-    foo: "4" # {"$ref": "#/definitions/io.k8s.cli.setters.foo"}
+    foo: 4 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"}
  `,
 		},
 		{
-			name:        "set-foo-type-wrong",
-			description: "if a type is specified for a setter, for the field to the type",
+			name:        "set-foo-type-float",
+			description: "if a type is specified for a setter, ensure the field is of provided type",
 			setter:      "foo",
 			openapi: `
 openAPI:
@@ -103,8 +103,8 @@ openAPI:
       x-k8s-cli:
         setter:
           name: foo
-          value: "4"
-      type: boolean
+          value: "4.0"
+      type: number
  `,
 			input: `
 apiVersion: apps/v1
@@ -120,7 +120,7 @@ kind: Deployment
 metadata:
   name: nginx-deployment
   annotations:
-    foo: !!bool 4 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"}
+    foo: 4.0 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"}
  `,
 		},
 		{

kyaml/setters2/settersutil/fieldsetter.go

@@ -4,6 +4,9 @@
 package settersutil
 
 import (
+	"io/ioutil"
+	"os"
+
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/openapi"
 	"sigs.k8s.io/kustomize/kyaml/setters2"
@@ -47,6 +50,22 @@ func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) {
 		Description: fs.Description,
 		SetBy:       fs.SetBy,
 	}
+
+	// the input field value is updated in the openAPI file and then parsed
+	// at to get the value and set it to resource files, but if there is error
+	// after updating openAPI file and while updating resources, the openAPI
+	// file should be reverted, as set operation failed
+	stat, err := os.Stat(openAPIPath)
+	if err != nil {
+		return 0, err
+	}
+
+	curOpenAPI, err := ioutil.ReadFile(openAPIPath)
+	if err != nil {
+		return 0, err
+	}
+
+	// write the new input value to openAPI file
 	if err := soa.UpdateFile(openAPIPath); err != nil {
 		return 0, err
 	}
@@ -61,11 +80,18 @@ func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) {
 	// hence, rest of the files should not be deleted
 	inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath, NoDeleteFiles: true}
 	s := &setters2.Set{Name: fs.Name}
-	err := kio.Pipeline{
+	err = kio.Pipeline{
 		Inputs:  []kio.Reader{inout},
 		Filters: []kio.Filter{setters2.SetAll(s)},
 		Outputs: []kio.Writer{inout},
 	}.Execute()
+
+	// revert openAPI file if set operation fails
+	if err != nil {
+		if writeErr := ioutil.WriteFile(openAPIPath, curOpenAPI, stat.Mode().Perm()); writeErr != nil {
+			return 0, writeErr
+		}
+	}
 	return s.Count, err
 }