Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add --set-json flag to set json values. #10693

Merged
merged 1 commit into from Aug 31, 2022
Merged

feat: add --set-json flag to set json values. #10693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/helm/flags.go
View file
Open in desktop
Expand Up @@ -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) {
Expand Down
15 changes: 14 additions & 1 deletion cmd/helm/install.go
View file
Open in desktop
Expand Up @@ -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

Expand All @@ -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:
Expand All @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion cmd/helm/upgrade.go
View file
Open in desktop
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/test/test.go
View file
Open in desktop
Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/cli/values/options.go
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
104 changes: 100 additions & 4 deletions pkg/strvals/parser.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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}
}
Expand Down Expand Up @@ -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()
Expand All @@ -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), "")
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
101 changes: 101 additions & 0 deletions pkg/strvals/parser_test.go
View file
Open in desktop
Expand Up @@ -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
Copy link
Contributor

@joejulian joejulian Mar 1, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the flag is specified twice with conflicting values or subsets of values?

--set-json='foo={"key1":"value1","key2":"value2"}' --set-json='foo.key2="bar"'

or

--set-json='foo=["one", "two", "three"]' --set-json='foo=["four"]'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to the other --set-x flags, for each key the rightmost assignment prevails.
This implies that with --set-json='foo={"key1":"value1","key2":"value2"}' --set-json='foo.key2="bar"' foo is set to {"key1":"value1","key2":"bar"}
while with --set-json='foo=["one", "two", "three"]' --set-json='foo=["four"]' foo is set to ["four"]

Copy link
Contributor

@joejulian joejulian Mar 2, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be explicitly called out in documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I can add some more info/examples to install.go and upgrade.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joejulian
I have checked that in cmd/helm/install.go (const installDesc) it is already explained the behavior of the --set flag when it is specified multiple times.
I have added the above examples in order to highlight a similar behavior with --set-json:

You can specify the ‘–set’ flag multiple times. The priority will be given to the last (right-most) set specified. For example, if both ‘bar’ and ‘newbar’ values are 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

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{}{
Expand Down