fix(helm): improve --set parser

This replaces the old set parser with a brand new one. This also changes
the internal algorithm from duplicating YAML to merging YAML, which
might solve a problem one user reported in chat, but which was never
captured in an issue.

Closes #1540
Closes #1556
This commit is contained in:
Matt Butcher 2016-11-21 19:06:03 -07:00
parent 5517d00a48
commit 6a1aab7fc8
No known key found for this signature in database
GPG Key ID: DCD5F5E5EF32C345
7 changed files with 596 additions and 175 deletions

View File

@ -24,7 +24,6 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strconv"
"strings"
"text/template"
@ -35,6 +34,7 @@ import (
"k8s.io/helm/cmd/helm/downloader"
"k8s.io/helm/cmd/helm/helmpath"
"k8s.io/helm/cmd/helm/strvals"
"k8s.io/helm/pkg/helm"
"k8s.io/helm/pkg/kube"
"k8s.io/helm/pkg/proto/hapi/release"
@ -95,7 +95,7 @@ type installCmd struct {
keyring string
out io.Writer
client helm.Interface
values *values
values string
nameTemplate string
version string
}
@ -104,7 +104,6 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
inst := &installCmd{
out: out,
client: c,
values: new(values),
}
cmd := &cobra.Command{
@ -133,7 +132,7 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command {
f.BoolVar(&inst.dryRun, "dry-run", false, "simulate an install")
f.BoolVar(&inst.disableHooks, "no-hooks", false, "prevent hooks from running during install")
f.BoolVar(&inst.replace, "replace", false, "re-use the given name, even if that name is already used. This is unsafe in production")
f.Var(inst.values, "set", "set values on the command line. Separate values with commas: key1=val1,key2=val2")
f.StringVar(&inst.values, "set", "", "set values on the command line. Separate values with commas: key1=val1,key2=val2")
f.StringVar(&inst.nameTemplate, "name-template", "", "specify template used to name the release")
f.BoolVar(&inst.verify, "verify", false, "verify the package before installing it")
f.StringVar(&inst.keyring, "keyring", defaultKeyring(), "location of public keys used for verification")
@ -199,7 +198,7 @@ func (i *installCmd) run() error {
}
func (i *installCmd) vals() ([]byte, error) {
var buffer bytes.Buffer
base := map[string]interface{}{}
// User specified a values file via -f/--values
if i.valuesFile != "" {
@ -207,24 +206,17 @@ func (i *installCmd) vals() ([]byte, error) {
if err != nil {
return []byte{}, err
}
buffer.Write(bytes)
// Force a new line. An extra won't matter, but a missing one can
// break things. https://github.com/kubernetes/helm/issues/1430
buffer.WriteRune('\n')
}
// User specified value pairs via --set
// These override any values in the specified file
if len(i.values.pairs) > 0 {
bytes, err := i.values.yaml()
if err != nil {
return []byte{}, err
if err := yaml.Unmarshal(bytes, &base); err != nil {
return []byte{}, fmt.Errorf("failed to parse %s: %s", i.valuesFile, err)
}
buffer.Write(bytes)
}
return buffer.Bytes(), nil
if err := strvals.ParseInto(i.values, base); err != nil {
return []byte{}, fmt.Errorf("failed parsing --set data: %s", err)
}
return yaml.Marshal(base)
}
// printRelease prints info about a release if the flagDebug is true.
@ -243,81 +235,6 @@ func (i *installCmd) printRelease(rel *release.Release) {
}
}
// values represents the command-line value pairs
type values struct {
pairs map[string]interface{}
}
func (v *values) yaml() ([]byte, error) {
return yaml.Marshal(v.pairs)
}
func (v *values) String() string {
out, _ := v.yaml()
return string(out)
}
func (v *values) Type() string {
// Added to pflags.Value interface, but not documented there.
return "struct"
}
func (v *values) Set(data string) error {
v.pairs = map[string]interface{}{}
items := strings.Split(data, ",")
for _, item := range items {
n, val := splitPair(item)
names := strings.Split(n, ".")
ln := len(names)
current := &v.pairs
for i := 0; i < ln; i++ {
if i+1 == ln {
// We're at the last element. Set it.
(*current)[names[i]] = val
} else {
//
if e, ok := (*current)[names[i]]; !ok {
m := map[string]interface{}{}
(*current)[names[i]] = m
current = &m
} else if m, ok := e.(map[string]interface{}); ok {
current = &m
}
}
}
}
return nil
}
func typedVal(val string) interface{} {
if strings.EqualFold(val, "true") {
return true
}
if strings.EqualFold(val, "false") {
return false
}
if iv, err := strconv.ParseInt(val, 10, 64); err == nil {
return iv
}
if fv, err := strconv.ParseFloat(val, 64); err == nil {
return fv
}
return val
}
func splitPair(item string) (name string, value interface{}) {
pair := strings.SplitN(item, "=", 2)
if len(pair) == 1 {
return pair[0], true
}
return pair[0], typedVal(pair[1])
}
// locateChartPath looks for a chart directory in known places, and returns either the full path or an error.
//
// This does not ensure that the chart is well-formed; only that the requested filename exists.

View File

@ -17,7 +17,6 @@ limitations under the License.
package main
import (
"fmt"
"io"
"regexp"
"strings"
@ -99,71 +98,6 @@ func TestInstall(t *testing.T) {
})
}
func TestValues(t *testing.T) {
args := "sailor=sinbad,good,port.source=baghdad,port.destination=basrah,success=True"
vobj := new(values)
vobj.Set(args)
if vobj.Type() != "struct" {
t.Fatalf("Expected Type to be struct, got %s", vobj.Type())
}
vals := vobj.pairs
if fmt.Sprint(vals["good"]) != "true" {
t.Errorf("Expected good to be true. Got %v", vals["good"])
}
if !vals["success"].(bool) {
t.Errorf("Expected boolean true. Got %T, %v", vals["success"], vals["success"])
}
port := vals["port"].(map[string]interface{})
if fmt.Sprint(port["source"]) != "baghdad" {
t.Errorf("Expected source to be baghdad. Got %s", port["source"])
}
if fmt.Sprint(port["destination"]) != "basrah" {
t.Errorf("Expected source to be baghdad. Got %s", port["source"])
}
y := `good: true
port:
destination: basrah
source: baghdad
sailor: sinbad
success: true
`
out, err := vobj.yaml()
if err != nil {
t.Fatal(err)
}
if string(out) != y {
t.Errorf("Expected YAML to be \n%s\nGot\n%s\n", y, out)
}
if vobj.String() != y {
t.Errorf("Expected String() to be \n%s\nGot\n%s\n", y, out)
}
// Combined case, overriding a property
vals["sailor"] = "pisti"
updatedYAML := `good: true
port:
destination: basrah
source: baghdad
sailor: pisti
success: true
`
newOut, err := vobj.yaml()
if err != nil {
t.Fatal(err)
}
if string(newOut) != updatedYAML {
t.Errorf("Expected YAML to be \n%s\nGot\n%s\n", updatedYAML, newOut)
}
}
type nameTemplateTestCase struct {
tpl string
expected string

32
cmd/helm/strvals/doc.go Normal file
View File

@ -0,0 +1,32 @@
/*
Copyright 2016 The Kubernetes Authors All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
/*Package strvals provides tools for working with strval lines.
Helm supports a compressed format for YAML settings which we call strvals.
The format is roughly like this:
name=value,topname.subname=value
The above is equivalent to the YAML document
name: value
topname:
subname: value
This package provides a parser and utilities for converting the strvals format
to other formats.
*/
package strvals

258
cmd/helm/strvals/parser.go Normal file
View File

@ -0,0 +1,258 @@
/*
Copyright 2016 The Kubernetes Authors All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package strvals
import (
"bytes"
"errors"
"fmt"
"io"
"strconv"
"strings"
"github.com/ghodss/yaml"
)
// ToYAML takes a string of arguments and converts to a YAML document.
func ToYAML(s string) (string, error) {
m, err := Parse(s)
if err != nil {
return "", err
}
d, err := yaml.Marshal(m)
return string(d), err
}
// Parse parses a set line.
//
// A set line is of the form name1=value1,name2=value2
func Parse(s string) (map[string]interface{}, error) {
vals := map[string]interface{}{}
scanner := bytes.NewBufferString(s)
t := newParser(scanner, vals)
err := t.parse()
return vals, err
}
//ParseInto parses a strvals line and merges the result into dest.
//
// If the strval string has a key that exists in dest, it overwrites the
// dest version.
func ParseInto(s string, dest map[string]interface{}) error {
scanner := bytes.NewBufferString(s)
t := newParser(scanner, dest)
return t.parse()
}
// parser is a simple parser that takes a strvals line and parses it into a
// map representation.
type parser struct {
sc *bytes.Buffer
data map[string]interface{}
}
func newParser(sc *bytes.Buffer, data map[string]interface{}) *parser {
return &parser{sc: sc, data: data}
}
func (t *parser) parse() error {
for {
err := t.key(t.data)
if err == nil {
continue
}
if err == io.EOF {
return nil
}
return err
}
}
func runeSet(r []rune) map[rune]bool {
s := make(map[rune]bool, len(r))
for _, rr := range r {
s[rr] = true
}
return s
}
func (t *parser) key(data map[string]interface{}) error {
stop := runeSet([]rune{'=', ',', '.'})
for {
switch k, last, err := runesUntil(t.sc, stop); {
case err != nil:
if len(k) == 0 {
return err
}
return fmt.Errorf("key %q has no value", string(k))
//set(data, string(k), "")
//return err
case last == '=':
//End of key. Consume =, Get value.
// FIXME: Get value list first
vl, e := t.valList()
switch e {
case nil:
set(data, string(k), vl)
return nil
case io.EOF:
set(data, string(k), "")
return e
case ErrNotList:
v, e := t.val()
set(data, string(k), typedVal(v))
return e
default:
return e
}
case last == ',':
// No value given. Set the value to empty string. Return error.
set(data, string(k), "")
return fmt.Errorf("key %q has no value (cannot end with ,)", string(k))
case last == '.':
// First, create or find the target map.
inner := map[string]interface{}{}
if _, ok := data[string(k)]; ok {
inner = data[string(k)].(map[string]interface{})
}
// Recurse
e := t.key(inner)
if len(inner) == 0 {
return fmt.Errorf("key map %q has no value", string(k))
}
set(data, string(k), inner)
return e
}
}
}
func set(data map[string]interface{}, key string, val interface{}) {
// If key is empty, don't set it.
if len(key) == 0 {
return
}
data[key] = val
}
func (t *parser) val() ([]rune, error) {
v := []rune{}
stop := runeSet([]rune{','})
v, _, err := runesUntil(t.sc, stop)
return v, err
}
var ErrNotList = errors.New("not a list")
func (t *parser) valList() ([]interface{}, error) {
r, _, e := t.sc.ReadRune()
if e != nil {
return []interface{}{}, e
}
if r != '{' {
t.sc.UnreadRune()
return []interface{}{}, ErrNotList
}
list := []interface{}{}
stop := runeSet([]rune{',', '}'})
for {
switch v, last, err := runesUntil(t.sc, stop); {
case err != nil:
if err == io.EOF {
err = errors.New("list must terminate with '}'")
}
return list, err
case last == '}':
// If this is followed by ',', consume it.
if r, _, e := t.sc.ReadRune(); e == nil && r != ',' {
t.sc.UnreadRune()
}
list = append(list, typedVal(v))
return list, nil
case last == ',':
list = append(list, typedVal(v))
}
}
}
func runesUntil(in *bytes.Buffer, stop map[rune]bool) ([]rune, rune, error) {
v := []rune{}
for {
switch r, _, e := in.ReadRune(); {
case e != nil:
return v, r, e
case inMap(r, stop):
return v, r, nil
case r == '\\':
next, _, e := in.ReadRune()
if e != nil {
return v, next, e
}
v = append(v, next)
default:
v = append(v, r)
}
}
}
func inMap(k rune, m map[rune]bool) bool {
_, ok := m[k]
return ok
}
func (t *parser) listVal() []rune {
v := []rune{}
for {
switch r, _, e := t.sc.ReadRune(); {
case e != nil:
// End of input or error with reader stops value parsing.
return v
case r == '\\':
//Escape char. Consume next and append.
next, _, e := t.sc.ReadRune()
if e != nil {
return v
}
v = append(v, next)
case r == ',':
//End of key. Consume ',' and return.
return v
default:
v = append(v, r)
}
}
}
func typedVal(v []rune) interface{} {
val := string(v)
if strings.EqualFold(val, "true") {
return true
}
if strings.EqualFold(val, "false") {
return false
}
if iv, err := strconv.ParseInt(val, 10, 64); err == nil {
return iv
}
return val
}

View File

@ -0,0 +1,232 @@
/*
Copyright 2016 The Kubernetes Authors All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package strvals
import (
"testing"
"github.com/ghodss/yaml"
)
func TestParseSet(t *testing.T) {
tests := []struct {
str string
expect map[string]interface{}
err bool
}{
{
"name1=value1",
map[string]interface{}{"name1": "value1"},
false,
},
{
"name1=value1,name2=value2",
map[string]interface{}{"name1": "value1", "name2": "value2"},
false,
},
{
"name1=value1,name2=value2,",
map[string]interface{}{"name1": "value1", "name2": "value2"},
false,
},
{
str: "name1=value1,,,,name2=value2,",
err: true,
},
{
str: "name1=,name2=value2",
expect: map[string]interface{}{"name1": "", "name2": "value2"},
},
{
str: "name1,name2=",
err: true,
},
{
str: "name1,name2=value2",
err: true,
},
{
str: "name1,name2=value2\\",
err: true,
},
{
str: "name1,name2",
err: true,
},
{
"name1=one\\,two,name2=three\\,four",
map[string]interface{}{"name1": "one,two", "name2": "three,four"},
false,
},
{
"name1=one\\=two,name2=three\\=four",
map[string]interface{}{"name1": "one=two", "name2": "three=four"},
false,
},
{
"name1=one two three,name2=three two one",
map[string]interface{}{"name1": "one two three", "name2": "three two one"},
false,
},
{
"outer.inner=value",
map[string]interface{}{"outer": map[string]interface{}{"inner": "value"}},
false,
},
{
"outer.middle.inner=value",
map[string]interface{}{"outer": map[string]interface{}{"middle": map[string]interface{}{"inner": "value"}}},
false,
},
{
"outer.inner1=value,outer.inner2=value2",
map[string]interface{}{"outer": map[string]interface{}{"inner1": "value", "inner2": "value2"}},
false,
},
{
"outer.inner1=value,outer.middle.inner=value",
map[string]interface{}{
"outer": map[string]interface{}{
"inner1": "value",
"middle": map[string]interface{}{
"inner": "value",
},
},
},
false,
},
{
str: "name1.name2",
err: true,
},
{
str: "name1.name2,name1.name3",
err: true,
},
{
str: "name1.name2=",
expect: map[string]interface{}{"name1": map[string]interface{}{"name2": ""}},
},
{
str: "name1.=name2",
err: true,
},
{
str: "name1.,name2",
err: true,
},
{
"name1={value1,value2}",
map[string]interface{}{"name1": []string{"value1", "value2"}},
false,
},
{
"name1={value1,value2},name2={value1,value2}",
map[string]interface{}{
"name1": []string{"value1", "value2"},
"name2": []string{"value1", "value2"},
},
false,
},
{
"name1={1021,902}",
map[string]interface{}{"name1": []int{1021, 902}},
false,
},
{
"name1.name2={value1,value2}",
map[string]interface{}{"name1": map[string]interface{}{"name2": []string{"value1", "value2"}}},
false,
},
{
str: "name1={1021,902",
err: true,
},
}
for _, tt := range tests {
got, err := Parse(tt.str)
if err != nil {
if tt.err {
continue
}
t.Fatalf("%s: %s", tt.str, err)
}
if tt.err {
t.Errorf("%s: Expected error. Got nil", tt.str)
}
y1, err := yaml.Marshal(tt.expect)
if err != nil {
t.Fatal(err)
}
y2, err := yaml.Marshal(got)
if err != nil {
t.Fatalf("Error serializing parsed value: %s", err)
}
if string(y1) != string(y2) {
t.Errorf("%s: Expected:\n%s\nGot:\n%s", tt.str, y1, y2)
}
}
}
func TestParseInto(t *testing.T) {
got := map[string]interface{}{
"outer": map[string]interface{}{
"inner1": "overwrite",
"inner2": "value2",
},
}
input := "outer.inner1=value1,outer.inner3=value3"
expect := map[string]interface{}{
"outer": map[string]interface{}{
"inner1": "value1",
"inner2": "value2",
"inner3": "value3",
},
}
if err := ParseInto(input, got); err != nil {
t.Fatal(err)
}
y1, err := yaml.Marshal(expect)
if err != nil {
t.Fatal(err)
}
y2, err := yaml.Marshal(got)
if err != nil {
t.Fatalf("Error serializing parsed value: %s", err)
}
if string(y1) != string(y2) {
t.Errorf("%s: Expected:\n%s\nGot:\n%s", input, y1, y2)
}
}
func TestToYAML(t *testing.T) {
// The TestParse does the hard part. We just verify that YAML formatting is
// happening.
o, err := ToYAML("name=value")
if err != nil {
t.Fatal(err)
}
expect := "name: value\n"
if o != expect {
t.Errorf("Expected %q, got %q", expect, o)
}
}

View File

@ -17,14 +17,15 @@ limitations under the License.
package main
import (
"bytes"
"fmt"
"io"
"io/ioutil"
"strings"
"github.com/ghodss/yaml"
"github.com/spf13/cobra"
"k8s.io/helm/cmd/helm/strvals"
"k8s.io/helm/pkg/helm"
"k8s.io/helm/pkg/storage/driver"
)
@ -49,7 +50,7 @@ type upgradeCmd struct {
dryRun bool
disableHooks bool
valuesFile string
values *values
values string
verify bool
keyring string
install bool
@ -62,7 +63,6 @@ func newUpgradeCmd(client helm.Interface, out io.Writer) *cobra.Command {
upgrade := &upgradeCmd{
out: out,
client: client,
values: new(values),
}
cmd := &cobra.Command{
@ -86,7 +86,7 @@ func newUpgradeCmd(client helm.Interface, out io.Writer) *cobra.Command {
f := cmd.Flags()
f.StringVarP(&upgrade.valuesFile, "values", "f", "", "path to a values YAML file")
f.BoolVar(&upgrade.dryRun, "dry-run", false, "simulate an upgrade")
f.Var(upgrade.values, "set", "set values on the command line. Separate values with commas: key1=val1,key2=val2")
f.StringVar(&upgrade.values, "set", "", "set values on the command line. Separate values with commas: key1=val1,key2=val2")
f.BoolVar(&upgrade.disableHooks, "disable-hooks", false, "disable pre/post upgrade hooks")
f.BoolVar(&upgrade.verify, "verify", false, "verify the provenance of the chart before upgrading")
f.StringVar(&upgrade.keyring, "keyring", defaultKeyring(), "path to the keyring that contains public singing keys")
@ -154,7 +154,7 @@ func (u *upgradeCmd) run() error {
}
func (u *upgradeCmd) vals() ([]byte, error) {
var buffer bytes.Buffer
base := map[string]interface{}{}
// User specified a values file via -f/--values
if u.valuesFile != "" {
@ -162,18 +162,15 @@ func (u *upgradeCmd) vals() ([]byte, error) {
if err != nil {
return []byte{}, err
}
buffer.Write(bytes)
}
// User specified value pairs via --set
// These override any values in the specified file
if len(u.values.pairs) > 0 {
bytes, err := u.values.yaml()
if err != nil {
return []byte{}, err
if err := yaml.Unmarshal(bytes, base); err != nil {
return []byte{}, fmt.Errorf("failed to parse %s: %s", u.valuesFile, err)
}
buffer.Write(bytes)
}
return buffer.Bytes(), nil
if err := strvals.ParseInto(u.values, base); err != nil {
return []byte{}, fmt.Errorf("failed parsing --set data: %s", err)
}
return yaml.Marshal(base)
}

View File

@ -224,6 +224,57 @@ $ helm install -f config.yaml stable/mariadb
The above will set the default MariaDB user to `user0`, but accept all
the rest of the defaults for that chart.
There are two ways to pass configuration data during install:
- `--values` (or `-f`): Specifiy a YAML file with overrides.
- `--set`: Specify overrides on the command line.
If both are used, `--set` values are merged into `--values` with higher precedence.
#### The Format and Limitations of `--set`
The `--set` option takes zero or more name/value pairs. At its simplest, it is
used like this: `--set name=value`. The YAML equivalent of that is:
```yaml
name: value
```
Multiple values are separated by `,` characters. So `--set a=b,c=d` becomes:
```yaml
a: b
c: d
```
More complex expressions are supported. For example, `--set outer.inner=value` is
translated into this:
```yaml
outer:
inner: value
```
Lists can be expressed by enclosing values in `{` and `}`. For example,
`--set name={a, b, c}` translates to:
```yaml
name:
- a
- b
- c
```
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:
```yaml
name: "value1,value2"
```
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...".
### More Installation Methods
The `helm install` command can install from several sources: