Implemented index list override for values files

Signed-off-by: Miles Wilson <wilson.mil@icloud.com>
pull/13086/head
Miles Wilson 1 year ago
parent 0d66425d9a
commit 68b2aa131d

@ -49,6 +49,7 @@ func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) {
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)")
f.StringArrayVar(&v.LiteralValues, "set-literal", []string{}, "set a literal STRING value on the command line")
f.BoolVar(&v.IndexOverride, "index-override", false, "if set, allow keys in value files to override list indexes similar to --set syntax")
}
func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) {

@ -318,6 +318,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options
cancel()
}()
chartRequested.IndexOverride = valueOpts.IndexOverride
return client.RunWithContext(ctx, chartRequested, vals)
}

@ -243,6 +243,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
cancel()
}()
ch.IndexOverride = valueOpts.IndexOverride
rel, err := client.RunWithContext(ctx, args[0], ch, vals)
if err != nil {
return errors.Wrap(err, "UPGRADE FAILED")

@ -51,6 +51,8 @@ type Chart struct {
// Files are miscellaneous files in a chart archive,
// e.g. README, LICENSE, etc.
Files []*File `json:"files"`
// specify whether index override is being used for this chart
IndexOverride bool
parent *Chart
dependencies []*Chart

@ -24,6 +24,7 @@ import (
"github.com/pkg/errors"
"helm.sh/helm/v4/pkg/chart"
"helm.sh/helm/v4/pkg/strvals"
)
func concatPrefix(a, b string) string {
@ -219,6 +220,10 @@ func coalesceValues(printf printFn, c *chart.Chart, v map[string]interface{}, pr
}
}
if c.IndexOverride {
processListKeyOverride(printf, subPrefix, v, vc)
}
for key, val := range vc {
if value, ok := v[key]; ok {
if value == nil && !merge {
@ -261,6 +266,68 @@ func childChartMergeTrue(chrt *chart.Chart, key string, merge bool) bool {
return merge
}
func processListKeyOverride(printf printFn, subPrefix string, v map[string]interface{}, vc map[string]interface{}) {
// if there are list override fields in v, we need to combine them with the lists in vc
for key, val := range v {
listKeyName, t := strvals.FindListRune(key)
// if this is not a list key override, potential to recur
if listKeyName == "" {
if vcValue, vcHasKey := vc[key]; vcHasKey {
nestedV, vIsNested := val.(map[string]interface{})
nestedVC, vcIsNested := vcValue.(map[string]interface{})
if vIsNested && vcIsNested {
processListKeyOverride(printf, subPrefix+key, nestedV, nestedVC)
}
}
continue
}
// remove list key the dict, if it should be merged it will be handled
delete(v, key)
// process any lists to merge. there is an index field if this is a listKeyName
// if the list key name is also in the values, then ignore this list key
if _, ok := v[listKeyName]; ok {
continue
}
baseKeyValue, ok := vc[listKeyName]
// if the list key is not in the base list, do nothing
if !ok {
printf(
"warning: list override key %s.%s is invalid because there is no underlying key %s in the base values",
subPrefix, key, listKeyName)
continue
}
baseKeyList, ok := baseKeyValue.([]interface{})
if !ok {
printf(
"warning: list override key %s.%s is invalid because base values key %s is not a list",
subPrefix, key, listKeyName)
continue
}
overrideAtIdx, err := t.ParseListIndex()
if err != nil {
printf(
"warning: list override key %s.%s is invalid - %s",
subPrefix, key, err)
continue
}
baseKeyListLen := len(baseKeyList)
if baseKeyListLen-1 < overrideAtIdx {
printf(
"warning: list override key %s.%s is invalid because provided index is too big for base %s list length of %d",
subPrefix, key, listKeyName, baseKeyListLen)
continue
}
// we do not merge nested maps within list overrides because `coalesceValues` does not have a direct recursive call
// avoid mutating
destination := make([]interface{}, len(baseKeyList))
copy(destination, baseKeyList)
destination[overrideAtIdx] = val
v[listKeyName] = destination
}
}
// CoalesceTables merges a source map into a destination map.
//
// dest is considered authoritative.

@ -393,6 +393,199 @@ func TestMergeValues(t *testing.T) {
is.Equal(valsCopy, vals)
}
func TestMergeValuesListOverride(t *testing.T) {
is := assert.New(t)
expectedNonListOutput := map[string]interface{}{
"testing": "notAList",
}
var validListOverrideValues = []byte(`
"testing[0]":
foo: bar
these: values
will: override
`)
expectedValidListOverrideOutput := map[string]interface{}{
"testing": []interface{}{
map[string]interface{}{
"foo": "bar",
"these": "values",
"will": "override",
},
"itemTwo",
"itemThree",
},
}
var outOfBoundsOverrideValues = []byte(`
testing[10]:
foo: bar
these: values
would: "override on valid index"
`)
var malformedOverrideValues = []byte(`
testing[]:
foo: bar
these: values
will: override
`)
var conflictingIndexValues = []byte(`
testing[0]:
this: "will be ignored"
testing:
- "This will completely override lower layer"
`)
expectedConflictingIndexOutput := map[string]interface{}{
"testing": []interface{}{
"This will completely override lower layer",
},
}
var recursiveOverrideValues = []byte(`
header:
testing[0]:
override: "occurs"
`)
expectedRecursiveOutput := map[string]interface{}{
"header": map[string]interface{}{
"testing": []interface{}{
map[string]interface{}{"override": "occurs"},
"this one stays",
},
},
}
validListOverrideVals, err := ReadValues(validListOverrideValues)
if err != nil {
t.Fatal(err)
}
standardBaseValues := map[string]interface{}{
"testing": []interface{}{
map[string]interface{}{
"foo": "this will be overridden",
"this": "non-conflicting key will also be gone",
},
"itemTwo",
"itemThree",
},
}
c := &chart.Chart{
Metadata: &chart.Metadata{Name: "spouter"},
Values: standardBaseValues,
IndexOverride: true,
}
noIndexOverrideC := &chart.Chart{
Metadata: &chart.Metadata{Name: "spouter"},
Values: standardBaseValues,
}
// ensure that key override is ignored when flag is off
vals, err := MergeValues(noIndexOverrideC, validListOverrideVals)
if err != nil {
t.Fatal(err)
}
is.Equal(map[string]interface{}{
"testing": []interface{}{
map[string]interface{}{
"foo": "this will be overridden",
"this": "non-conflicting key will also be gone",
},
"itemTwo",
"itemThree",
},
"testing[0]": map[string]interface{}{
"foo": "bar",
"these": "values",
"will": "override",
},
}, vals.AsMap())
vals, err = MergeValues(c, validListOverrideVals)
if err != nil {
t.Fatal(err)
}
is.Equal(expectedValidListOverrideOutput, vals.AsMap())
// index out of bounds for merge
oobListOverrideVals, err := ReadValues(outOfBoundsOverrideValues)
if err != nil {
t.Fatal(err)
}
vals, _ = MergeValues(c, oobListOverrideVals)
is.Equal(standardBaseValues, vals.AsMap()) // should be unchanged
// malformed index
malformedOverideVals, err := ReadValues(malformedOverrideValues)
if err != nil {
t.Fatal(err)
}
vals, _ = MergeValues(c, malformedOverideVals)
is.Equal(standardBaseValues, vals.AsMap()) // should be unchanged
// conflicting list set
conflictingIndexVals, err := ReadValues(conflictingIndexValues)
if err != nil {
t.Fatal(err)
}
vals, _ = MergeValues(c, conflictingIndexVals)
is.Equal(expectedConflictingIndexOutput, vals.AsMap()) // should be unchanged
// no underlying key
noKeyBaseC := &chart.Chart{
Metadata: &chart.Metadata{Name: "nonLister"},
Values: map[string]interface{}{
"other key": "some value",
},
IndexOverride: true,
}
validListOverrideVals, err = ReadValues(validListOverrideValues)
if err != nil {
t.Fatal(err)
}
vals, _ = MergeValues(noKeyBaseC, validListOverrideVals)
is.Equal(map[string]interface{}{
"other key": "some value",
}, vals.AsMap()) // should be unchanged
// merge on top of non-list
nonListBaseC := &chart.Chart{
Metadata: &chart.Metadata{Name: "nonLister"},
Values: map[string]interface{}{
"testing": "notAList",
},
IndexOverride: true,
}
validListOverrideVals, err = ReadValues(validListOverrideValues)
if err != nil {
t.Fatal(err)
}
vals, _ = MergeValues(nonListBaseC, validListOverrideVals)
is.Equal(expectedNonListOutput, vals.AsMap()) // should be unchanged
// recursive list override
recursiveBaseC := &chart.Chart{
Metadata: &chart.Metadata{Name: "nonLister"},
Values: map[string]interface{}{
"header": map[string]interface{}{
"testing": []interface{}{
"this one goes",
"this one stays",
},
},
},
IndexOverride: true,
}
recursiveOverrideVals, err := ReadValues(recursiveOverrideValues)
if err != nil {
t.Fatal(err)
}
vals, _ = MergeValues(recursiveBaseC, recursiveOverrideVals)
is.Equal(expectedRecursiveOutput, vals.AsMap()) // should be unchanged
}
func TestCoalesceTables(t *testing.T) {
dst := map[string]interface{}{
"name": "Ishmael",

@ -149,7 +149,6 @@ Loop:
}
}
c.SetDependencies(chartDependencies...)
// set all to true
for _, lr := range c.Metadata.Dependencies {
lr.Enabled = true

@ -17,6 +17,7 @@ limitations under the License.
package values
import (
"fmt"
"io"
"net/url"
"os"
@ -37,6 +38,7 @@ type Options struct {
FileValues []string // --set-file
JSONValues []string // --set-json
LiteralValues []string // --set-literal
IndexOverride bool
}
// MergeValues merges values from files specified via -f/--values and directly
@ -57,7 +59,9 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er
return nil, errors.Wrapf(err, "failed to parse %s", filePath)
}
// Merge with the previous map
base = mergeMaps(base, currentMap)
if base, err = mergeMaps(base, currentMap, opts.IndexOverride); err != nil {
return nil, errors.Errorf("failed merging values files: %s", err)
}
}
// User specified a value via --set-json
@ -72,6 +76,7 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er
if err := strvals.ParseInto(value, base); err != nil {
return nil, errors.Wrap(err, "failed parsing --set data")
}
}
// User specified a value via --set-string
@ -105,23 +110,62 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er
return base, nil
}
func mergeMaps(a, b map[string]interface{}) map[string]interface{} {
func mergeMaps(a, b map[string]interface{}, indexOverride bool) (map[string]interface{}, error) {
out := make(map[string]interface{}, len(a))
var err error
for k, v := range a {
out[k] = v
}
for k, v := range b {
if v, ok := v.(map[string]interface{}); ok {
listKeyName, t := strvals.FindListRune(k)
if indexOverride && listKeyName != "" { // to allow list overrides - no merging within lists
// ignore list index if there is the plain key in the same list already
if _, ok := b[listKeyName]; ok {
continue
}
overrideIdx, err := t.ParseListIndex()
if err != nil {
return out, fmt.Errorf("invalid key format %s for list override - %s", k, err)
}
// we can't assume that baseValue exists because we still have to merge to the base layer
baseValue, hasBaseValue := out[listKeyName]
var bvList []interface{}
var ok bool
if hasBaseValue {
if bvList, ok = baseValue.([]interface{}); !ok {
return out, fmt.Errorf("invalid key %s - the underlying value in the base layer is not a list", k)
}
}
if bvList == nil {
// if there is no underlying list, preserve the index override for the base values
out[k] = v
} else {
// validate index and list merge operation
if len(bvList)-1 < overrideIdx {
return out, fmt.Errorf("invalid key format %s - index %d does not exist in the destination list", listKeyName, overrideIdx)
}
// copy the list so that we don't mutate the input
destination := make([]interface{}, len(bvList))
copy(destination, bvList)
destination[overrideIdx] = v
out[listKeyName] = destination
continue
}
} else if v, ok := v.(map[string]interface{}); ok {
if bv, ok := out[k]; ok {
if bv, ok := bv.(map[string]interface{}); ok {
out[k] = mergeMaps(bv, v)
out[k], err = mergeMaps(bv, v, indexOverride)
if err != nil {
return out, fmt.Errorf("failed to merge values - %s", err)
}
continue
}
}
}
out[k] = v
}
return out
return out, err
}
// readFile load a file from stdin, the local directory, or a remote file with a url.

@ -44,26 +44,91 @@ func TestMergeValues(t *testing.T) {
anotherFlatMap := map[string]interface{}{
"testing": "fun",
}
nonListKeyMap := map[string]interface{}{
"test": "fun",
}
stringListMap := map[string]interface{}{
"test": []interface{}{
"valueOne", "valueTwo", "valueThree",
},
}
complexListMap := map[string]interface{}{
"test": []interface{}{
map[string]interface{}{
"successful": "value will be gone",
"someKeyNotOverridden": "will also be gone",
},
map[string]interface{}{
"someKey": "someValue",
},
},
}
ignoredStringListOverrideMap := map[string]interface{}{
"test": []interface{}{
"valueFour",
},
"test[1]": "valueFive",
}
invalidStringListOverride := map[string]interface{}{
"test[7]": "invalidOverride",
}
invalidStringListFormat := map[string]interface{}{
"test[]": "invalidOverride",
}
validStringListOverride := map[string]interface{}{
"test[1]": "newValue",
}
testMap := mergeMaps(flatMap, nestedMap)
validMapListOverride := map[string]interface{}{
"test[0]": map[string]interface{}{
"nested": "values",
"successful": "override",
},
}
recursiveListMap := map[string]interface{}{
"header": map[string]interface{}{
"testing": []interface{}{
map[string]interface{}{"override": "occurs"},
"this one stays",
},
},
}
recursiveListOverride := map[string]interface{}{
"header": map[string]interface{}{
"testing[0]": "time to override",
},
}
expectedRecursiveListOverride := map[string]interface{}{
"header": map[string]interface{}{
"testing": []interface{}{
"time to override",
"this one stays",
},
},
}
testMap, _ := mergeMaps(flatMap, nestedMap, false)
equal := reflect.DeepEqual(testMap, nestedMap)
if !equal {
t.Errorf("Expected a nested map to overwrite a flat value. Expected: %v, got %v", nestedMap, testMap)
}
testMap = mergeMaps(nestedMap, flatMap)
testMap, _ = mergeMaps(nestedMap, flatMap, false)
equal = reflect.DeepEqual(testMap, flatMap)
if !equal {
t.Errorf("Expected a flat value to overwrite a map. Expected: %v, got %v", flatMap, testMap)
}
testMap = mergeMaps(nestedMap, anotherNestedMap)
testMap, _ = mergeMaps(nestedMap, anotherNestedMap, false)
equal = reflect.DeepEqual(testMap, anotherNestedMap)
if !equal {
t.Errorf("Expected a nested map to overwrite another nested map. Expected: %v, got %v", anotherNestedMap, testMap)
}
testMap = mergeMaps(anotherFlatMap, anotherNestedMap)
testMap, _ = mergeMaps(anotherFlatMap, anotherNestedMap, false)
expectedMap := map[string]interface{}{
"testing": "fun",
"foo": "bar",
@ -76,6 +141,96 @@ func TestMergeValues(t *testing.T) {
if !equal {
t.Errorf("Expected a map with different keys to merge properly with another map. Expected: %v, got %v", expectedMap, testMap)
}
testMap, _ = mergeMaps(stringListMap, ignoredStringListOverrideMap, true)
expectedMap = map[string]interface{}{
"test": []interface{}{
"valueFour",
},
}
equal = reflect.DeepEqual(testMap, expectedMap)
if !equal {
t.Errorf("Expected an index key to be ignored when passed in with matching list key. Expected %v, got %v", expectedMap, testMap)
}
_, err := mergeMaps(stringListMap, invalidStringListOverride, true)
if err == nil || err.Error() != "invalid key format test - index 7 does not exist in the destination list" {
t.Errorf("Expected error for invalid list override index")
}
_, err = mergeMaps(stringListMap, invalidStringListFormat, true)
if err == nil || err.Error() != "invalid key format test[] for list override - failed to find a valid index" {
t.Errorf("Expected error for invalid key format")
}
_, err = mergeMaps(nonListKeyMap, validStringListOverride, true)
if err == nil || err.Error() != "invalid key test[1] - the underlying value in the base layer is not a list" {
t.Errorf("Expected error for invalid type of destination")
}
testMap, _ = mergeMaps(stringListMap, validStringListOverride, true)
expectedMap = map[string]interface{}{
"test": []interface{}{
"valueOne", "newValue", "valueThree",
},
}
equal = reflect.DeepEqual(testMap, expectedMap)
if !equal {
t.Errorf("Expected index 1 to be overridden with string. Expected %v, got %v", expectedMap, testMap)
}
testMap, _ = mergeMaps(stringListMap, validMapListOverride, true)
expectedMap = map[string]interface{}{
"test": []interface{}{
map[string]interface{}{
"nested": "values",
"successful": "override",
},
"valueTwo",
"valueThree",
},
}
equal = reflect.DeepEqual(testMap, expectedMap)
if !equal {
t.Errorf("Expected index 0 to be overridden with map. Expected %v, got %v", expectedMap, testMap)
}
testMap, _ = mergeMaps(complexListMap, validMapListOverride, true)
expectedMap = map[string]interface{}{
"test": []interface{}{
map[string]interface{}{
// no merge of nested keys
"nested": "values",
"successful": "override",
},
map[string]interface{}{
"someKey": "someValue",
},
},
}
equal = reflect.DeepEqual(testMap, expectedMap)
if !equal {
t.Errorf("Expected map at index 0 to be overridden without merge. Expected %v, got %v", expectedMap, testMap)
}
testMap, _ = mergeMaps(recursiveListMap, recursiveListOverride, true)
equal = reflect.DeepEqual(testMap, expectedRecursiveListOverride)
if !equal {
t.Errorf("Expected recursive list at index 0 to be overridden. Expected %v, got %v", expectedRecursiveListOverride, testMap)
}
testMap, _ = mergeMaps(stringListMap, validStringListOverride, false)
expectedMap = map[string]interface{}{
"test": []interface{}{
"valueOne", "valueTwo", "valueThree",
},
"test[1]": "newValue",
}
equal = reflect.DeepEqual(testMap, expectedMap)
if !equal {
t.Errorf("Expected index 1 override to be ignored because of flag disable. Expected %v, got %v", expectedMap, testMap)
}
}
func TestReadFile(t *testing.T) {

@ -128,34 +128,54 @@ func ParseIntoFile(s string, dest map[string]interface{}, reader RunesValueReade
// and returns the parsed value
type RunesValueReader func([]rune) (interface{}, error)
// parser is a simple parser that takes a strvals line and parses it into a
// Parser is a simple Parser that takes a strvals line and parses it into a
// map representation.
//
// 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 {
type Parser struct {
sc *bytes.Buffer
data map[string]interface{}
reader RunesValueReader
isjsonval bool
}
func newParser(sc *bytes.Buffer, data map[string]interface{}, stringBool bool) *parser {
func newParser(sc *bytes.Buffer, data map[string]interface{}, stringBool bool) *Parser {
stringConverter := func(rs []rune) (interface{}, error) {
return typedVal(rs, stringBool), nil
}
return &parser{sc: sc, data: data, reader: stringConverter}
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}
// determine whether the provided string has an open bracket for a list rune
func FindListRune(in string) (string, *Parser) {
var t *Parser
found := ""
scanner := bytes.NewBufferString(in)
t = newParser(scanner, map[string]interface{}{}, false)
stop := runeSet([]rune{'['})
// as long as we dont hit eof, we are good. and we've seeked to open bracket
v, _, err := runesUntil(t.sc, stop)
if err == nil {
found = string(v)
}
return found, t
}
func newFileParser(sc *bytes.Buffer, data map[string]interface{}, reader RunesValueReader) *parser {
return &parser{sc: sc, data: data, reader: reader}
func newJSONParser(sc *bytes.Buffer, data map[string]interface{}) *Parser {
return &Parser{sc: sc, data: data, reader: nil, isjsonval: true}
}
func (t *parser) parse() error {
func newFileParser(sc *bytes.Buffer, data map[string]interface{}, reader RunesValueReader) *Parser {
return &Parser{sc: sc, data: data, reader: reader}
}
func (t *Parser) parse() error {
for {
err := t.key(t.data, 0)
if err == nil {
@ -168,6 +188,19 @@ func (t *parser) parse() error {
}
}
// read string index and validate it
func (t *Parser) ParseListIndex() (int, error) {
idx, err := t.keyIndex()
if err != nil {
return 0, errors.New("failed to find a valid index")
}
if idx < 0 {
return 0, errors.New("index cannot be negative")
}
return idx, nil
}
func runeSet(r []rune) map[rune]bool {
s := make(map[rune]bool, len(r))
for _, rr := range r {
@ -176,7 +209,7 @@ func runeSet(r []rune) map[rune]bool {
return s
}
func (t *parser) key(data map[string]interface{}, nestedNameLevel int) (reterr error) {
func (t *Parser) key(data map[string]interface{}, nestedNameLevel int) (reterr error) {
defer func() {
if r := recover(); r != nil {
reterr = fmt.Errorf("unable to parse key: %s", r)
@ -321,7 +354,7 @@ func setIndex(list []interface{}, index int, val interface{}) (l2 []interface{},
return list, nil
}
func (t *parser) keyIndex() (int, error) {
func (t *Parser) keyIndex() (int, error) {
// First, get the key.
stop := runeSet([]rune{']'})
v, _, err := runesUntil(t.sc, stop)
@ -332,7 +365,7 @@ func (t *parser) keyIndex() (int, error) {
return strconv.Atoi(string(v))
}
func (t *parser) listItem(list []interface{}, i, nestedNameLevel int) ([]interface{}, error) {
func (t *Parser) listItem(list []interface{}, i, nestedNameLevel int) ([]interface{}, error) {
if i < 0 {
return list, fmt.Errorf("negative %d index not allowed", i)
}
@ -437,7 +470,7 @@ func (t *parser) listItem(list []interface{}, i, nestedNameLevel int) ([]interfa
// 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 consumed
func (t *parser) emptyVal() (bool, error) {
func (t *Parser) emptyVal() (bool, error) {
for {
r, _, e := t.sc.ReadRune()
if e == io.EOF {
@ -456,13 +489,13 @@ func (t *parser) emptyVal() (bool, error) {
}
}
func (t *parser) val() ([]rune, error) {
func (t *Parser) val() ([]rune, error) {
stop := runeSet([]rune{','})
v, _, err := runesUntil(t.sc, stop)
return v, err
}
func (t *parser) valList() ([]interface{}, error) {
func (t *Parser) valList() ([]interface{}, error) {
r, _, e := t.sc.ReadRune()
if e != nil {
return []interface{}{}, e

Loading…
Cancel
Save