Fix panic related to env values from files and sprig dict funcs (#625)

* fix: 0.68.0: go panic w/ multiple bases/layers

Fixes #623

* fix: 0.64.1+: getOrNil/hasKey fails on Environment.Values with nested maps

Fixes #624

environment/environment.go

@@ -1,8 +1,9 @@
 package environment
 
 import (
-	"encoding/json"
 	"github.com/imdario/mergo"
+	"github.com/roboll/helmfile/pkg/maputil"
+	"gopkg.in/yaml.v2"
 )
 
 type Environment struct {
@@ -13,12 +14,16 @@ type Environment struct {
 var EmptyEnvironment Environment
 
 func (e Environment) DeepCopy() Environment {
-	bytes, err := json.Marshal(e.Values)
+	bytes, err := yaml.Marshal(e.Values)
 	if err != nil {
 		panic(err)
 	}
 	var values map[string]interface{}
-	if err := json.Unmarshal(bytes, &values); err != nil {
+	if err := yaml.Unmarshal(bytes, &values); err != nil {
+		panic(err)
+	}
+	values, err = maputil.CastKeysToStrings(values)
+	if err != nil {
 		panic(err)
 	}
 	return Environment{

pkg/app/app_test.go

@@ -1356,3 +1356,138 @@ releases:
 		t.Errorf("unexpected releases[0].chart: expected=BAR, got=%s", st.Releases[0].Chart)
 	}
 }
+
+// See https://github.com/roboll/helmfile/issues/623
+func TestLoadDesiredStateFromYaml_MultiPartTemplate_MergeMapsVariousKeys(t *testing.T) {
+	type testcase struct {
+		overrideValues interface{}
+		expected       string
+	}
+	testcases := []testcase{
+		{map[interface{}]interface{}{"foo": "FOO"}, `FOO`},
+		{map[interface{}]interface{}{"foo": map[interface{}]interface{}{"foo": "FOO"}}, `map[foo:FOO]`},
+		{map[interface{}]interface{}{"foo": map[string]interface{}{"foo": "FOO"}}, `map[foo:FOO]`},
+		{map[interface{}]interface{}{"foo": []interface{}{"foo"}}, `[foo]`},
+		{map[interface{}]interface{}{"foo": "FOO"}, `FOO`},
+	}
+	for i := range testcases {
+		tc := testcases[i]
+		statePath := "/path/to/helmfile.yaml"
+		stateContent := `
+environments:
+  default:
+    values:
+    - 1.yaml
+    - 2.yaml
+---
+releases:
+- name: {{ .Environment.Values.foo | quote }}
+  chart: {{ .Environment.Values.bar | quote }}
+`
+		testFs := state.NewTestFs(map[string]string{
+			statePath:         stateContent,
+			"/path/to/1.yaml": `bar: ["bar"]`,
+			"/path/to/2.yaml": `bar: ["BAR"]`,
+		})
+		app := &App{
+			readFile: testFs.ReadFile,
+			glob:     testFs.Glob,
+			abs:      testFs.Abs,
+			Env:      "default",
+			Logger:   helmexec.NewLogger(os.Stderr, "debug"),
+			Reverse:  true,
+		}
+		opts := LoadOpts{
+			CalleePath: statePath,
+			Environment: state.SubhelmfileEnvironmentSpec{
+				OverrideValues: []interface{}{tc.overrideValues},
+			},
+		}
+		st, err := app.loadDesiredStateFromYaml(statePath, opts)
+
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		if st.Releases[0].Name != tc.expected {
+			t.Errorf("unexpected releases[0].name: expected=%s, got=%s", tc.expected, st.Releases[0].Name)
+		}
+		if st.Releases[0].Chart != "[BAR]" {
+			t.Errorf("unexpected releases[0].chart: expected=BAR, got=%s", st.Releases[0].Chart)
+		}
+	}
+}
+
+func TestLoadDesiredStateFromYaml_MultiPartTemplate_SprigDictFuncs(t *testing.T) {
+	type testcase struct {
+		state    string
+		expr     string
+		expected string
+	}
+	stateInline := `
+environments:
+  default:
+    values:
+    - foo: FOO
+      bar: { "baz": "BAZ" }
+---
+releases:
+- name: %s
+  chart: stable/nginx
+`
+	stateExternal := `
+environments:
+  default:
+    values:
+    - 1.yaml
+    - 2.yaml
+---
+releases:
+- name: %s
+  chart: stable/nginx
+`
+	testcases := []testcase{
+		{stateInline, `{{ getOrNil "foo" .Environment.Values }}`, `FOO`},
+		{stateInline, `{{ getOrNil "baz" (getOrNil "bar" .Environment.Values) }}`, `BAZ`},
+		{stateInline, `{{ if hasKey .Environment.Values "foo" }}{{ .Environment.Values.foo }}{{ end }}`, `FOO`},
+		{stateInline, `{{ if hasKey .Environment.Values "bar" }}{{ .Environment.Values.bar.baz }}{{ end }}`, `BAZ`},
+		{stateInline, `{{ if (keys .Environment.Values | has "foo") }}{{ .Environment.Values.foo }}{{ end }}`, `FOO`},
+		// See https://github.com/roboll/helmfile/issues/624
+		// This fails when .Environment.Values.bar is not map[string]interface{}. At the time of #624 it was map[interface{}]interface{}, which sprig's dict funcs don't support.
+		{stateInline, `{{ if (keys .Environment.Values | has "bar") }}{{ if (keys .Environment.Values.bar | has "baz") }}{{ .Environment.Values.bar.baz }}{{ end }}{{ end }}`, `BAZ`},
+		{stateExternal, `{{ getOrNil "foo" .Environment.Values }}`, `FOO`},
+		{stateExternal, `{{ getOrNil "baz" (getOrNil "bar" .Environment.Values) }}`, `BAZ`},
+		{stateExternal, `{{ if hasKey .Environment.Values "foo" }}{{ .Environment.Values.foo }}{{ end }}`, `FOO`},
+		{stateExternal, `{{ if hasKey .Environment.Values "bar" }}{{ .Environment.Values.bar.baz }}{{ end }}`, `BAZ`},
+		{stateExternal, `{{ if (keys .Environment.Values | has "foo") }}{{ .Environment.Values.foo }}{{ end }}`, `FOO`},
+		// See https://github.com/roboll/helmfile/issues/624
+		{stateExternal, `{{ if (keys .Environment.Values | has "bar") }}{{ if (keys .Environment.Values.bar | has "baz") }}{{ .Environment.Values.bar.baz }}{{ end }}{{ end }}`, `BAZ`},
+	}
+	for i := range testcases {
+		tc := testcases[i]
+		statePath := "/path/to/helmfile.yaml"
+		stateContent := fmt.Sprintf(tc.state, tc.expr)
+		testFs := state.NewTestFs(map[string]string{
+			statePath:         stateContent,
+			"/path/to/1.yaml": `foo: FOO`,
+			"/path/to/2.yaml": `bar: { "baz": "BAZ" }`,
+		})
+		app := &App{
+			readFile: testFs.ReadFile,
+			glob:     testFs.Glob,
+			abs:      testFs.Abs,
+			Env:      "default",
+			Logger:   helmexec.NewLogger(os.Stderr, "debug"),
+			Reverse:  true,
+		}
+		st, err := app.loadDesiredStateFromYaml(statePath)
+
+		if err != nil {
+			t.Fatalf("unexpected error at %d: %v", i, err)
+		}
+
+		if st.Releases[0].Name != tc.expected {
+			t.Errorf("unexpected releases[0].name at %d: expected=%s, got=%s", i, tc.expected, st.Releases[0].Name)
+		}
+	}
+}

pkg/app/two_pass_renderer.go

@@ -80,7 +80,7 @@ func (r *desiredStateLoader) twoPassRenderTemplateToYaml(inherited, overrode *en
 	renderedEnv := r.renderEnvironment(initEnv, baseDir, filename, content)
 
 	if r.logger != nil {
-		r.logger.Debugf("first-pass produced: %v", initEnv)
+		r.logger.Debugf("first-pass produced: %v", renderedEnv)
 	}
 
 	finalEnv, err := renderedEnv.Merge(overrode)

pkg/maputil/maputil.go

@@ -0,0 +1,50 @@
+package maputil
+
+import "fmt"
+
+func CastKeysToStrings(s interface{}) (map[string]interface{}, error) {
+	new := map[string]interface{}{}
+	switch src := s.(type) {
+	case map[interface{}]interface{}:
+		for k, v := range src {
+			var str_k string
+			switch typed_k := k.(type) {
+			case string:
+				str_k = typed_k
+			default:
+				return nil, fmt.Errorf("unexpected type of key in map: expected string, got %T: value=%v, map=%v", typed_k, typed_k, src)
+			}
+
+			var casted_v interface{}
+			switch typed_v := v.(type) {
+			case map[interface{}]interface{}:
+				tmp, err := CastKeysToStrings(typed_v)
+				if err != nil {
+					return nil, err
+				}
+				casted_v = tmp
+			default:
+				casted_v = typed_v
+			}
+
+			new[str_k] = casted_v
+		}
+	case map[string]interface{}:
+		for k, v := range src {
+			var casted_v interface{}
+			switch typed_v := v.(type) {
+			case map[interface{}]interface{}:
+				tmp, err := CastKeysToStrings(typed_v)
+				if err != nil {
+					return nil, err
+				}
+				casted_v = tmp
+			default:
+				casted_v = typed_v
+			}
+
+			new[k] = casted_v
+		}
+	}
+	return new, nil
+}

state/environment_values_loader.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"github.com/imdario/mergo"
 	"github.com/roboll/helmfile/environment"
+	"github.com/roboll/helmfile/pkg/maputil"
 	"github.com/roboll/helmfile/tmpl"
 	"gopkg.in/yaml.v2"
 	"path/filepath"
@@ -53,21 +54,16 @@ func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *str
 				}
 			}
 		case map[interface{}]interface{}:
-			m := map[string]interface{}{}
+			m, err := maputil.CastKeysToStrings(typedValue)
-			for k, v := range typedValue {
+			if err != nil {
-				switch typedKey := k.(type) {
+				return nil, err
-				case string:
-					m[typedKey] = v
-				default:
-					return nil, fmt.Errorf("unexpected type of key in inline environment values %v: expected string, got %T", typedValue, typedKey)
-				}
 			}
 			if err := mergo.Merge(&envVals, &m, mergo.WithOverride); err != nil {
 				return nil, fmt.Errorf("failed to merge %v: %v", typedValue, err)
 			}
 			continue
 		default:
-			return nil, fmt.Errorf("unexpected type of values entry: %T", typedValue)
+			return nil, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue)
 		}
 	}
 

state/state.go

@@ -1319,7 +1319,7 @@ func (st *HelmState) generateTemporaryValuesFiles(values []interface{}, missingF
 			}
 			generatedFiles = append(generatedFiles, valfile.Name())
 		default:
-			return nil, fmt.Errorf("unexpected type of values entry: %T", typedValue)
+			return nil, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue)
 		}
 	}
 	return generatedFiles, nil