From 11e7d0cd73fb924d3347228c68d10ccdb123b6c6 Mon Sep 17 00:00:00 2001 From: Luca Di Rocco Date: Mon, 21 Feb 2022 23:48:41 +0000 Subject: [PATCH] feat: add --set-json flag to set json values. When used with helm install, helm template, helm upgrade, it enables to set json values (scalars/objects/arrays) from the command line. Closes #10428 Signed-off-by: Luca Di Rocco --- cmd/helm/flags.go | 1 + cmd/helm/install.go | 15 +++++- cmd/helm/upgrade.go | 3 +- internal/test/test.go | 2 +- pkg/cli/values/options.go | 8 +++ pkg/strvals/parser.go | 104 +++++++++++++++++++++++++++++++++++-- pkg/strvals/parser_test.go | 101 +++++++++++++++++++++++++++++++++++ 7 files changed, 227 insertions(+), 7 deletions(-) diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 0cc0564e2..76d6e0476 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -47,6 +47,7 @@ func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { f.StringArrayVar(&v.Values, "set", []string{}, "set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") f.StringArrayVar(&v.StringValues, "set-string", []string{}, "set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") f.StringArrayVar(&v.FileValues, "set-file", []string{}, "set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)") + f.StringArrayVar(&v.JSONValues, "set-json", []string{}, "set JSON values on the command line (can specify multiple or separate values with commas: key1=jsonval1,key2=jsonval2)") } func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 0e63ab3a5..9ccffed4b 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -51,7 +51,8 @@ To override values in a chart, use either the '--values' flag and pass in a file or use the '--set' flag and pass configuration from the command line, to force a string value use '--set-string'. You can use '--set-file' to set individual values from a file when the value itself is too long for the command line -or is dynamically generated. +or is dynamically generated. You can also use '--set-json' to set json values +(scalars/objects/arrays) from the command line. $ helm install -f myvalues.yaml myredis ./redis @@ -67,6 +68,11 @@ or $ helm install --set-file my_script=dothings.sh myredis ./redis +or + + $ helm install --set-json 'master.sidecars=[{"name":"sidecar","image":"myImage","imagePullPolicy":"Always","ports":[{"name":"portname","containerPort":1234}]}]' myredis ./redis + + You can specify the '--values'/'-f' flag multiple times. The priority will be given to the last (right-most) file specified. For example, if both myvalues.yaml and override.yaml contained a key called 'Test', the value set in override.yaml would take precedence: @@ -79,6 +85,13 @@ set for a key called 'foo', the 'newbar' value would take precedence: $ helm install --set foo=bar --set foo=newbar myredis ./redis +Similarly, in the following example 'foo' is set to '["four"]': + + $ helm install --set-json='foo=["one", "two", "three"]' --set-json='foo=["four"]' myredis ./redis + +And in the following example, 'foo' is set to '{"key1":"value1","key2":"bar"}': + + $ helm install --set-json='foo={"key1":"value1","key2":"value2"}' --set-json='foo.key2="bar"' myredis ./redis To check the generated manifests of a release without installing the chart, the '--debug' and '--dry-run' flags can be combined. diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 7ada8e3b1..7aed385bb 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -51,7 +51,8 @@ To override values in a chart, use either the '--values' flag and pass in a file or use the '--set' flag and pass configuration from the command line, to force string values, use '--set-string'. You can use '--set-file' to set individual values from a file when the value itself is too long for the command line -or is dynamically generated. +or is dynamically generated. You can also use '--set-json' to set json values +(scalars/objects/arrays) from the command line. You can specify the '--values'/'-f' flag multiple times. The priority will be given to the last (right-most) file specified. For example, if both myvalues.yaml and override.yaml diff --git a/internal/test/test.go b/internal/test/test.go index 646037606..e4f7f639a 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -88,7 +88,7 @@ func compare(actual []byte, filename string) error { } expected = normalize(expected) if !bytes.Equal(expected, actual) { - return errors.Errorf("does not match golden file %s\n\nWANT:\n'%s'\n\nGOT:\n'%s'\n", filename, expected, actual) + return errors.Errorf("does not match golden file %s\n\nWANT:\n'%s'\n\nGOT:\n'%s'", filename, expected, actual) } return nil } diff --git a/pkg/cli/values/options.go b/pkg/cli/values/options.go index e6ad71767..fea25185b 100644 --- a/pkg/cli/values/options.go +++ b/pkg/cli/values/options.go @@ -34,6 +34,7 @@ type Options struct { StringValues []string Values []string FileValues []string + JSONValues []string } // MergeValues merges values from files specified via -f/--values and directly @@ -57,6 +58,13 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er base = mergeMaps(base, currentMap) } + // User specified a value via --set-json + for _, value := range opts.JSONValues { + if err := strvals.ParseJSON(value, base); err != nil { + return nil, errors.Errorf("failed parsing --set-json data %s", value) + } + } + // User specified a value via --set for _, value := range opts.Values { if err := strvals.ParseInto(value, base); err != nil { diff --git a/pkg/strvals/parser.go b/pkg/strvals/parser.go index 457b99f94..fb5df33e0 100644 --- a/pkg/strvals/parser.go +++ b/pkg/strvals/parser.go @@ -17,10 +17,13 @@ package strvals import ( "bytes" + "encoding/json" "fmt" "io" + "io/ioutil" "strconv" "strings" + "unicode" "github.com/pkg/errors" "sigs.k8s.io/yaml" @@ -94,6 +97,18 @@ func ParseIntoString(s string, dest map[string]interface{}) error { return t.parse() } +// ParseJSON parses a string with format key1=val1, key2=val2, ... +// where values are json strings (null, or scalars, or arrays, or objects). +// An empty val is treated as null. +// +// If a key exists in dest, the new value overwrites the dest version. +// +func ParseJSON(s string, dest map[string]interface{}) error { + scanner := bytes.NewBufferString(s) + t := newJSONParser(scanner, dest) + return t.parse() +} + // ParseIntoFile parses a filevals line and merges the result into dest. // // This method always returns a string as the value. @@ -113,9 +128,10 @@ type RunesValueReader func([]rune) (interface{}, error) // where sc is the source of the original data being parsed // where data is the final parsed data from the parses with correct types type parser struct { - sc *bytes.Buffer - data map[string]interface{} - reader RunesValueReader + sc *bytes.Buffer + data map[string]interface{} + reader RunesValueReader + isjsonval bool } func newParser(sc *bytes.Buffer, data map[string]interface{}, stringBool bool) *parser { @@ -125,6 +141,10 @@ func newParser(sc *bytes.Buffer, data map[string]interface{}, stringBool bool) * return &parser{sc: sc, data: data, reader: stringConverter} } +func newJSONParser(sc *bytes.Buffer, data map[string]interface{}) *parser { + return &parser{sc: sc, data: data, reader: nil, isjsonval: true} +} + func newFileParser(sc *bytes.Buffer, data map[string]interface{}, reader RunesValueReader) *parser { return &parser{sc: sc, data: data, reader: reader} } @@ -184,6 +204,33 @@ func (t *parser) key(data map[string]interface{}) (reterr error) { set(data, kk, list) return err case last == '=': + if t.isjsonval { + empval, err := t.emptyVal() + if err != nil { + return err + } + if empval { + set(data, string(k), nil) + return nil + } + // parse jsonvals by using Go’s JSON standard library + // Decode is preferred to Unmarshal in order to parse just the json parts of the list key1=jsonval1,key2=jsonval2,... + // Since Decode has its own buffer that consumes more characters (from underlying t.sc) than the ones actually decoded, + // we invoke Decode on a separate reader built with a copy of what is left in t.sc. After Decode is executed, we + // discard in t.sc the chars of the decoded json value (the number of those characters is returned by InputOffset). + var jsonval interface{} + dec := json.NewDecoder(strings.NewReader(t.sc.String())) + if err = dec.Decode(&jsonval); err != nil { + return err + } + set(data, string(k), jsonval) + if _, err = io.CopyN(ioutil.Discard, t.sc, dec.InputOffset()); err != nil { + return err + } + // skip possible blanks and comma + _, err = t.emptyVal() + return err + } //End of key. Consume =, Get value. // FIXME: Get value list first vl, e := t.valList() @@ -205,7 +252,6 @@ func (t *parser) key(data map[string]interface{}) (reterr error) { default: return e } - case last == ',': // No value given. Set the value to empty string. Return error. set(data, string(k), "") @@ -280,6 +326,34 @@ func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) { case err != nil: return list, err case last == '=': + if t.isjsonval { + empval, err := t.emptyVal() + if err != nil { + return list, err + } + if empval { + return setIndex(list, i, nil) + } + // parse jsonvals by using Go’s JSON standard library + // Decode is preferred to Unmarshal in order to parse just the json parts of the list key1=jsonval1,key2=jsonval2,... + // Since Decode has its own buffer that consumes more characters (from underlying t.sc) than the ones actually decoded, + // we invoke Decode on a separate reader built with a copy of what is left in t.sc. After Decode is executed, we + // discard in t.sc the chars of the decoded json value (the number of those characters is returned by InputOffset). + var jsonval interface{} + dec := json.NewDecoder(strings.NewReader(t.sc.String())) + if err = dec.Decode(&jsonval); err != nil { + return list, err + } + if list, err = setIndex(list, i, jsonval); err != nil { + return list, err + } + if _, err = io.CopyN(ioutil.Discard, t.sc, dec.InputOffset()); err != nil { + return list, err + } + // skip possible blanks and comma + _, err = t.emptyVal() + return list, err + } vl, e := t.valList() switch e { case nil: @@ -343,6 +417,28 @@ func (t *parser) listItem(list []interface{}, i int) ([]interface{}, error) { } } +// check for an empty value +// read and consume optional spaces until comma or EOF (empty val) or any other char (not empty val) +// comma and spaces are consumed, while any other char is not cosumed +func (t *parser) emptyVal() (bool, error) { + for { + r, _, e := t.sc.ReadRune() + if e == io.EOF { + return true, nil + } + if e != nil { + return false, e + } + if r == ',' { + return true, nil + } + if !unicode.IsSpace(r) { + t.sc.UnreadRune() + return false, nil + } + } +} + func (t *parser) val() ([]rune, error) { stop := runeSet([]rune{','}) v, _, err := runesUntil(t.sc, stop) diff --git a/pkg/strvals/parser_test.go b/pkg/strvals/parser_test.go index cef98ba0a..57623eae5 100644 --- a/pkg/strvals/parser_test.go +++ b/pkg/strvals/parser_test.go @@ -567,6 +567,107 @@ func TestParseIntoString(t *testing.T) { } } +func TestParseJSON(t *testing.T) { + tests := []struct { + input string + got map[string]interface{} + expect map[string]interface{} + err bool + }{ + { // set json scalars values, and replace one existing key + input: "outer.inner1=\"1\",outer.inner3=3,outer.inner4=true,outer.inner5=\"true\"", + got: map[string]interface{}{ + "outer": map[string]interface{}{ + "inner1": "overwrite", + "inner2": "value2", + }, + }, + expect: map[string]interface{}{ + "outer": map[string]interface{}{ + "inner1": "1", + "inner2": "value2", + "inner3": 3, + "inner4": true, + "inner5": "true", + }, + }, + err: false, + }, + { // set json objects and arrays, and replace one existing key + input: "outer.inner1={\"a\":\"1\",\"b\":2,\"c\":[1,2,3]},outer.inner3=[\"new value 1\",\"new value 2\"],outer.inner4={\"aa\":\"1\",\"bb\":2,\"cc\":[1,2,3]},outer.inner5=[{\"A\":\"1\",\"B\":2,\"C\":[1,2,3]}]", + got: map[string]interface{}{ + "outer": map[string]interface{}{ + "inner1": map[string]interface{}{ + "x": "overwrite", + }, + "inner2": "value2", + "inner3": []interface{}{ + "overwrite", + }, + }, + }, + expect: map[string]interface{}{ + "outer": map[string]interface{}{ + "inner1": map[string]interface{}{"a": "1", "b": 2, "c": []interface{}{1, 2, 3}}, + "inner2": "value2", + "inner3": []interface{}{"new value 1", "new value 2"}, + "inner4": map[string]interface{}{"aa": "1", "bb": 2, "cc": []interface{}{1, 2, 3}}, + "inner5": []interface{}{map[string]interface{}{"A": "1", "B": 2, "C": []interface{}{1, 2, 3}}}, + }, + }, + err: false, + }, + { // null assigment, and no value assigned (equivalent to null) + input: "outer.inner1=,outer.inner3={\"aa\":\"1\",\"bb\":2,\"cc\":[1,2,3]},outer.inner3.cc[1]=null", + got: map[string]interface{}{ + "outer": map[string]interface{}{ + "inner1": map[string]interface{}{ + "x": "overwrite", + }, + "inner2": "value2", + }, + }, + expect: map[string]interface{}{ + "outer": map[string]interface{}{ + "inner1": nil, + "inner2": "value2", + "inner3": map[string]interface{}{"aa": "1", "bb": 2, "cc": []interface{}{1, nil, 3}}, + }, + }, + err: false, + }, + { // syntax error + input: "outer.inner1={\"a\":\"1\",\"b\":2,\"c\":[1,2,3]},outer.inner3=[\"new value 1\",\"new value 2\"],outer.inner4={\"aa\":\"1\",\"bb\":2,\"cc\":[1,2,3]},outer.inner5={\"A\":\"1\",\"B\":2,\"C\":[1,2,3]}]", + got: nil, + expect: nil, + err: true, + }, + } + for _, tt := range tests { + if err := ParseJSON(tt.input, tt.got); err != nil { + if tt.err { + continue + } + t.Fatalf("%s: %s", tt.input, err) + } + if tt.err { + t.Fatalf("%s: Expected error. Got nil", tt.input) + } + y1, err := yaml.Marshal(tt.expect) + if err != nil { + t.Fatalf("Error serializing expected value: %s", err) + } + y2, err := yaml.Marshal(tt.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.input, y1, y2) + } + } +} + func TestParseFile(t *testing.T) { input := "name1=path1" expect := map[string]interface{}{