From 71bb7354e7f0e65f5e0140b76fc843b4d852e273 Mon Sep 17 00:00:00 2001
From: Danny Shemesh <dany74q@gmail.com>
Date: Fri, 17 Apr 2020 03:18:01 +0300
Subject: [PATCH] Fix: populate .Values regardless of prestate success (#1202)

This commit proposes a potential solution for
https://github.com/roboll/helmfile/issues/1201

The gist is that, if prestate rendering fails, for any reason,
we do not populate the .Values in the second pass renderer.

I think that what have been expected in this case is to populate the
.Values irregardless.

pkg/app/two_pass_renderer.go
- Migrated to use finalEnv.GetMergedValues()

pkg/environment/environment.go
- Introduced GetMergedValues, which merges the environment's defaults
and current values, and then casts the keys to string;
This was previously defined in HelmState.Values() - however, as this
method is only concerned with the environment, I think it's more
appropriate for it to sit here.

pkg/state/state_exec_tmpl.go
- Extracted out HelmState.Values() to environment.go, see above
---
 pkg/app/two_pass_renderer.go   | 12 +++++-------
 pkg/environment/environment.go | 19 +++++++++++++++++++
 pkg/state/state_exec_tmpl.go   | 18 +-----------------
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/pkg/app/two_pass_renderer.go b/pkg/app/two_pass_renderer.go
index 0907b496..2b927ec4 100644
--- a/pkg/app/two_pass_renderer.go
+++ b/pkg/app/two_pass_renderer.go
@@ -116,15 +116,13 @@ func (r *desiredStateLoader) twoPassRenderTemplateToYaml(inherited, overrode *en
 		r.logger.Debugf("first-pass rendering result of \"%s\": %v", filename, *finalEnv)
 	}
 
-	vals := map[string]interface{}{}
+	vals, err := finalEnv.GetMergedValues()
+	if err != nil {
+		return nil, err
+	}
+
 	if prestate != nil {
 		prestate.Env = *finalEnv
-		vals, err = prestate.Values()
-		if err != nil {
-			return nil, err
-		}
-	}
-	if prestate != nil {
 		r.logger.Debugf("vals:\n%v\ndefaultVals:%v", vals, prestate.DefaultValues)
 	}
 
diff --git a/pkg/environment/environment.go b/pkg/environment/environment.go
index ba95213f..29579ed0 100644
--- a/pkg/environment/environment.go
+++ b/pkg/environment/environment.go
@@ -64,3 +64,22 @@ func (e *Environment) Merge(other *Environment) (*Environment, error) {
 	}
 	return &copy, nil
 }
+
+func (e *Environment) GetMergedValues() (map[string]interface{}, error) {
+	vals := map[string]interface{}{}
+
+	if err := mergo.Merge(&vals, e.Defaults, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
+		return nil, err
+	}
+
+	if err := mergo.Merge(&vals, e.Values, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
+		return nil, err
+	}
+
+	vals, err := maputil.CastKeysToStrings(vals)
+	if err != nil {
+		return nil, err
+	}
+
+	return vals, nil
+}
diff --git a/pkg/state/state_exec_tmpl.go b/pkg/state/state_exec_tmpl.go
index aa590ec5..68f0af7f 100644
--- a/pkg/state/state_exec_tmpl.go
+++ b/pkg/state/state_exec_tmpl.go
@@ -4,28 +4,12 @@ import (
 	"fmt"
 	"reflect"
 
-	"github.com/imdario/mergo"
-	"github.com/roboll/helmfile/pkg/maputil"
 	"github.com/roboll/helmfile/pkg/tmpl"
 	"gopkg.in/yaml.v2"
 )
 
 func (st *HelmState) Values() (map[string]interface{}, error) {
-	vals := map[string]interface{}{}
-
-	if err := mergo.Merge(&vals, st.Env.Defaults, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
-		return nil, err
-	}
-	if err := mergo.Merge(&vals, st.Env.Values, mergo.WithOverride, mergo.WithOverwriteWithEmptyValue); err != nil {
-		return nil, err
-	}
-
-	vals, err := maputil.CastKeysToStrings(vals)
-	if err != nil {
-		return nil, err
-	}
-
-	return vals, nil
+	return st.Env.GetMergedValues()
 }
 
 func (st *HelmState) mustLoadVals() map[string]interface{} {