Avoid manual step for creating array setters

cmd/config/internal/commands/cmdcreatesetter.go

@@ -91,7 +91,9 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error {
 	}
 
 	if setterVersion == "" {
-		if len(args) < 2 || !c.Flag("value").Changed && len(args) < 3 {
+		if len(args) == 2 && r.Set.SetPartialField.Type == "array" && c.Flag("field").Changed {
+			setterVersion = "v2"
+		} else if len(args) < 2 || !c.Flag("value").Changed && len(args) < 3 {
 			setterVersion = "v1"
 		} else if err := initSetterVersion(c, args); err != nil {
 			return err
@@ -124,6 +126,12 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error {
 		r.CreateSetter.Description = r.Set.SetPartialField.Description
 		r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy
 		r.CreateSetter.Type = r.Set.SetPartialField.Type
+
+		if r.CreateSetter.Type == "array" {
+			// this is just a place holder value, we derive the actual value at
+			// later point from the field path
+			r.CreateSetter.FieldValue = "listValues"
+		}
 	}
 	return nil
 }

cmd/config/internal/commands/cmdcreatesetter_test.go

@@ -130,14 +130,16 @@ spec:
 
 		{
 			name:   "add replicas with schema list values",
-			args:   []string{"list", "a", "--description", "hello world", "--set-by", "me", "--type", "array"},
+			args:   []string{"list", "a", "--description", "hello world", "--set-by", "me", "--type", "array", "--field", "spec.list"},
-			schema: `{"maxItems": 2, "type": "array", "items": {"type": "string"}}`,
+			schema: `{"maxItems": 3, "type": "array", "items": {"type": "string"}}`,
 			input: `
 apiVersion: example.com/v1beta1
 kind: Example
 spec:
   list:
   - "a"
+  - "b"
+  - "c"
  `,
 			inputOpenAPI: `
 apiVersion: v1alpha1
@@ -151,21 +153,26 @@ openAPI:
     io.k8s.cli.setters.list:
       items:
         type: string
-      maxItems: 2
+      maxItems: 3
       type: array
       description: hello world
       x-k8s-cli:
         setter:
           name: list
-          value: a
+          listValues:
+          - a
+          - b
+          - c
           setBy: me
  `,
 			expectedResources: `
 apiVersion: example.com/v1beta1
 kind: Example
 spec:
-  list:
+  list: # {"$openapi":"list"}
-  - "a" # {"$openapi":"list"}
+  - "a"
+  - "b"
+  - "c"
  `,
 		},
 		{

cmd/config/internal/commands/cmdlistsetters.go

@@ -113,17 +113,20 @@ func (r *ListSettersRunner) ListSubstitutions(c *cobra.Command, args []string) e
 		return err
 	}
 	table := newTable(c.OutOrStdout(), r.Markdown)
-	table.SetHeader([]string{"SUBSTITUTION", "PATTERN", "SETTERS"})
+	table.SetHeader([]string{"SUBSTITUTION", "PATTERN", "REFERENCES"})
 	for i := range r.List.Substitutions {
 		s := r.List.Substitutions[i]
-		setters := ""
+		refs := ""
 		for _, value := range s.Values {
-			setter := strings.TrimPrefix(value.Ref, fieldmeta.DefinitionsPrefix+fieldmeta.SetterDefinitionPrefix)
+			// trim setter and substitution prefixes
-			setters = setters + "," + setter
+			ref := strings.TrimPrefix(
+				strings.TrimPrefix(value.Ref, fieldmeta.DefinitionsPrefix+fieldmeta.SetterDefinitionPrefix),
+				fieldmeta.DefinitionsPrefix+fieldmeta.SubstitutionDefinitionPrefix)
+			refs = refs + "," + ref
 		}
-		setters = fmt.Sprintf("[%s]", strings.TrimPrefix(setters, ","))
+		refs = fmt.Sprintf("[%s]", strings.TrimPrefix(refs, ","))
 		table.Append([]string{
-			s.Name, s.Pattern, setters})
+			s.Name, s.Pattern, refs})
 	}
 	if len(r.List.Substitutions) == 0 {
 		return nil

cmd/config/internal/commands/cmdlistsetters_test.go

@@ -104,7 +104,7 @@ spec:
   image      nginx   me2      hello world 2   2      
   replicas   3       me1      hello world 1   1      
   tag        1.7.9   me3      hello world 3   1      
-  SUBSTITUTION    PATTERN      SETTERS    
+  SUBSTITUTION    PATTERN    REFERENCES   
   image          IMAGE:TAG   [image,tag]  
 `,
 		},
@@ -178,7 +178,7 @@ spec:
   image      nginx   me2      hello world 2   3      
   replicas   3       me1      hello world 1   2      
   tag        1.7.9   me3      hello world 3   2      
-  SUBSTITUTION    PATTERN      SETTERS    
+  SUBSTITUTION    PATTERN    REFERENCES   
   image          IMAGE:TAG   [image,tag]  
 `,
 		},
@@ -251,8 +251,116 @@ spec:
 `,
 			expected: `  NAME    VALUE   SET BY    DESCRIPTION    COUNT  
   image   nginx   me2      hello world 2   3      
-  SUBSTITUTION    PATTERN      SETTERS    
+  SUBSTITUTION    PATTERN    REFERENCES   
   image          IMAGE:TAG   [image,tag]  
+`,
+		},
+
+		{
+			name: "list array setter",
+			openapi: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.list:
+      items:
+        type: string
+      maxItems: 3
+      type: array
+      description: hello world
+      x-k8s-cli:
+        setter:
+          name: list
+          value: List Values
+          listValues:
+          - a
+          - b
+          - c
+          setBy: me
+ `,
+			input: `
+apiVersion: example.com/v1beta1
+kind: Example
+metadata:
+  annotations:
+    foo: bar
+spec:
+  list: # {"$openapi":"list"}
+  - "a"
+  - "b"
+  - "c"
+`,
+			expected: `  NAME    VALUE    SET BY   DESCRIPTION   COUNT  
+  list   [a,b,c]   me       hello world   1      
+`,
+		},
+
+		{
+			name: "nested substitution",
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+ `,
+			openapi: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-image-setter}::${my-tag-setter}
+          values:
+          - marker: ${my-image-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-image-setter'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+ `,
+			expected: `       NAME              VALUE        SET BY   DESCRIPTION   COUNT  
+  my-image-setter   nginx                                    2      
+  my-other-setter   nginxotherthing                          1      
+  my-tag-setter     1.7.9                                    2      
+   SUBSTITUTION                        PATTERN                                  REFERENCES             
+  my-image-subst    ${my-image-setter}::${my-tag-setter}             [my-image-setter,my-tag-setter]   
+  my-nested-subst   something/${my-image-subst}/${my-other-setter}   [my-image-subst,my-other-setter]  
 `,
 		},
 	}

kyaml/setters2/add.go

@@ -30,6 +30,9 @@ type Add struct {
 
 	// Ref is the OpenAPI reference to set on the matching fields as a comment.
 	Ref string
+
+	// ListValues are the value of a list setter.
+	ListValues []string
 }
 
 // Filter implements yaml.Filter
@@ -48,6 +51,37 @@ func (a *Add) visitSequence(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema)
 	return nil
 }
 
+// visitMapping implements visitor
+// visitMapping visits the fields in input MappingNode and adds setter/subst ref
+// if the path path spec matches with input FiledName
+func (a *Add) visitMapping(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error {
+	return object.VisitFields(func(node *yaml.MapNode) error {
+		if node.Value.YNode().Kind != yaml.SequenceNode {
+			return nil
+		}
+
+		key, err := node.Key.String()
+		if err != nil {
+			return err
+		}
+
+		// derive the list values for the sequence node to write it to openAPI definitions
+		var values []string
+		for _, sc := range node.Value.Content() {
+			values = append(values, sc.Value)
+		}
+		a.ListValues = values
+
+		// pathToKey refers to the path address of the key node ex: metadata.annotations
+		// p is the path till parent node, pathToKey is obtained by appending child key
+		pathToKey := p + "." + strings.Trim(key, "\n")
+		if a.FieldName != "" && strings.HasSuffix(pathToKey, a.FieldName) {
+			return a.addRef(node.Key)
+		}
+		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, _ *openapi.ResourceSchema) error {
@@ -58,7 +92,11 @@ func (a *Add) visitScalar(object *yaml.RNode, p string, _ *openapi.ResourceSchem
 	if a.FieldValue != "" && a.FieldValue != object.YNode().Value {
 		return nil
 	}
+	return a.addRef(object)
+}
 
+// addRef adds the setter/subst ref to the object node as a line comment
+func (a *Add) addRef(object *yaml.RNode) error {
 	// read the field metadata
 	fm := fieldmeta.FieldMeta{}
 	if err := fm.Read(object); err != nil {
@@ -85,7 +123,7 @@ type SetterDefinition struct {
 	Name string `yaml:"name"`
 
 	// Value is the value of the setter.
-	Value string `yaml:"value"`
+	Value string `yaml:"value,omitempty"`
 
 	// ListValues are the value of a list setter.
 	ListValues []string `yaml:"listValues,omitempty"`

kyaml/setters2/set.go

@@ -45,6 +45,10 @@ func (s *Set) isMatch(name string) bool {
 	return s.SetAll || s.Name == name
 }
 
+func (s *Set) visitMapping(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error {
+	return nil
+}
+
 // visitSequence will perform setters for sequences
 func (s *Set) visitSequence(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error {
 	ext, err := getExtFromComment(schema)

kyaml/setters2/settersutil/settercreator.go

@@ -60,16 +60,32 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error {
 
 	// Update the resources with the setter reference
 	inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath}
-	return kio.Pipeline{
+	a := &setters2.Add{
-		Inputs: []kio.Reader{inout},
+		FieldName:  c.FieldName,
-		Filters: []kio.Filter{kio.FilterAll(
+		FieldValue: c.FieldValue,
-			&setters2.Add{
+		Ref:        fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + c.Name,
-				FieldName:  c.FieldName,
+	}
-				FieldValue: c.FieldValue,
+	err = kio.Pipeline{
-				Ref:        fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + c.Name,
+		Inputs:  []kio.Reader{inout},
-			})},
+		Filters: []kio.Filter{kio.FilterAll(a)},
 		Outputs: []kio.Writer{inout},
 	}.Execute()
+
+	if err != nil {
+		return err
+	}
+
+	// if the setter is of array type write the derived list values back to
+	// openAPI definitions
+	if len(a.ListValues) > 0 {
+		sd.ListValues = a.ListValues
+		sd.Value = ""
+		if err := sd.AddToFile(openAPIPath); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 // schemaFromFile reads the contents from schemaPath and returns schema

kyaml/setters2/walk.go

@@ -23,6 +23,12 @@ type visitor interface {
 	// 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
+
+	// visitMapping is called for each Mapping field value on a resource
+	// node is the mapping field value
+	// path is the path to the field
+	// oa is the OpenAPI schema for the field
+	visitMapping(node *yaml.RNode, path string, oa *openapi.ResourceSchema) error
 }
 
 // accept invokes the appropriate function on v for each field in object
@@ -39,6 +45,9 @@ func acceptImpl(v visitor, object *yaml.RNode, p string, oa *openapi.ResourceSch
 		// Traverse the child of the document
 		return accept(v, yaml.NewRNode(object.YNode()))
 	case yaml.MappingNode:
+		if err := v.visitMapping(object, p, oa); err != nil {
+			return err
+		}
 		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)

kyaml/yaml/compatibility_test.go

@@ -23,7 +23,7 @@ func TestIsYaml1_1NonString(t *testing.T) {
 		{val: "2", expected: true},
 		{val: "true", expected: true},
 		{val: "1.0\nhello", expected: false}, // multiline strings should always be false
-		{val: "", expected: false}, // empty string should be considered a string
+		{val: "", expected: false},           // empty string should be considered a string
 	}
 
 	for k := range valueToTagMap {