Repository: roboll/helmfile
Pull request #938 (2019-11-06 09:20:21)

Allow combining valuesTemplate and values, same for setTemplate
Right now, one has to use either `valuesTemplate` or `values`. This is unexpected, it would be much more natural to allow using both at the same time. This implements that. The same for `setTemplate` and `set`.

mumoshu: @jdemeyer Thanks! What was your use-case? I have never had a chance to use `values` and `valuesTemplate` at the same time.
jdemeyer: I made the requested changes.

The use case is just that we have charts which take many values. Some of these have a fixed structure and can be defined in the template (as explained in https://github.com/roboll/helmfile/blob/master/docs/writing-helmfile.md), but some values really need to customized on a case-by-case basis and cannot be templated. So I would use `valuesTemplate` for the former and `values` for the latter. With this PR, this works as expected.
mumoshu: @jdemeyer Thanks! LGTM overall.

I'm currently working hard to stabilize the `needs`/DAG feature I introduced recently. So I'd be glad if you could give me a day or two until this gets merged.

Until then, I'd appreciate it if you could add some test to ensure this works and not regress.
Fradulentus: Всем привет.

Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 6bcee07..f9bac9f 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -136,7 +136,7 @@ jobs:
     - run:
         name: Install helm
         environment:
-          HELM_VERSION: v3.0.0-rc.2
+          HELM_VERSION: v3.0.0
         command: |
           HELM_FILENAME="helm-${HELM_VERSION}-linux-amd64.tar.gz"
           curl -Lo ${HELM_FILENAME} "https://get.helm.sh/${HELM_FILENAME}"
diff --git a/Dockerfile.helm3 b/Dockerfile.helm3
index e0846d4..1436b2c 100644
--- a/Dockerfile.helm3
+++ b/Dockerfile.helm3
@@ -11,10 +11,10 @@ FROM alpine:3.10
 
 RUN apk add --no-cache ca-certificates git bash curl jq
 
-ARG HELM_VERSION=v3.0.0-rc.2
+ARG HELM_VERSION=v3.0.0
 ARG HELM_LOCATION="https://get.helm.sh"
 ARG HELM_FILENAME="helm-${HELM_VERSION}-linux-amd64.tar.gz"
-ARG HELM_SHA256="b6fff8e01aa6cd9a4541bd48172bb53b9a0ae38d7e7783a8e0fcc1db63802aaa"
+ARG HELM_SHA256="10e1fdcca263062b1d7b2cb93a924be1ef3dd6c381263d8151dd1a20a3d8c0dc"
 RUN wget ${HELM_LOCATION}/${HELM_FILENAME} && \
     echo Verifying ${HELM_FILENAME}... && \
     sha256sum ${HELM_FILENAME} | grep -q "${HELM_SHA256}" && \
diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go
index 79b4120..67d1029 100644
--- a/pkg/helmexec/exec.go
+++ b/pkg/helmexec/exec.go
@@ -20,6 +20,7 @@ type decryptedSecret struct {
 
 type execer struct {
 	helmBinary           string
+	isHelm3              bool
 	runner               Runner
 	logger               *zap.SugaredLogger
 	kubeContext          string
@@ -45,15 +46,31 @@ func NewLogger(writer io.Writer, logLevel string) *zap.SugaredLogger {
 	return zap.New(core).Sugar()
 }
 
+func detectHelm3(helmBinary string, logger *zap.SugaredLogger, runner Runner) bool {
+	// Support explicit opt-in via environment variable
+	if os.Getenv("HELMFILE_HELM3") != "" {
+		return true
+	}
+
+	// Autodetect from `helm verison`
+	bytes, err := runner.Execute(helmBinary, []string{"version", "--client", "--short"}, nil)
+	if err != nil {
+		panic(err)
+	}
+	return strings.HasPrefix(string(bytes), "v3.")
+}
+
 // New for running helm commands
 func New(helmBinary string, logger *zap.SugaredLogger, kubeContext string, runner Runner) *execer {
 	return &execer{
 		helmBinary:       helmBinary,
+		isHelm3:          detectHelm3(helmBinary, logger, runner),
 		logger:           logger,
 		kubeContext:      kubeContext,
 		runner:           runner,
 		decryptedSecrets: make(map[string]*decryptedSecret),
 	}
+
 }
 
 func (helm *execer) SetExtraArgs(args ...string) {
@@ -126,7 +143,7 @@ func (helm *execer) List(context HelmContext, filter string, flags ...string) (s
 	preArgs := context.GetTillerlessArgs(helm.helmBinary)
 	env := context.getTillerlessEnv()
 	var args []string
-	if helm.isHelm3() {
+	if helm.IsHelm3() {
 		args = []string{"list", "--filter", filter}
 	} else {
 		args = []string{"list", filter}
@@ -139,7 +156,7 @@ func (helm *execer) List(context HelmContext, filter string, flags ...string) (s
 	// of the release to exist.
 	//
 	// This fixes it by removing the header from the v3 output, so that the output is formatted the same as that of v2.
-	if helm.isHelm3() {
+	if helm.IsHelm3() {
 		lines := strings.Split(string(out), "\n")
 		lines = lines[1:]
 		out = []byte(strings.Join(lines, "\n"))
@@ -219,7 +236,7 @@ func (helm *execer) DecryptSecret(context HelmContext, name string, flags ...str
 func (helm *execer) TemplateRelease(name string, chart string, flags ...string) error {
 	helm.logger.Infof("Templating release=%v, chart=%v", name, chart)
 	var args []string
-	if helm.isHelm3() {
+	if helm.IsHelm3() {
 		args = []string{"template", name, chart}
 	} else {
 		args = []string{"template", chart, "--name", name}
@@ -286,7 +303,7 @@ func (helm *execer) TestRelease(context HelmContext, name string, flags ...strin
 	preArgs := context.GetTillerlessArgs(helm.helmBinary)
 	env := context.getTillerlessEnv()
 	var args []string
-	if helm.isHelm3() {
+	if helm.IsHelm3() {
 		args = []string{"test", "run", name}
 	} else {
 		args = []string{"test", name}
@@ -323,6 +340,6 @@ func (helm *execer) write(out []byte) {
 	}
 }
 
-func (helm *execer) isHelm3() bool {
-	return os.Getenv("HELMFILE_HELM3") != ""
+func (helm *execer) IsHelm3() bool {
+	return helm.isHelm3
 }
diff --git a/pkg/helmexec/helmexec.go b/pkg/helmexec/helmexec.go
index 54c3fea..74a40d3 100644
--- a/pkg/helmexec/helmexec.go
+++ b/pkg/helmexec/helmexec.go
@@ -19,8 +19,10 @@ type Interface interface {
 	TestRelease(context HelmContext, name string, flags ...string) error
 	List(context HelmContext, filter string, flags ...string) (string, error)
 	DecryptSecret(context HelmContext, name string, flags ...string) (string, error)
+	IsHelm3() bool
 }
 
 type DependencyUpdater interface {
 	UpdateDeps(chart string) error
+	IsHelm3() bool
 }
diff --git a/pkg/state/chart_dependency.go b/pkg/state/chart_dependency.go
index 467e479..ccd649a 100644
--- a/pkg/state/chart_dependency.go
+++ b/pkg/state/chart_dependency.go
@@ -280,7 +280,7 @@ func (m *chartDependencyManager) lockFileName() string {
 }
 
 func (m *chartDependencyManager) Update(shell helmexec.DependencyUpdater, wd string, unresolved *UnresolvedDependencies) (*ResolvedDependencies, error) {
-	if isHelm3() {
+	if shell.IsHelm3() {
 		return m.updateHelm3(shell, wd, unresolved)
 	}
 	return m.updateHelm2(shell, wd, unresolved)
@@ -330,7 +330,7 @@ func (m *chartDependencyManager) doUpdate(chartLockFile string, unresolved *Unre
 		return nil, err
 	}
 
-	if isHelm3() && originalLockFileContent != nil {
+	if shell.IsHelm3() && originalLockFileContent != nil {
 		if err := m.writeBytes(filepath.Join(wd, chartLockFile), originalLockFileContent); err != nil {
 			return nil, err
 		}
@@ -357,7 +357,7 @@ func (m *chartDependencyManager) doUpdate(chartLockFile string, unresolved *Unre
 	})
 
 	// Don't update lock file if no dependency updated.
-	if !isHelm3() && originalLockFileContent != nil {
+	if !shell.IsHelm3() && originalLockFileContent != nil {
 		originalLockedReqs := &ChartLockedRequirements{}
 		if err := yaml.Unmarshal(originalLockFileContent, originalLockedReqs); err != nil {
 			return nil, err
diff --git a/pkg/state/release.go b/pkg/state/release.go
index 3ea0c24..219fd68 100644
--- a/pkg/state/release.go
+++ b/pkg/state/release.go
@@ -93,7 +93,7 @@ func (r ReleaseSpec) ExecuteTemplateExpressions(renderer *tmpl.FileRenderer) (*R
 		result.Labels[key] = s.String()
 	}
 
-	if result.ValuesTemplate != nil && len(result.ValuesTemplate) > 0 {
+	if len(result.ValuesTemplate) > 0 {
 		for i, t := range result.ValuesTemplate {
 			switch ts := t.(type) {
 			case map[interface{}]interface{}:
@@ -117,7 +117,9 @@ func (r ReleaseSpec) ExecuteTemplateExpressions(renderer *tmpl.FileRenderer) (*R
 			}
 		}
 
-		result.Values = result.ValuesTemplate
+		var newvals []interface{}
+		newvals = append(newvals, result.ValuesTemplate...)
+		result.Values = append(newvals, result.Values...)
 	}
 
 	for i, t := range result.Values {
@@ -139,7 +141,7 @@ func (r ReleaseSpec) ExecuteTemplateExpressions(renderer *tmpl.FileRenderer) (*R
 		result.Secrets[i] = s.String()
 	}
 
-	if result.SetValuesTemplate != nil && len(result.SetValuesTemplate) > 0 {
+	if len(result.SetValuesTemplate) > 0 {
 		for i, val := range result.SetValuesTemplate {
 			{
 				// name
@@ -178,7 +180,9 @@ func (r ReleaseSpec) ExecuteTemplateExpressions(renderer *tmpl.FileRenderer) (*R
 			}
 		}
 
-		result.SetValues = result.SetValuesTemplate
+		var newvals []SetValue
+		newvals = append(newvals, result.SetValuesTemplate...)
+		result.SetValues = append(newvals, result.SetValues...)
 	}
 
 	return result, nil
diff --git a/pkg/state/state.go b/pkg/state/state.go
index f6dd690..fbc2258 100644
--- a/pkg/state/state.go
+++ b/pkg/state/state.go
@@ -477,7 +477,7 @@ func (st *HelmState) DeleteReleasesForSync(affectedReleases *AffectedReleases, h
 					relErr = newReleaseError(release, err)
 				} else {
 					var args []string
-					if isHelm3() {
+					if helm.IsHelm3() {
 						args = []string{}
 						if release.Namespace != "" {
 							args = append(args, "--namespace", release.Namespace)
@@ -577,7 +577,7 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme
 						relErr = newReleaseError(release, err)
 					} else if installed {
 						var args []string
-						if isHelm3() {
+						if helm.IsHelm3() {
 							args = []string{}
 						} else {
 							args = []string{"--purge"}
@@ -646,7 +646,7 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme
 
 func (st *HelmState) listReleases(context helmexec.HelmContext, helm helmexec.Interface, release *ReleaseSpec) (string, error) {
 	flags := st.connectionFlags(release)
-	if isHelm3() && release.Namespace != "" {
+	if helm.IsHelm3() && release.Namespace != "" {
 		flags = append(flags, "--namespace", release.Namespace)
 	}
 	return helm.List(context, "^"+release.Name+"$", flags...)
@@ -1161,11 +1161,11 @@ func (st *HelmState) DeleteReleases(affectedReleases *AffectedReleases, helm hel
 		st.ApplyOverrides(&release)
 
 		flags := []string{}
-		if purge && !isHelm3() {
+		if purge && !helm.IsHelm3() {
 			flags = append(flags, "--purge")
 		}
 		flags = st.appendConnectionFlags(flags, &release)
-		if isHelm3() && release.Namespace != "" {
+		if helm.IsHelm3() && release.Namespace != "" {
 			flags = append(flags, "--namespace", release.Namespace)
 		}
 		context := st.createHelmContext(&release, workerIndex)
@@ -1192,7 +1192,7 @@ func (st *HelmState) TestReleases(helm helmexec.Interface, cleanup bool, timeout
 			flags = append(flags, "--cleanup")
 		}
 		duration := strconv.Itoa(timeout)
-		if isHelm3() {
+		if helm.IsHelm3() {
 			duration += "s"
 		}
 		flags = append(flags, "--timeout", duration)
@@ -1202,10 +1202,6 @@ func (st *HelmState) TestReleases(helm helmexec.Interface, cleanup bool, timeout
 	})
 }
 
-func isHelm3() bool {
-	return os.Getenv("HELMFILE_HELM3") != ""
-}
-
 // Clean will remove any generated secrets
 func (st *HelmState) Clean() []error {
 	errs := []error{}
@@ -1532,7 +1528,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
 	}
 	if timeout != 0 {
 		duration := strconv.Itoa(timeout)
-		if isHelm3() {
+		if helm.IsHelm3() {
 			duration += "s"
 		}
 		flags = append(flags, "--timeout", duration)


Test patch:
diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go
index fbf94ad..0a16983 100644
--- a/pkg/app/app_test.go
+++ b/pkg/app/app_test.go
@@ -1994,6 +1994,9 @@ func (helm *mockHelmExec) Fetch(chart string, flags ...string) error {
 func (helm *mockHelmExec) Lint(name, chart string, flags ...string) error {
 	return nil
 }
+func (helm *mockHelmExec) IsHelm3() bool {
+	return false
+}
 
 func TestTemplate_SingleStateFile(t *testing.T) {
 	files := map[string]string{
@@ -3593,6 +3596,64 @@ myrelease4	         	true     	id:myrelease1
 	assert.Equal(t, expected, out)
 }
 
+func TestSetValuesTemplate(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+releases:
+- name: zipkin
+  chart: stable/zipkin
+  values:
+  - val2: "val2"
+  valuesTemplate:
+  - val1: '{{"{{ .Release.Name }}"}}'
+  set:
+  - name: "name"
+    value: "val"
+  setTemplate:
+  - name: name-{{"{{ .Release.Name }}"}}
+    value: val-{{"{{ .Release.Name }}"}}
+`,
+	}
+	expectedValues := []interface{}{
+		map[interface{}]interface{}{"val1": "zipkin"},
+		map[interface{}]interface{}{"val2": "val2"}}
+	expectedSetValues := []state.SetValue{
+		state.SetValue{Name: "name-zipkin", Value: "val-zipkin"},
+		state.SetValue{Name: "name", Value: "val"}}
+
+	app := appWithFs(&App{
+		KubeContext: "default",
+		Logger:      helmexec.NewLogger(os.Stderr, "debug"),
+		Env:         "default",
+	}, files)
+
+	var specs []state.ReleaseSpec
+	collectReleases := func(st *state.HelmState, helm helmexec.Interface) []error {
+		specs = append(specs, st.Releases...)
+		return nil
+	}
+
+	err := app.VisitDesiredStatesWithReleasesFiltered(
+		"helmfile.yaml", collectReleases,
+	)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if len(specs) != 1 {
+		t.Fatalf("expected 1 release; got %d releases", len(specs))
+	}
+	actualValues := specs[0].Values
+	actualSetValues := specs[0].SetValues
+
+	if !reflect.DeepEqual(expectedValues, actualValues) {
+		t.Errorf("expected values: %v; got values: %v", expectedValues, actualValues)
+	}
+	if !reflect.DeepEqual(expectedSetValues, actualSetValues) {
+		t.Errorf("expected set: %v; got set: %v", expectedValues, actualValues)
+	}
+}
+
 func location() string {
 	_, fn, line, _ := runtime.Caller(1)
 	return fmt.Sprintf("%s:%d", filepath.Base(fn), line)
diff --git a/pkg/exectest/helm.go b/pkg/exectest/helm.go
index ebcf1fa..3a759c5 100644
--- a/pkg/exectest/helm.go
+++ b/pkg/exectest/helm.go
@@ -157,6 +157,10 @@ func (helm *Helm) TemplateRelease(name, chart string, flags ...string) error {
 	return nil
 }
 
+func (helm *Helm) IsHelm3() bool {
+	return false
+}
+
 func (helm *Helm) sync(m *sync.Mutex, f func()) {
 	if m != nil {
 		m.Lock()
diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go
index 0bb32ff..91f4f64 100644
--- a/pkg/helmexec/exec_test.go
+++ b/pkg/helmexec/exec_test.go
@@ -20,7 +20,7 @@ type mockRunner struct {
 }
 
 func (mock *mockRunner) Execute(cmd string, args []string, env map[string]string) ([]byte, error) {
-	return []byte{}, nil
+	return mock.output, mock.err
 }
 
 func MockExecer(logger *zap.SugaredLogger, kubeContext string) *execer {
@@ -32,10 +32,7 @@ func MockExecer(logger *zap.SugaredLogger, kubeContext string) *execer {
 
 func TestNewHelmExec(t *testing.T) {
 	buffer := bytes.NewBufferString("something")
-	logger := NewLogger(buffer, "debug")
-	helm := New("helm", logger, "dev", &ShellRunner{
-		Logger: logger,
-	})
+	helm := MockExecer(NewLogger(buffer, "debug"), "dev")
 	if helm.kubeContext != "dev" {
 		t.Error("helmexec.New() - kubeContext")
 	}
@@ -48,11 +45,7 @@ func TestNewHelmExec(t *testing.T) {
 }
 
 func Test_SetExtraArgs(t *testing.T) {
-	buffer := bytes.NewBufferString("something")
-	logger := NewLogger(buffer, "debug")
-	helm := New("helm", NewLogger(os.Stdout, "info"), "dev", &ShellRunner{
-		Logger: logger,
-	})
+	helm := MockExecer(NewLogger(os.Stdout, "info"), "dev")
 	helm.SetExtraArgs()
 	if len(helm.extra) != 0 {
 		t.Error("helmexec.SetExtraArgs() - passing no arguments should not change extra field")
@@ -68,11 +61,7 @@ func Test_SetExtraArgs(t *testing.T) {
 }
 
 func Test_SetHelmBinary(t *testing.T) {
-	buffer := bytes.NewBufferString("something")
-	logger := NewLogger(buffer, "debug")
-	helm := New("helm", NewLogger(os.Stdout, "info"), "dev", &ShellRunner{
-		Logger: logger,
-	})
+	helm := MockExecer(NewLogger(os.Stdout, "info"), "dev")
 	if helm.helmBinary != "helm" {
 		t.Error("helmexec.command - default command is not helm")
 	}
@@ -517,3 +506,17 @@ exec: helm template path/to/chart --name release --values file.yml --kube-contex
 		t.Errorf("helmexec.Template()\nactual = %v\nexpect = %v", buffer.String(), expected)
 	}
 }
+
+func Test_IsHelm3(t *testing.T) {
+	helm2Runner := mockRunner{output: []byte("Client: v2.16.0+ge13bc94\n")}
+	helm := New("helm", NewLogger(os.Stdout, "info"), "dev", &helm2Runner)
+	if helm.IsHelm3() {
+		t.Error("helmexec.IsHelm3() - Detected Helm 3 with Helm 2 version")
+	}
+
+	helm3Runner := mockRunner{output: []byte("v3.0.0+ge29ce2a\n")}
+	helm = New("helm", NewLogger(os.Stdout, "info"), "dev", &helm3Runner)
+	if !helm.IsHelm3() {
+		t.Error("helmexec.IsHelm3() - Failed to detect Helm 3")
+	}
+}
diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go
index caa2c6f..eb865ce 100644
--- a/pkg/state/state_test.go
+++ b/pkg/state/state_test.go
@@ -145,6 +145,14 @@ func boolValue(v bool) *bool {
 	return &v
 }
 
+// Mocking the command-line runner
+
+type mockRunner struct{}
+
+func (mock *mockRunner) Execute(cmd string, args []string, env map[string]string) ([]byte, error) {
+	return []byte{}, nil
+}
+
 func TestHelmState_flagsForUpgrade(t *testing.T) {
 	enable := true
 	disable := false
@@ -524,9 +532,7 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
 				HelmDefaults:      tt.defaults,
 				valsRuntime:       valsRuntime,
 			}
-			helm := helmexec.New("helm", logger, "default", &helmexec.ShellRunner{
-				Logger: logger,
-			})
+			helm := helmexec.New("helm", logger, "default", &mockRunner{})
 			args, err := state.flagsForUpgrade(helm, tt.release, 0)
 			if err != nil {
 				t.Errorf("unexpected error flagsForUpgade: %v", err)
diff --git a/test/integration/run.sh b/test/integration/run.sh
index 03d55c9..ec9605f 100755
--- a/test/integration/run.sh
+++ b/test/integration/run.sh
@@ -46,7 +46,7 @@ function retry() {
 set -e
 info "Using namespace: ${test_ns}"
 # helm v2
-if helm version --client 1>/dev/null 2>/dev/null; then
+if helm version --client 2>/dev/null | grep '"v2\.'; then
   info "Using Helm version: $(helm version --short --client | grep -o v.*$)"
   ${helm} init --wait --override spec.template.spec.automountServiceAccountToken=true
 # helm v3