Merge pull request #2265 from pwittrock/merge3

Setters: support for setting string list fields

cmd/config/internal/commands/cmdlistsetters.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strings"
 
 	"github.com/olekukonko/tablewriter"
 	"github.com/spf13/cobra"
@@ -67,8 +68,15 @@ func (r *ListSettersRunner) runE(c *cobra.Command, args []string) error {
 		table.SetHeader([]string{"NAME", "VALUE", "SET BY", "DESCRIPTION", "COUNT"})
 		for i := range r.List.Setters {
 			s := r.List.Setters[i]
+			v := s.Value
+
+			// if the setter is for a list, populate the values
+			if len(s.ListValues) > 0 {
+				v = strings.Join(s.ListValues, ",")
+				v = fmt.Sprintf("[%s]", v)
+			}
 			table.Append([]string{
-				s.Name, s.Value, s.SetBy, s.Description, fmt.Sprintf("%d", s.Count)})
+				s.Name, v, s.SetBy, s.Description, fmt.Sprintf("%d", s.Count)})
 		}
 		table.Render()
 

cmd/config/internal/commands/cmdset.go

@@ -21,7 +21,7 @@ func NewSetRunner(parent string) *SetRunner {
 	r := &SetRunner{}
 	c := &cobra.Command{
 		Use:     "set DIR NAME [VALUE]",
-		Args:    cobra.RangeArgs(1, 3),
+		Args:    cobra.MinimumNArgs(3),
 		Short:   commands.SetShort,
 		Long:    commands.SetLong,
 		Example: commands.SetExamples,
@@ -94,6 +94,11 @@ func (r *SetRunner) preRunE(c *cobra.Command, args []string) error {
 		var err error
 		r.Set.Name = args[1]
 		r.Set.Value = args[2]
+
+		// set remaining values as list values
+		if len(args) > 3 {
+			r.Set.ListValues = args[3:]
+		}
 		r.Set.Description = r.Perform.Description
 		r.Set.SetBy = r.Perform.SetBy
 		r.OpenAPIFile, err = ext.GetOpenAPIFile(args)

kyaml/fieldmeta/fieldmeta.go

@@ -140,11 +140,11 @@ func (it FieldValueType) Validate(value string) error {
 func (it FieldValueType) Tag() string {
 	switch it {
 	case String:
-		return "!!str"
+		return yaml.StringTag
 	case Bool:
-		return "!!bool"
+		return yaml.BoolTag
 	case Int:
-		return "!!int"
+		return yaml.IntTag
 	}
 	return ""
 }
@@ -152,17 +152,17 @@ func (it FieldValueType) Tag() string {
 func (it FieldValueType) TagForValue(value string) string {
 	switch it {
 	case String:
-		return "!!str"
+		return yaml.StringTag
 	case Bool:
 		if _, err := strconv.ParseBool(string(it)); err != nil {
 			return ""
 		}
-		return "!!bool"
+		return yaml.BoolTag
 	case Int:
 		if _, err := strconv.ParseInt(string(it), 0, 32); err != nil {
 			return ""
 		}
-		return "!!int"
+		return yaml.IntTag
 	}
 	return ""
 }

kyaml/openapi/openapi.go

@@ -210,6 +210,10 @@ func (rs *ResourceSchema) Elements() *ResourceSchema {
 		// either not an array, or array has multiple types
 		return nil
 	}
+	if rs == nil || rs.Schema == nil || rs.Schema.Items == nil {
+		// no-scheme for the items
+		return nil
+	}
 	s := *rs.Schema.Items.Schema
 	for s.Ref.String() != "" {
 		sc, e := Resolve(&s.Ref)

kyaml/setters2/add.go

@@ -43,9 +43,14 @@ func (a *Add) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 	return object, accept(a, object)
 }
 
+func (a *Add) visitSequence(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema) error {
+	// no-op
+	return nil
+}
+
 // 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) error {
+func (a *Add) visitScalar(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error {
 	// check if the field matches
 	if a.FieldName != "" && !strings.HasSuffix(p, a.FieldName) {
 		return nil
@@ -96,6 +101,9 @@ type SetterDefinition struct {
 	// Value is the value of the setter.
 	Value string `yaml:"value"`
 
+	// ListValues are the value of a list setter.
+	ListValues []string `yaml:"listValues,omitempty"`
+
 	// SetBy is the person or role that last set the value.
 	SetBy string `yaml:"setBy,omitempty"`
 

kyaml/setters2/set.go

@@ -27,10 +27,36 @@ func (s *Set) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 	return object, accept(s, object)
 }
 
+// visitSequence will perform setters for sequences
+func (s *Set) visitSequence(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error {
+	ext, err := getExtFromComment(schema)
+	if err != nil {
+		return err
+	}
+	if ext == nil || ext.Setter == nil || ext.Setter.Name != s.Name ||
+		len(ext.Setter.ListValues) == 0 {
+		// setter was not invoked for this sequence
+		return nil
+	}
+	s.Count++
+
+	// set the values on the sequences
+	var elements []*yaml.Node
+	for i := range ext.Setter.ListValues {
+		v := ext.Setter.ListValues[i]
+		n := yaml.NewScalarRNode(v).YNode()
+		n.Style = yaml.DoubleQuotedStyle
+		elements = append(elements, n)
+	}
+	object.YNode().Content = elements
+	object.YNode().Style = yaml.FoldedStyle
+	return nil
+}
+
 // visitScalar
-func (s *Set) visitScalar(object *yaml.RNode, p string) error {
+func (s *Set) visitScalar(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error {
 	// get the openAPI for this field describing how to apply the setter
-	ext, sch, err := getExtFromComment(object)
+	ext, err := getExtFromComment(schema)
 	if err != nil {
 		return err
 	}
@@ -39,13 +65,13 @@ func (s *Set) visitScalar(object *yaml.RNode, p string) error {
 	}
 
 	// perform a direct set of the field if it matches
-	if s.set(object, ext, sch) {
+	if s.set(object, ext, schema.Schema) {
 		s.Count++
 		return nil
 	}
 
 	// perform a substitution of the field if it matches
-	sub, err := s.substitute(object, ext, sch)
+	sub, err := s.substitute(object, ext, schema.Schema)
 	if err != nil {
 		return err
 	}
@@ -111,7 +137,7 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) (
 	field.YNode().Value = p
 
 	// substitutions are always strings
-	field.YNode().Tag = "!!str"
+	field.YNode().Tag = yaml.StringTag
 
 	return true, nil
 }
@@ -147,6 +173,9 @@ type SetOpenAPI struct {
 	// Value is the current value of the setter
 	Value string `yaml:"value"`
 
+	// ListValue is the current value for a list of items
+	ListValues []string `yaml:"listValue"`
+
 	Description string `yaml:"description"`
 
 	SetBy string `yaml:"setBy"`
@@ -159,8 +188,14 @@ func (s SetOpenAPI) UpdateFile(path string) error {
 
 func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 	key := SetterDefinitionPrefix + s.Name
-	def, err := object.Pipe(yaml.Lookup(
+	oa, err := object.Pipe(yaml.Lookup("openAPI", "definitions", key))
-		"openAPI", "definitions", key, "x-k8s-cli", "setter"))
+	if err != nil {
+		return nil, err
+	}
+	if oa == nil {
+		return nil, errors.Errorf("no setter %s found", s.Name)
+	}
+	def, err := oa.Pipe(yaml.Lookup("x-k8s-cli", "setter"))
 	if err != nil {
 		return nil, err
 	}
@@ -168,9 +203,16 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 		return nil, errors.Errorf("no setter %s found", s.Name)
 	}
 
+	// record the OpenAPI type for the setter
+	var t string
+	if n := oa.Field("type"); n != nil {
+		t = n.Value.YNode().Value
+	}
+
 	// if the setter contains an enumValues map, then ensure the set value appears
 	// as a key in the map
-	if values, err := def.Pipe(yaml.Lookup("enumValues")); err != nil {
+	if values, err := def.Pipe(
+		yaml.Lookup("enumValues")); err != nil {
 		// error looking up the enumValues
 		return nil, err
 	} else if values != nil {
@@ -202,11 +244,40 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 	// values are always represented as strings the OpenAPI
 	// since the are unmarshalled into strings.  Use double quote style to
 	// ensure this consistently.
-	v.YNode().Tag = "!!str"
+	v.YNode().Tag = yaml.StringTag
 	v.YNode().Style = yaml.DoubleQuotedStyle
 
-	if err := def.PipeE(&yaml.FieldSetter{Name: "value", Value: v}); err != nil {
+	if t != "array" {
-		return nil, err
+		// set a scalar value
+		if err := def.PipeE(&yaml.FieldSetter{Name: "value", Value: v}); err != nil {
+			return nil, err
+		}
+	} else {
+		// set a list value
+		if err := def.PipeE(&yaml.FieldClearer{Name: "value"}); err != nil {
+			return nil, err
+		}
+		// create the list values
+		var elements []*yaml.Node
+		n := yaml.NewScalarRNode(s.Value).YNode()
+		n.Tag = yaml.StringTag
+		n.Style = yaml.DoubleQuotedStyle
+		elements = append(elements, n)
+		for i := range s.ListValues {
+			v := s.ListValues[i]
+			n := yaml.NewScalarRNode(v).YNode()
+			n.Style = yaml.DoubleQuotedStyle
+			elements = append(elements, n)
+		}
+		l := yaml.NewRNode(&yaml.Node{
+			Kind:    yaml.SequenceNode,
+			Content: elements,
+		})
+
+		def.YNode().Style = yaml.FoldedStyle
+		if err := def.PipeE(&yaml.FieldSetter{Name: "listValues", Value: l}); err != nil {
+			return nil, err
+		}
 	}
 
 	if err := def.PipeE(&yaml.FieldSetter{Name: "setBy", StringValue: s.SetBy}); err != nil {

kyaml/setters2/set_test.go

@@ -595,6 +595,76 @@ spec:
       containers:
       - name: nginx
         image: nginx:1.8.1 # {"$ref": "#/definitions/io.k8s.cli.substitutions.image"}
+ `,
+		},
+		{
+			name:   "set-args-list",
+			setter: "args",
+			openapi: `
+openAPI:
+  definitions:
+    io.k8s.cli.setters.args:
+      x-k8s-cli:
+        type: array
+        setter:
+          name: args
+          listValues: ["1", "2", "3"]
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  # {"$ref": "#/definitions/io.k8s.cli.setters.args"}
+  replicas: []
+ `,
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  # {"$ref": "#/definitions/io.k8s.cli.setters.args"}
+  replicas:
+  - "1"
+  - "2"
+  - "3"
+ `,
+		},
+		{
+			name:   "set-args-list-replace",
+			setter: "args",
+			openapi: `
+openAPI:
+  definitions:
+    io.k8s.cli.setters.args:
+      x-k8s-cli:
+        type: array
+        setter:
+          name: args
+          listValues: ["1", "2", "3"]
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  # {"$ref": "#/definitions/io.k8s.cli.setters.args"}
+  replicas: ["4", "5"]
+ `,
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  # {"$ref": "#/definitions/io.k8s.cli.setters.args"}
+  replicas:
+  - "1"
+  - "2"
+  - "3"
  `,
 		},
 	}
@@ -678,6 +748,7 @@ func TestSetOpenAPI_Filter(t *testing.T) {
 		name        string
 		setter      string
 		value       string
+		values      []string
 		input       string
 		expected    string
 		description string
@@ -1004,6 +1075,33 @@ openAPI:
           value: "2"
  `,
 		},
+
+		{
+			name:   "set-args-list",
+			setter: "args",
+			value:  "2",
+			values: []string{"3", "4"},
+			input: `
+openAPI:
+  definitions:
+    io.k8s.cli.setters.args:
+      type: array
+      x-k8s-cli:
+        setter:
+          name: args
+          listValues: ["1"]
+ `,
+			expected: `
+openAPI:
+  definitions:
+    io.k8s.cli.setters.args:
+      type: array
+      x-k8s-cli:
+        setter:
+          name: args
+          listValues: ["2", "3", "4"]
+`,
+		},
 	}
 	for i := range tests {
 		test := tests[i]
@@ -1015,7 +1113,7 @@ openAPI:
 
 			// invoke the setter
 			instance := &SetOpenAPI{
-				Name: test.setter, Value: test.value,
+				Name: test.setter, Value: test.value, ListValues: test.values,
 				SetBy: test.setBy, Description: test.description}
 			result, err := instance.Filter(in)
 			if test.err != "" {

kyaml/setters2/settersutil/fieldsetter.go

@@ -17,6 +17,9 @@ type FieldSetter struct {
 	// Value is the value to set
 	Value string
 
+	// ListValues contains a list of values to set on a Sequence
+	ListValues []string
+
 	Description string
 
 	SetBy string
@@ -26,7 +29,12 @@ type FieldSetter struct {
 func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) {
 	// Update the OpenAPI definitions
 	soa := setters2.SetOpenAPI{
-		Name: fs.Name, Value: fs.Value, Description: fs.Description, SetBy: fs.SetBy}
+		Name:        fs.Name,
+		Value:       fs.Value,
+		ListValues:  fs.ListValues,
+		Description: fs.Description,
+		SetBy:       fs.SetBy,
+	}
 	if err := soa.UpdateFile(openAPIPath); err != nil {
 		return 0, err
 	}

kyaml/setters2/types.go

@@ -8,9 +8,7 @@ import (
 
 	"github.com/go-openapi/spec"
 	"sigs.k8s.io/kustomize/kyaml/errors"
-	"sigs.k8s.io/kustomize/kyaml/fieldmeta"
 	"sigs.k8s.io/kustomize/kyaml/openapi"
-	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
 type cliExtension struct {
@@ -21,6 +19,7 @@ type cliExtension struct {
 type setter struct {
 	Name       string            `yaml:"name,omitempty" json:"name,omitempty"`
 	Value      string            `yaml:"value,omitempty" json:"value,omitempty"`
+	ListValues []string          `yaml:"listValues,omitempty" json:"listValues,omitempty"`
 	EnumValues map[string]string `yaml:"enumValues,omitempty" json:"enumValues,omitempty"`
 }
 
@@ -57,32 +56,17 @@ func getExtFromSchema(schema *spec.Schema) (*cliExtension, error) {
 
 // getExtFromComment returns the cliExtension openAPI extension if it is present as
 // a comment on the field.
-func getExtFromComment(object *yaml.RNode) (*cliExtension, *spec.Schema, error) {
+func getExtFromComment(schema *openapi.ResourceSchema) (*cliExtension, error) {
-	// TODO(pwittrock): also use path to the field to get openapi, not just comments
+	if schema == nil {
-	// parse comment containing the extended openapi for this field
-	fm := fieldmeta.FieldMeta{}
-	if err := fm.Read(object); err != nil {
-		return nil, nil, errors.Wrap(err)
-	}
-	if fm.Schema.Ref.String() == "" {
-		return nil, nil, nil
-	}
-
-	// resolve the comment reference to the extended openapi definitions
-	r, err := openapi.Resolve(&fm.Schema.Ref)
-	if err != nil {
-		return nil, nil, errors.Wrap(err)
-	}
-	if r == nil {
 		// no schema found
 		// TODO(pwittrock): should this be an error if it doesn't resolve?
-		return nil, nil, nil
+		return nil, nil
 	}
 
 	// get the cli extension from the openapi (contains setter information)
-	ext, err := getExtFromSchema(r)
+	ext, err := getExtFromSchema(schema.Schema)
 	if err != nil {
-		return nil, nil, errors.Wrap(err)
+		return nil, errors.Wrap(err)
 	}
-	return ext, r, nil
+	return ext, nil
 }

kyaml/setters2/walk.go

@@ -4,6 +4,8 @@
 package setters2
 
 import (
+	"sigs.k8s.io/kustomize/kyaml/fieldmeta"
+	"sigs.k8s.io/kustomize/kyaml/openapi"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
@@ -13,33 +15,100 @@ type visitor interface {
 	// visitScalar is called for each scalar field value on a resource
 	// node is the scalar field value
 	// path is the path to the field; path elements are separated by '.'
-	visitScalar(node *yaml.RNode, path string) error
+	// oa is the OpenAPI schema for the field
+	visitScalar(node *yaml.RNode, path string, oa *openapi.ResourceSchema) error
+
+	// visitSequence is called for each sequence field value on a resource
+	// node is the sequence field value
+	// path is the path to the field
+	// oa is the OpenAPI schema for the field
+	visitSequence(node *yaml.RNode, path string, oa *openapi.ResourceSchema) error
 }
 
 // accept invokes the appropriate function on v for each field in object
 func accept(v visitor, object *yaml.RNode) error {
-	return acceptImpl(v, object, "")
+	// get the OpenAPI for the type if it exists
+	oa := getSchema(object, nil, "")
+	return acceptImpl(v, object, "", oa)
 }
 
 // acceptImpl implements accept using recursion
-func acceptImpl(v visitor, object *yaml.RNode, p string) error {
+func acceptImpl(v visitor, object *yaml.RNode, p string, oa *openapi.ResourceSchema) error {
 	switch object.YNode().Kind {
 	case yaml.DocumentNode:
 		// Traverse the child of the document
 		return accept(v, yaml.NewRNode(object.YNode()))
 	case yaml.MappingNode:
 		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)
 			// Traverse each field value
-			return acceptImpl(v, node.Value, p+"."+node.Key.YNode().Value)
+			return acceptImpl(v, node.Value, p+"."+node.Key.YNode().Value, oa)
 		})
 	case yaml.SequenceNode:
+		// get the schema for the sequence node, use the schema provided if not present
+		// on the field
+		if err := v.visitSequence(object, p, oa); err != nil {
+			return err
+		}
+		// get the schema for the elements
+		oa = getSchema(object, oa, "")
 		return object.VisitElements(func(node *yaml.RNode) error {
 			// Traverse each list element
-			return acceptImpl(v, node, p)
+			return acceptImpl(v, node, p, oa)
 		})
 	case yaml.ScalarNode:
 		// Visit the scalar field
-		return v.visitScalar(object, p)
+		oa = getSchema(object, oa, "")
+		return v.visitScalar(object, p, oa)
 	}
 	return nil
 }
+
+// getSchema returns OpenAPI schema for an RNode or field of the
+// RNode.  It will overriding the provide schema with field specific values
+// if they are found
+// r is the Node to get the Schema for
+// s is the provided schema for the field if known
+// field is the name of the field
+func getSchema(r *yaml.RNode, s *openapi.ResourceSchema, field string) *openapi.ResourceSchema {
+	// get the override schema if it exists on the field
+	fm := fieldmeta.FieldMeta{}
+	if err := fm.Read(r); err == nil && !fm.IsEmpty() {
+		// per-field schema, this is fine
+		if fm.Schema.Ref.String() != "" {
+			// resolve the reference
+			s, err := openapi.Resolve(&fm.Schema.Ref)
+			if err == nil && s != nil {
+				fm.Schema = *s
+			}
+		}
+		return &openapi.ResourceSchema{Schema: &fm.Schema}
+	}
+
+	// get the schema for a field of the node if the field is provided
+	if s != nil && field != "" {
+		return s.Field(field)
+	}
+
+	// get the schema for the elements if this is a list
+	if s != nil && r.YNode().Kind == yaml.SequenceNode {
+		return s.Elements()
+	}
+
+	// use the provided schema if present
+	if s != nil {
+		return s
+	}
+
+	if yaml.IsEmpty(r) {
+		return nil
+	}
+
+	// lookup the schema for the type
+	m, _ := r.GetMeta()
+	if m.Kind == "" || m.APIVersion == "" {
+		return nil
+	}
+	return openapi.SchemaForResourceType(yaml.TypeMeta{Kind: m.Kind, APIVersion: m.APIVersion})
+}

kyaml/yaml/compatibility.go

@@ -14,9 +14,9 @@ import (
 
 // typeToTag maps OpenAPI schema types to yaml 1.2 tags
 var typeToTag = map[string]string{
-	"string":  "!!str",
+	"string":  StringTag,
-	"integer": "!!int",
+	"integer": IntTag,
-	"boolean": "!!bool",
+	"boolean": BoolTag,
 	"number":  "!!float",
 }
 

kyaml/yaml/kfns.go

@@ -3,7 +3,9 @@
 
 package yaml
 
-import "gopkg.in/yaml.v3"
+import (
+	"gopkg.in/yaml.v3"
+)
 
 // AnnotationClearer removes an annotation at metadata.annotations.
 // Returns nil if the annotation or field does not exist.
@@ -33,7 +35,7 @@ type AnnotationSetter struct {
 func (s AnnotationSetter) Filter(rn *RNode) (*RNode, error) {
 	// some tools get confused about the type if annotations are not quoted
 	v := NewScalarRNode(s.Value)
-	v.YNode().Tag = "!!str"
+	v.YNode().Tag = StringTag
 	v.YNode().Style = yaml.SingleQuotedStyle
 	return rn.Pipe(
 		PathGetter{Path: []string{"metadata", "annotations"}, Create: yaml.MappingNode},
@@ -80,7 +82,7 @@ type LabelSetter struct {
 func (s LabelSetter) Filter(rn *RNode) (*RNode, error) {
 	// some tools get confused about the type if labels are not quoted
 	v := NewScalarRNode(s.Value)
-	v.YNode().Tag = "!!str"
+	v.YNode().Tag = StringTag
 	v.YNode().Style = yaml.SingleQuotedStyle
 	return rn.Pipe(
 		PathGetter{Path: []string{"metadata", "labels"}, Create: yaml.MappingNode},

kyaml/yaml/types.go

@@ -588,6 +588,12 @@ func (rn *RNode) VisitFields(fn func(node *MapNode) error) error {
 	return nil
 }
 
+const (
+	StringTag = "!!str"
+	BoolTag   = "!!bool"
+	IntTag    = "!!int"
+)
+
 // Elements returns the list of elements in the RNode.
 // Returns an error for non-SequenceNodes.
 func (rn *RNode) Elements() ([]*RNode, error) {