fix: --state-values-set unable to set booleans (#1199)

This pr fixes auto-wrapping of booleans and integers into quotes when using --state-values-set by:

- Adding: --state-values-set-string flag for intentional string set of boolean or integer
- Changing: --state-values-set flag not wrapping now
- Removing -

Resolves https://github.com/roboll/helmfile/issues/1347

Signed-off-by: Tunahan Sezen <sezentunahan@outlook.com>

cmd/root.go

@@ -119,6 +119,7 @@ func setGlobalOptionsForRootCmd(fs *pflag.FlagSet, globalOptions *config.GlobalO
 	fs.StringVarP(&globalOptions.File, "file", "f", "", "load config from file or directory. defaults to \"`helmfile.yaml`\" or \"helmfile.yaml.gotmpl\" or \"helmfile.d\" (means \"helmfile.d/*.yaml\" or \"helmfile.d/*.yaml.gotmpl\") in this preference. Specify - to load the config from the standard input.")
 	fs.StringVarP(&globalOptions.Environment, "environment", "e", "", `specify the environment name. Overrides "HELMFILE_ENVIRONMENT" OS environment variable when specified. defaults to "default"`)
 	fs.StringArrayVar(&globalOptions.StateValuesSet, "state-values-set", nil, "set state values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2). Used to override .Values within the helmfile template (not values template).")
+	fs.StringArrayVar(&globalOptions.StateValuesSetString, "state-values-set-string", nil, "set state STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2). Used to override .Values within the helmfile template (not values template).")
 	fs.StringArrayVar(&globalOptions.StateValuesFile, "state-values-file", nil, "specify state values in a YAML file. Used to override .Values within the helmfile template (not values template).")
 	fs.BoolVar(&globalOptions.SkipDeps, "skip-deps", false, `skip running "helm repo update" and "helm dependency build"`)
 	fs.BoolVar(&globalOptions.StripArgsValuesOnExitError, "strip-args-values-on-exit-error", true, `Strip the potential secret values of the helm command args contained in a helmfile error message`)

pkg/config/config.go

@@ -7,7 +7,7 @@ import (
 )
 
 func NewCLIConfigImpl(g *GlobalImpl) error {
-	optsSet := g.RawStateValuesSet()
+	optsSet := g.RawStateValuesSetString()
 	if len(optsSet) > 0 {
 		set := map[string]any{}
 		for i := range optsSet {
@@ -17,7 +17,22 @@ func NewCLIConfigImpl(g *GlobalImpl) error {
 				k := maputil.ParseKey(op[0])
 				v := op[1]
 
-				maputil.Set(set, k, v)
+				maputil.Set(set, k, v, true)
+			}
+		}
+		g.SetSet(set)
+	}
+	optsSet = g.RawStateValuesSet()
+	if len(optsSet) > 0 {
+		set := map[string]any{}
+		for i := range optsSet {
+			ops := strings.Split(optsSet[i], ",")
+			for j := range ops {
+				op := strings.SplitN(ops[j], "=", 2)
+				k := maputil.ParseKey(op[0])
+				v := op[1]
+
+				maputil.Set(set, k, v, false)
 			}
 		}
 		g.SetSet(set)

pkg/config/global.go

@@ -8,6 +8,7 @@ import (
 	"go.uber.org/zap"
 	"golang.org/x/term"
 
+	"github.com/helmfile/helmfile/pkg/maputil"
 	"github.com/helmfile/helmfile/pkg/state"
 )
 
@@ -23,6 +24,8 @@ type GlobalOptions struct {
 	Environment string
 	// StateValuesSet is a list of state values to set on the command line.
 	StateValuesSet []string
+	// StateValuesSetString is a list of state values to set on the command line.
+	StateValuesSetString []string
 	// StateValuesFiles is a list of state values files to use.
 	StateValuesFile []string
 	// SkipDeps is true if the running "helm repo update" and "helm dependency build" should be skipped
@@ -87,7 +90,7 @@ func NewGlobalImpl(opts *GlobalOptions) *GlobalImpl {
 
 // Setset sets the set
 func (g *GlobalImpl) SetSet(set map[string]any) {
-	g.set = set
+	g.set = maputil.MergeMaps(g.set, set)
 }
 
 // HelmBinary returns the path to the Helm binary.
@@ -135,6 +138,11 @@ func (g *GlobalImpl) RawStateValuesSet() []string {
 	return g.GlobalOptions.StateValuesSet
 }
 
+// RawStateValuesSetString returns the set
+func (g *GlobalImpl) RawStateValuesSetString() []string {
+	return g.GlobalOptions.StateValuesSetString
+}
+
 // StateValuesFiles returns the state values files
 func (g *GlobalImpl) StateValuesFiles() []string {
 	return g.GlobalOptions.StateValuesFile

pkg/maputil/maputil.go

@@ -2,6 +2,7 @@ package maputil
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 )
 
@@ -65,7 +66,7 @@ func recursivelyStringifyMapKey(v any) (any, error) {
 
 type arg interface {
 	getMap(map[string]any) map[string]any
-	set(map[string]any, string)
+	set(map[string]any, any)
 }
 
 type keyArg struct {
@@ -85,7 +86,7 @@ func (a keyArg) getMap(m map[string]any) map[string]any {
 	}
 }
 
-func (a keyArg) set(m map[string]any, value string) {
+func (a keyArg) set(m map[string]any, value any) {
 	m[a.key] = value
 }
 
@@ -125,7 +126,7 @@ func (a indexedKeyArg) getMap(m map[string]any) map[string]any {
 	}
 }
 
-func (a indexedKeyArg) set(m map[string]any, value string) {
+func (a indexedKeyArg) set(m map[string]any, value any) {
 	t := a.getArray(m)
 	t[a.index] = value
 	m[a.key] = t
@@ -178,7 +179,7 @@ func ParseKey(key string) []string {
 	return r
 }
 
-func Set(m map[string]any, key []string, value string) {
+func Set(m map[string]any, key []string, value string, stringBool bool) {
 	if len(key) == 0 {
 		panic(fmt.Errorf("bug: unexpected length of key: %d", len(key)))
 	}
@@ -187,5 +188,59 @@ func Set(m map[string]any, key []string, value string) {
 		m, key = getCursor(key[0]).getMap(m), key[1:]
 	}
 
-	getCursor(key[0]).set(m, value)
+	getCursor(key[0]).set(m, typedVal(value, stringBool))
+}
+
+func typedVal(val string, st bool) any {
+	// if st is true, directly return it without casting it
+	if st {
+		return val
+	}
+
+	if strings.EqualFold(val, "true") {
+		return true
+	}
+
+	if strings.EqualFold(val, "false") {
+		return false
+	}
+
+	if strings.EqualFold(val, "null") {
+		return nil
+	}
+
+	// handling of only zero, if val has zero prefix, it will be considered as string
+	if strings.EqualFold(val, "0") {
+		return int64(0)
+	}
+
+	// If this value does not start with zero, try parsing it to an int
+	if len(val) != 0 && val[0] != '0' {
+		if iv, err := strconv.ParseInt(val, 10, 64); err == nil {
+			return iv
+		}
+	}
+
+	return val
+}
+
+func MergeMaps(a, b map[string]interface{}) map[string]interface{} {
+	out := make(map[string]interface{}, len(a))
+	// fill the out map with the first map
+	for k, v := range a {
+		out[k] = v
+	}
+	for k, v := range b {
+		if v, ok := v.(map[string]interface{}); ok {
+			if bv, ok := out[k]; ok {
+				if bv, ok := bv.(map[string]interface{}); ok {
+					// if b and out map has a map value, merge it too
+					out[k] = MergeMaps(bv, v)
+					continue
+				}
+			}
+		}
+		out[k] = v
+	}
+	return out
 }

pkg/maputil/maputil_test.go

@@ -69,7 +69,7 @@ func TestMapUtil_KeyArg(t *testing.T) {
 
 	key := []string{"a", "b", "c"}
 
-	Set(m, key, "C")
+	Set(m, key, "C", false)
 
 	c := (((m["a"].(map[string]any))["b"]).(map[string]any))["c"]
 
@@ -83,7 +83,7 @@ func TestMapUtil_IndexedKeyArg(t *testing.T) {
 
 	key := []string{"a", "b[0]", "c"}
 
-	Set(m, key, "C")
+	Set(m, key, "C", false)
 
 	c := (((m["a"].(map[string]any))["b"].([]any))[0].(map[string]any))["c"]
 
@@ -124,7 +124,7 @@ func TestMapUtil_IndexedKeyArg2(t *testing.T) {
 					k := ParseKey(op[0])
 					v := op[1]
 
-					Set(set, k, v)
+					Set(set, k, v, false)
 				}
 			}
 			if !reflect.DeepEqual(set, c.want) {
@@ -174,3 +174,77 @@ func TestMapUtil_ParseKey(t *testing.T) {
 		}
 	}
 }
+
+func TestMapUtil_typedVal(t *testing.T) {
+	typedValueTest(t, "true", true)
+	typedValueTest(t, "null", nil)
+	typedValueTest(t, "0", int64(0))
+	typedValueTest(t, "5", int64(5))
+	typedValueTest(t, "05", "05")
+}
+
+func typedValueTest(t *testing.T, input string, expectedWhenNoStr any) {
+	returnValue := typedVal(input, true)
+	if returnValue != input {
+		t.Errorf("unexpected typed value: expected=%s, got=%s", input, returnValue)
+	}
+	returnValue = typedVal(input, false)
+	if returnValue != expectedWhenNoStr {
+		t.Errorf("unexpected typed value: expected=%s, got=%s", input, returnValue)
+	}
+}
+
+func TestMapUtil_MergeMaps(t *testing.T) {
+	map1 := map[string]interface{}{
+		"debug": true,
+	}
+	map2 := map[string]interface{}{
+		"logLevel":     "info",
+		"replicaCount": 3,
+	}
+	map3 := map[string]interface{}{
+		"logLevel": "info",
+		"replicaCount": map[string]any{
+			"app1":    3,
+			"awesome": 4,
+		},
+	}
+	map4 := map[string]interface{}{
+		"logLevel": "info",
+		"replicaCount": map[string]any{
+			"app1": 3,
+		},
+	}
+
+	testMap := MergeMaps(map2, map4)
+	equal := reflect.DeepEqual(testMap, map4)
+	if !equal {
+		t.Errorf("Expected a nested map to overwrite a flat value. Expected: %v, got %v", map4, testMap)
+	}
+
+	testMap = MergeMaps(map4, map2)
+	equal = reflect.DeepEqual(testMap, map2)
+	if !equal {
+		t.Errorf("Expected a flat value to overwrite a map. Expected: %v, got %v", map2, testMap)
+	}
+
+	testMap = MergeMaps(map4, map3)
+	equal = reflect.DeepEqual(testMap, map3)
+	if !equal {
+		t.Errorf("Expected a nested map to overwrite another nested map. Expected: %v, got %v", map3, testMap)
+	}
+
+	testMap = MergeMaps(map1, map3)
+	expectedMap := map[string]interface{}{
+		"debug":    true,
+		"logLevel": "info",
+		"replicaCount": map[string]any{
+			"app1":    3,
+			"awesome": 4,
+		},
+	}
+	equal = reflect.DeepEqual(testMap, expectedMap)
+	if !equal {
+		t.Errorf("Expected a map with different keys to merge properly with another map. Expected: %v, got %v", expectedMap, testMap)
+	}
+}