Merge pull request #2545 from technosophos/feat/set-list-index

feat(helm): support array index format for --set.

cmd/helm/installer/install.go

@@ -107,7 +107,6 @@ func DeploymentManifest(opts *Options) (string, error) {
 // resource.
 func ServiceManifest(namespace string) (string, error) {
 	obj := service(namespace)
-
 	buf, err := yaml.Marshal(obj)
 	return string(buf), err
 }

docs/chart_best_practices/values.md

@@ -113,6 +113,11 @@ servers:
     port: 81
 ```
 
+The above cannot be expressed with `--set` in Helm `<=2.4`. In Helm 2.5, the
+accessing the port on foo is `--set servers[0].port=80`. Not only is it harder
+for the user to figure out, but it is prone to errors if at some later time the
+order of the `servers` is changed.
+
 Easy to use:
 
 ```yaml
@@ -123,6 +128,8 @@ servers:
     port: 81
 ```
 
+Accessing foo's port is much more obvious: `--set servers.foo.port=80`.
+
 ## Document 'values.yaml'
 
 Every defined property in 'values.yaml' should be documented. The documentation string should begin with the name of the property that it describes, and then give at least a one-sentence description.

docs/using_helm.md

@@ -264,6 +264,22 @@ name:
   - c
 ```
 
+As of Helm 2.5.0, it is possible to access list items using an array index syntax.
+For example, `--set servers[0].port=80` becomes:
+
+```yaml
+servers:
+  - port: 80
+```
+
+Multiple values can be set this way. The line `--set servers[0].port=80,servers[0].host=example` becomes:
+
+```yaml
+servers:
+  - port: 80
+    host: example
+```
+
 Sometimes you need to use special characters in your `--set` lines. You can use
 a backslash to escape the characters; `--set name=value1\,value2` will become:
 
@@ -280,9 +296,9 @@ nodeSelector:
   kubernetes.io/role: master
 ```
 
-The `--set` syntax is not as expressive as YAML, especially when it comes to
-collections. And there is currently no method for expressing things such as "set
-the third item in a list to...".
+Deeply nested datastructures can be difficult to express using `--set`. Chart
+designers are encouraged to consider the `--set` usage when designing the format
+of a `values.yaml` file.
 
 ### More Installation Methods
 

pkg/strvals/parser.go

@@ -93,7 +93,7 @@ func runeSet(r []rune) map[rune]bool {
 }
 
 func (t *parser) key(data map[string]interface{}) error {
-	stop := runeSet([]rune{'=', ',', '.'})
+	stop := runeSet([]rune{'=', '[', ',', '.'})
 	for {
 		switch k, last, err := runesUntil(t.sc, stop); {
 		case err != nil:
@@ -103,6 +103,23 @@ func (t *parser) key(data map[string]interface{}) error {
 			return fmt.Errorf("key %q has no value", string(k))
 			//set(data, string(k), "")
 			//return err
+		case last == '[':
+			// We are in a list index context, so we need to set an index.
+			i, err := t.keyIndex()
+			if err != nil {
+				return fmt.Errorf("error parsing index: %s", err)
+			}
+			kk := string(k)
+			// Find or create target list
+			list := []interface{}{}
+			if _, ok := data[kk]; ok {
+				list = data[kk].([]interface{})
+			}
+
+			// Now we need to get the value after the ].
+			list, err = t.listItem(list, i)
+			set(data, kk, list)
+			return err
 		case last == '=':
 			//End of key. Consume =, Get value.
 			// FIXME: Get value list first
@@ -152,6 +169,71 @@ func set(data map[string]interface{}, key string, val interface{}) {
 	data[key] = val
 }
 
+func setIndex(list []interface{}, index int, val interface{}) []interface{} {
+	if len(list) <= index {
+		newlist := make([]interface{}, index+1)
+		copy(newlist, list)
+		list = newlist
+	}
+	list[index] = val
+	return list
+}
+
+func (t *parser) keyIndex() (int, error) {
+	// First, get the key.
+	stop := runeSet([]rune{']'})
+	v, _, err := runesUntil(t.sc, stop)
+	if err != nil {
+		return 0, err
+	}
+	// v should be the index
+	return strconv.Atoi(string(v))
+
+}
+func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) {
+	stop := runeSet([]rune{'[', '.', '='})
+	switch k, last, err := runesUntil(t.sc, stop); {
+	case len(k) > 0:
+		return list, fmt.Errorf("unexpected data at end of array index: %q", k)
+	case err != nil:
+		return list, err
+	case last == '=':
+		vl, e := t.valList()
+		switch e {
+		case nil:
+			return setIndex(list, i, vl), nil
+		case io.EOF:
+			return setIndex(list, i, ""), err
+		case ErrNotList:
+			v, e := t.val()
+			return setIndex(list, i, typedVal(v)), e
+		default:
+			return list, e
+		}
+	case last == '[':
+		// now we have a nested list. Read the index and handle.
+		i, err := t.keyIndex()
+		if err != nil {
+			return list, fmt.Errorf("error parsing index: %s", err)
+		}
+		// Now we need to get the value after the ].
+		list2, err := t.listItem(list, i)
+		return setIndex(list, i, list2), err
+	case last == '.':
+		// We have a nested object. Send to t.key
+		inner := map[string]interface{}{}
+		if len(list) > i {
+			inner = list[i].(map[string]interface{})
+		}
+
+		// Recurse
+		e := t.key(inner)
+		return setIndex(list, i, inner), e
+	default:
+		return nil, fmt.Errorf("parse error: unexpected token %v", last)
+	}
+}
+
 func (t *parser) val() ([]rune, error) {
 	stop := runeSet([]rune{','})
 	v, _, err := runesUntil(t.sc, stop)

pkg/strvals/parser_test.go

@@ -21,6 +21,49 @@ import (
 	"github.com/ghodss/yaml"
 )
 
+func TestSetIndex(t *testing.T) {
+	tests := []struct {
+		name    string
+		initial []interface{}
+		expect  []interface{}
+		add     int
+		val     int
+	}{
+		{
+			name:    "short",
+			initial: []interface{}{0, 1},
+			expect:  []interface{}{0, 1, 2},
+			add:     2,
+			val:     2,
+		},
+		{
+			name:    "equal",
+			initial: []interface{}{0, 1},
+			expect:  []interface{}{0, 2},
+			add:     1,
+			val:     2,
+		},
+		{
+			name:    "long",
+			initial: []interface{}{0, 1, 2, 3, 4, 5},
+			expect:  []interface{}{0, 1, 2, 4, 4, 5},
+			add:     3,
+			val:     4,
+		},
+	}
+
+	for _, tt := range tests {
+		got := setIndex(tt.initial, tt.add, tt.val)
+		if len(got) != len(tt.expect) {
+			t.Fatalf("%s: Expected length %d, got %d", tt.name, len(tt.expect), len(got))
+		}
+
+		if gg := got[tt.add].(int); gg != tt.val {
+			t.Errorf("%s, Expected value %d, got %d", tt.name, tt.val, gg)
+		}
+	}
+}
+
 func TestParseSet(t *testing.T) {
 	tests := []struct {
 		str    string
@@ -155,6 +198,59 @@ func TestParseSet(t *testing.T) {
 			str: "name1={1021,902",
 			err: true,
 		},
+		// List support
+		{
+			str:    "list[0]=foo",
+			expect: map[string]interface{}{"list": []string{"foo"}},
+		},
+		{
+			str: "list[0].foo=bar",
+			expect: map[string]interface{}{
+				"list": []interface{}{
+					map[string]interface{}{"foo": "bar"},
+				},
+			},
+		},
+		{
+			str: "list[0].foo=bar,list[0].hello=world",
+			expect: map[string]interface{}{
+				"list": []interface{}{
+					map[string]interface{}{"foo": "bar", "hello": "world"},
+				},
+			},
+		},
+		{
+			str:    "list[0]=foo,list[1]=bar",
+			expect: map[string]interface{}{"list": []string{"foo", "bar"}},
+		},
+		{
+			str:    "list[0]=foo,list[1]=bar,",
+			expect: map[string]interface{}{"list": []string{"foo", "bar"}},
+		},
+		{
+			str:    "list[0]=foo,list[3]=bar",
+			expect: map[string]interface{}{"list": []interface{}{"foo", nil, nil, "bar"}},
+		},
+		{
+			str: "illegal[0]name.foo=bar",
+			err: true,
+		},
+		{
+			str:    "noval[0]",
+			expect: map[string]interface{}{"noval": []interface{}{}},
+		},
+		{
+			str:    "noval[0]=",
+			expect: map[string]interface{}{"noval": []interface{}{""}},
+		},
+		{
+			str:    "nested[0][0]=1",
+			expect: map[string]interface{}{"nested": []interface{}{[]interface{}{1}}},
+		},
+		{
+			str:    "nested[1][1]=1",
+			expect: map[string]interface{}{"nested": []interface{}{nil, []interface{}{nil, 1}}},
+		},
 	}
 
 	for _, tt := range tests {