Merge pull request #2792 from mortent/FixSetterWalk

Fix issue where the schema was not propagated correctly when walking yaml doc

kyaml/setters2/add.go

@@ -97,7 +97,7 @@ func (a *Add) visitMapping(object *yaml.RNode, p string, _ *openapi.ResourceSche
 
 // visitScalar implements visitor
 // visitScalar will set the field metadata on each scalar field whose name + value match
-func (a *Add) visitScalar(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error {
+func (a *Add) visitScalar(object *yaml.RNode, p string, _, _ *openapi.ResourceSchema) error {
 	// check if the field matches
 	if a.Type == "array" {
 		return nil

kyaml/setters2/delete.go

@@ -45,7 +45,7 @@ func (d *Delete) visitMapping(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema
 
 // visitScalar implements visitor
 // visitScalar will remove the reference on each scalar field whose name matches.
-func (d *Delete) visitScalar(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error {
+func (d *Delete) visitScalar(object *yaml.RNode, p string, _, _ *openapi.ResourceSchema) error {
 	// check if the field matches
 	if d.FieldName != "" && !strings.HasSuffix(p, d.FieldName) {
 		return nil

kyaml/setters2/set.go

@@ -81,9 +81,9 @@ func (s *Set) visitSequence(object *yaml.RNode, p string, schema *openapi.Resour
 }
 
 // visitScalar
-func (s *Set) visitScalar(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error {
+func (s *Set) visitScalar(object *yaml.RNode, p string, oa, setterSchema *openapi.ResourceSchema) error {
 	// get the openAPI for this field describing how to apply the setter
-	ext, err := getExtFromComment(schema)
+	ext, err := getExtFromComment(setterSchema)
 	if err != nil {
 		return err
 	}
@@ -91,8 +91,13 @@ func (s *Set) visitScalar(object *yaml.RNode, p string, schema *openapi.Resource
 		return nil
 	}
 
+	var k8sSchema *spec.Schema
+	if oa != nil {
+		k8sSchema = oa.Schema
+	}
+
 	// perform a direct set of the field if it matches
-	ok, err := s.set(object, ext, schema.Schema)
+	ok, err := s.set(object, ext, k8sSchema, setterSchema.Schema)
 	if err != nil {
 		return err
 	}
@@ -214,7 +219,7 @@ func (s *Set) substituteUtil(ext *CliExtension, visited sets.String, nameMatch *
 }
 
 // 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, error) {
+func (s *Set) set(field *yaml.RNode, ext *CliExtension, k8sSch, sch *spec.Schema) (bool, error) {
 	// check full setter
 	if ext.Setter == nil || !s.isMatch(ext.Setter.Name) {
 		return false, nil
@@ -234,8 +239,14 @@ func (s *Set) set(field *yaml.RNode, ext *CliExtension, sch *spec.Schema) (bool,
 	// this has a full setter, set its value
 	field.YNode().Value = ext.Setter.Value
 
-	// format the node so it is quoted if it is a string
-	yaml.FormatNonStringStyle(field.YNode(), *sch)
+	// format the node so it is quoted if it is a string. If there is
+	// type information on the setter schema, we use it. Otherwise we
+	// fall back to the field schema if it exists.
+	if len(sch.Type) > 0 {
+		yaml.FormatNonStringStyle(field.YNode(), *sch)
+	} else if k8sSch != nil {
+		yaml.FormatNonStringStyle(field.YNode(), *k8sSch)
+	}
 	return true, nil
 }
 

kyaml/setters2/set_test.go

@@ -126,7 +126,7 @@ metadata:
 		},
 		{
 			name:        "set-foo-no-type",
-			description: "if a type is not specified for a setter, keep the existing quoting",
+			description: "if a type is not specified for a setter or k8s schema, keep existing quoting",
 			setter:      "foo",
 			openapi: `
 openAPI:
@@ -138,16 +138,16 @@ openAPI:
           value: "4"
  `,
 			input: `
-apiVersion: apps/v1
-kind: Deployment
+apiVersion: custom/v1
+kind: Example
 metadata:
   name: nginx-deployment
   annotations:
     foo: 3 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"}
  `,
 			expected: `
-apiVersion: apps/v1
-kind: Deployment
+apiVersion: custom/v1
+kind: Example
 metadata:
   name: nginx-deployment
   annotations:

kyaml/setters2/walk.go

@@ -16,7 +16,7 @@ type visitor interface {
 	// node is the scalar field value
 	// path is the path to the field; path elements are separated by '.'
 	// oa is the OpenAPI schema for the field
-	visitScalar(node *yaml.RNode, path string, oa *openapi.ResourceSchema) error
+	visitScalar(node *yaml.RNode, path string, oa *openapi.ResourceSchema, fieldOA *openapi.ResourceSchema) error
 
 	// visitSequence is called for each sequence field value on a resource
 	// node is the sequence field value
@@ -50,9 +50,9 @@ func acceptImpl(v visitor, object *yaml.RNode, p string, oa *openapi.ResourceSch
 		}
 		return object.VisitFields(func(node *yaml.MapNode) error {
 			// get the schema for the field and propagate it
-			oa = getSchema(node.Key, oa, node.Key.YNode().Value)
+			fieldSchema := getSchema(node.Key, oa, node.Key.YNode().Value)
 			// Traverse each field value
-			return acceptImpl(v, node.Value, p+"."+node.Key.YNode().Value, oa)
+			return acceptImpl(v, node.Value, p+"."+node.Key.YNode().Value, fieldSchema)
 		})
 	case yaml.SequenceNode:
 		// get the schema for the sequence node, use the schema provided if not present
@@ -61,15 +61,15 @@ func acceptImpl(v visitor, object *yaml.RNode, p string, oa *openapi.ResourceSch
 			return err
 		}
 		// get the schema for the elements
-		oa = getSchema(object, oa, "")
+		schema := getSchema(object, oa, "")
 		return object.VisitElements(func(node *yaml.RNode) error {
 			// Traverse each list element
-			return acceptImpl(v, node, p, oa)
+			return acceptImpl(v, node, p, schema)
 		})
 	case yaml.ScalarNode:
 		// Visit the scalar field
-		oa = getSchema(object, oa, "")
-		return v.visitScalar(object, p, oa)
+		fieldSchema := getSchema(object, oa, "")
+		return v.visitScalar(object, p, oa, fieldSchema)
 	}
 	return nil
 }