From feeaa994b7f11be54743802516c9edbb78dcedfc Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Fri, 31 Jul 2020 09:38:27 -0700 Subject: [PATCH 1/2] Fix issue where the schema was not propagated correctly when walking yaml doc --- kyaml/setters2/walk.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kyaml/setters2/walk.go b/kyaml/setters2/walk.go index 6ca698336..8c50cb99d 100644 --- a/kyaml/setters2/walk.go +++ b/kyaml/setters2/walk.go @@ -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, fieldSchema) } return nil } From 5c433ead5e157a871853e0a04d8b7c7a8e8c43a4 Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Fri, 31 Jul 2020 13:44:47 -0700 Subject: [PATCH 2/2] Use k8s schema to determine formatting if no type on setter --- kyaml/setters2/add.go | 2 +- kyaml/setters2/delete.go | 2 +- kyaml/setters2/set.go | 23 +++++++++++++++++------ kyaml/setters2/set_test.go | 10 +++++----- kyaml/setters2/walk.go | 4 ++-- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index 91f2ec046..5d3ccf279 100644 --- a/kyaml/setters2/add.go +++ b/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 diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go index 448a3467d..d6943138a 100644 --- a/kyaml/setters2/delete.go +++ b/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 diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index af73f074b..e03be56e5 100644 --- a/kyaml/setters2/set.go +++ b/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 } diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go index d1d7d289e..41c01214f 100644 --- a/kyaml/setters2/set_test.go +++ b/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: diff --git a/kyaml/setters2/walk.go b/kyaml/setters2/walk.go index 8c50cb99d..619cad0c6 100644 --- a/kyaml/setters2/walk.go +++ b/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 @@ -69,7 +69,7 @@ func acceptImpl(v visitor, object *yaml.RNode, p string, oa *openapi.ResourceSch case yaml.ScalarNode: // Visit the scalar field fieldSchema := getSchema(object, oa, "") - return v.visitScalar(object, p, fieldSchema) + return v.visitScalar(object, p, oa, fieldSchema) } return nil }