feat(#344): add sub helmfiles explicit selectors (#567)

Fixes #344 by allowing explicit selectors to be specified for composed helmfiles using the following structure ```yaml helmfiles: - path: helmfile.d/a*.yaml selectors: - name=prometheus - name!=zipkin - helmfile.d/b*.yaml - path: helmfile.d/c*.yaml selectors: {} ``` 2 modes here : * legacy mode when no the env var HELMFILE_EXPERIMENTAL is not set to true * no selector : inherit from the command line. * selector: is specified then it is used (an emty means no inheritance from command line and take everything). * experimental when the env var HELMFILE_EXPERIMENTAL=true * no selector : nothing is inherited from the command line so use all releases. * selector: is specified then it is used (an emty means no inheritance from command line and take everything).
2026-10-06 16:11:24 +02:00 · 2019-05-05 13:38:52 +09:00
parent d2353b5b70
commit 4581e004b8
11 changed files with 569 additions and 28 deletions
@@ -108,7 +108,7 @@ func (a *App) visitStateFiles(fileOrDir string, do func(string) error) error {
 	return nil
 }

-func (a *App) VisitDesiredStates(fileOrDir string, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) error {
+func (a *App) VisitDesiredStates(fileOrDir string, selector []string, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) error {
 	noMatchInHelmfiles := true

 	err := a.visitStateFiles(fileOrDir, func(f string) error {
@@ -153,16 +153,21 @@ func (a *App) VisitDesiredStates(fileOrDir string, converge func(*state.HelmStat
 				return err
 			}
 		}
+		st.Selectors = selector

 		if len(st.Helmfiles) > 0 {
 			noMatchInSubHelmfiles := true
 			for _, m := range st.Helmfiles {
-				if err := a.VisitDesiredStates(m, converge); err != nil {
+				//assign parent selector to sub helm selector in legacy mode or do not inherit in experimental mode
+				if (m.Selectors == nil && !isExplicitSelectorInheritanceEnabled()) || m.Inherits {
+					m.Selectors = selector
+				}
+				if err := a.VisitDesiredStates(m.Path, m.Selectors, converge); err != nil {
 					switch err.(type) {
 					case *NoMatchingHelmfileError:

 					default:
-						return fmt.Errorf("failed processing %s: %v", m, err)
+						return fmt.Errorf("failed processing %s: %v", m.Path, err)
 					}
 				} else {
 					noMatchInSubHelmfiles = false
@@ -192,11 +197,10 @@ func (a *App) VisitDesiredStates(fileOrDir string, converge func(*state.HelmStat
 }

 func (a *App) VisitDesiredStatesWithReleasesFiltered(fileOrDir string, converge func(*state.HelmState, helmexec.Interface) []error) error {
-	selectors := a.Selectors

-	err := a.VisitDesiredStates(fileOrDir, func(st *state.HelmState, helm helmexec.Interface) (bool, []error) {
-		if len(selectors) > 0 {
-			err := st.FilterReleases(selectors)
+	err := a.VisitDesiredStates(fileOrDir, a.Selectors, func(st *state.HelmState, helm helmexec.Interface) (bool, []error) {
+		if len(st.Selectors) > 0 {
+			err := st.FilterReleases()
 			if err != nil {
 				return false, []error{err}
 			}
@@ -311,8 +315,9 @@ func (a *App) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin
 		return nil, err
 	}

-	helmfiles := []string{}
-	for _, globPattern := range st.Helmfiles {
+	helmfiles := []state.SubHelmfileSpec{}
+	for _, hf := range st.Helmfiles {
+		globPattern := hf.Path
 		var absPathPattern string
 		if filepath.IsAbs(globPattern) {
 			absPathPattern = globPattern
@@ -324,8 +329,12 @@ func (a *App) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin
 			return nil, fmt.Errorf("failed processing %s: %v", globPattern, err)
 		}
 		sort.Strings(matches)
+		for _, match := range matches {
+			newHelmfile := hf
+			newHelmfile.Path = match
+			helmfiles = append(helmfiles, newHelmfile)
+		}

-		helmfiles = append(helmfiles, matches...)
 	}
 	st.Helmfiles = helmfiles

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"github.com/roboll/helmfile/helmexec"
 	"github.com/roboll/helmfile/state"
+	"gotest.tools/env"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -324,6 +325,242 @@ releases:
 	}
 }

+func TestVisitDesiredStatesWithReleasesFiltered_EmbeddedSelectors(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+helmfiles:
+- helmfile.d/a*.yaml:
+    selectors:
+    - name=prometheus      
+    - name=zipkin      
+- helmfile.d/b*.yaml
+- helmfile.d/c*.yaml:
+    selectors: {}
+`,
+		"/path/to/helmfile.d/a1.yaml": `
+releases:
+- name: zipkin
+  chart: stable/zipkin
+`,
+		"/path/to/helmfile.d/a2.yaml": `
+releases:
+- name: prometheus
+  chart: stable/prometheus
+`,
+		"/path/to/helmfile.d/a3.yaml": `
+releases:
+- name: mongodb
+  chart: stable/mongodb
+`,
+		"/path/to/helmfile.d/b.yaml": `
+releases:
+- name: grafana
+  chart: stable/grafana
+- name: bar
+  chart: charts/foo
+  tillerNamespace:  bar1
+  labels:
+    duplicatedOK: yes
+- name: bar
+  chart: charts/foo
+  tillerNamespace: bar2
+  labels:
+    duplicatedOK: yes
+`,
+		"/path/to/helmfile.d/c.yaml": `
+releases:
+- name: grafana
+  chart: stable/grafana
+- name: postgresql
+  chart: charts/postgresql
+  labels:
+    whatever: yes
+`,
+	}
+	//Check with legacy behavior, that is when no explicit selector then sub-helmfiles inherits from command line selector
+	legacyTestcases := []struct {
+		label            string
+		expectedReleases []string
+		expectErr        bool
+		errMsg           string
+	}{
+		{label: "duplicatedOK=yes", expectedReleases: []string{"zipkin", "prometheus", "bar", "bar", "grafana", "postgresql"}, expectErr: false},
+		{label: "name=zipkin", expectedReleases: []string{"zipkin", "prometheus", "grafana", "postgresql"}, expectErr: false},
+		{label: "name=grafana", expectedReleases: []string{"zipkin", "prometheus", "grafana", "grafana", "postgresql"}, expectErr: false},
+		{label: "name=doesnotexists", expectedReleases: []string{"zipkin", "prometheus", "grafana", "postgresql"}, expectErr: false},
+	}
+	runFilterSubHelmFilesTests(legacyTestcases, files, t, "1st EmbeddedSelectors")
+
+	//Check with experimental behavior, that is when no explicit selector then sub-helmfiles do no inherit from any selector
+	desiredTestcases := []struct {
+		label            string
+		expectedReleases []string
+		expectErr        bool
+		errMsg           string
+	}{
+		{label: "duplicatedOK=yes", expectedReleases: []string{"zipkin", "prometheus", "grafana", "bar", "bar", "grafana", "postgresql"}, expectErr: false},
+		{label: "name=doesnotexists", expectedReleases: []string{"zipkin", "prometheus", "grafana", "bar", "bar", "grafana", "postgresql"}, expectErr: false},
+	}
+
+	defer env.Patch(t, ExperimentalEnvVar, ExperimentalSelectorExplicit)()
+
+	runFilterSubHelmFilesTests(desiredTestcases, files, t, "2nd EmbeddedSelectors")
+
+}
+
+func TestVisitDesiredStatesWithReleasesFiltered_InheritedSelectors_3leveldeep(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+helmfiles:
+- helmfile.d/a*.yaml
+releases:
+- name: mongodb
+  chart: stable/mongodb
+`,
+		"/path/to/helmfile.d/a.yaml": `
+helmfiles:
+- b*.yaml
+releases:
+- name: zipkin
+  chart: stable/zipkin
+`,
+		"/path/to/helmfile.d/b.yaml": `
+releases:
+- name: grafana
+  chart: stable/grafana
+`,
+	}
+	//Check with legacy behavior, that is when no explicit selector then sub-helmfiles inherits from command line selector
+	legacyTestcases := []struct {
+		label            string
+		expectedReleases []string
+		expectErr        bool
+		errMsg           string
+	}{
+		{label: "name!=grafana", expectedReleases: []string{"zipkin", "mongodb"}, expectErr: false},
+	}
+	runFilterSubHelmFilesTests(legacyTestcases, files, t, "1st 3leveldeep")
+
+	//Check with experimental behavior, that is when no explicit selector then sub-helmfiles do no inherit from any selector
+	desiredTestcases := []struct {
+		label            string
+		expectedReleases []string
+		expectErr        bool
+		errMsg           string
+	}{
+		{label: "name!=grafana", expectedReleases: []string{"grafana", "zipkin", "mongodb"}, expectErr: false},
+	}
+
+	defer env.Patch(t, ExperimentalEnvVar, ExperimentalSelectorExplicit)()
+
+	runFilterSubHelmFilesTests(desiredTestcases, files, t, "2nd 3leveldeep")
+
+}
+
+func TestVisitDesiredStatesWithReleasesFiltered_InheritedSelectors_inherits(t *testing.T) {
+	files := map[string]string{
+		"/path/to/helmfile.yaml": `
+helmfiles:
+- helmfile.d/a*.yaml
+- helmfile.d/a*.yaml:
+    selectors:
+    - select=foo
+releases:
+- name: mongodb
+  chart: stable/mongodb
+`,
+		"/path/to/helmfile.d/a.yaml": `
+helmfiles:
+- b*.yaml:
+    selectors: inherits
+releases:
+- name: zipkin
+  chart: stable/zipkin
+  labels:
+    select: foo
+`,
+		"/path/to/helmfile.d/b.yaml": `
+releases:
+- name: grafana
+  chart: stable/grafana
+- name: prometheus
+  chart: stable/prometheus
+  labels:
+    select: foo
+`,
+	}
+	//Check with legacy behavior, that is when no explicit selector then sub-helmfiles inherits from command line selector
+	legacyTestcases := []struct {
+		label            string
+		expectedReleases []string
+		expectErr        bool
+		errMsg           string
+	}{
+		{label: "name=grafana", expectedReleases: []string{"grafana", "prometheus", "zipkin"}, expectErr: false},
+		{label: "select!=foo", expectedReleases: []string{"grafana", "prometheus", "zipkin", "mongodb"}, expectErr: false},
+	}
+	runFilterSubHelmFilesTests(legacyTestcases, files, t, "1st inherits")
+
+	//Check with experimental behavior, that is when no explicit selector then sub-helmfiles do no inherit from any selector
+	desiredTestcases := []struct {
+		label            string
+		expectedReleases []string
+		expectErr        bool
+		errMsg           string
+	}{
+		{label: "name=grafana", expectedReleases: []string{"grafana", "prometheus", "zipkin", "prometheus", "zipkin"}, expectErr: false},
+		{label: "select!=foo", expectedReleases: []string{"grafana", "prometheus", "zipkin", "prometheus", "zipkin", "mongodb"}, expectErr: false},
+	}
+
+	defer env.Patch(t, ExperimentalEnvVar, ExperimentalSelectorExplicit)()
+
+	runFilterSubHelmFilesTests(desiredTestcases, files, t, "2nd inherits")
+
+}
+
+func runFilterSubHelmFilesTests(testcases []struct {
+	label            string
+	expectedReleases []string
+	expectErr        bool
+	errMsg           string
+}, files map[string]string, t *testing.T, testName string) {
+	for _, testcase := range testcases {
+		actual := []string{}
+
+		collectReleases := func(st *state.HelmState, helm helmexec.Interface) []error {
+			for _, r := range st.Releases {
+				actual = append(actual, r.Name)
+			}
+			return []error{}
+		}
+
+		app := appWithFs(&App{
+			KubeContext: "default",
+			Logger:      helmexec.NewLogger(os.Stderr, "debug"),
+			Namespace:   "",
+			Selectors:   []string{testcase.label},
+			Env:         "default",
+		}, files)
+
+		err := app.VisitDesiredStatesWithReleasesFiltered(
+			"helmfile.yaml", collectReleases,
+		)
+		if testcase.expectErr {
+			if err == nil {
+				t.Errorf("[%s]error expected but not happened for selector %s", testName, testcase.label)
+			} else if err.Error() != testcase.errMsg {
+				t.Errorf("[%s]unexpected error message: expected=\"%s\", actual=\"%s\"", testName, testcase.errMsg, err.Error())
+			}
+		} else if !testcase.expectErr && err != nil {
+			t.Errorf("[%s]unexpected error for selector %s: %v", testName, testcase.label, err)
+		}
+		if !reflect.DeepEqual(actual, testcase.expectedReleases) {
+			t.Errorf("[%s]unexpected releases for selector %s: expected=%v, actual=%v", testName, testcase.label, testcase.expectedReleases, actual)
+		}
+	}
+
+}
+
 // See https://github.com/roboll/helmfile/issues/312
 func TestVisitDesiredStatesWithReleasesFiltered_ReverseOrder(t *testing.T) {
 	files := map[string]string{
@@ -1,7 +1,18 @@
 package app

-const (
-	DefaultHelmfile          = "helmfile.yaml"
-	DeprecatedHelmfile       = "charts.yaml"
-	DefaultHelmfileDirectory = "helmfile.d"
+import (
+	"os"
+	"strings"
 )
+
+const (
+	DefaultHelmfile              = "helmfile.yaml"
+	DeprecatedHelmfile           = "charts.yaml"
+	DefaultHelmfileDirectory     = "helmfile.d"
+	ExperimentalEnvVar           = "HELMFILE_EXPERIMENTAL"         // environment variable for experimental features, expecting "true" lower case
+	ExperimentalSelectorExplicit = "explicit-selector-inheritance" // value to remove default selector inheritance to sub-helmfiles and use the explicit one
+)
+
+func isExplicitSelectorInheritanceEnabled() bool {
+	return os.Getenv(ExperimentalEnvVar) == "true" || strings.Contains(os.Getenv(ExperimentalEnvVar), ExperimentalSelectorExplicit)
+}
@@ -0,0 +1,27 @@
+package app
+
+import (
+	. "gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
+	"gotest.tools/env"
+	"os"
+	"testing"
+)
+
+func TestIsExplicitSelectorInheritanceEnabled(t *testing.T) {
+	//env var ExperimentalEnvVar is set
+	Assert(t, is.Equal(os.Getenv(ExperimentalEnvVar), ""))
+	Check(t, !isExplicitSelectorInheritanceEnabled())
+
+	//check for env var ExperimentalEnvVar set to true
+	defer env.Patch(t, ExperimentalEnvVar, "true")()
+	Check(t, isExplicitSelectorInheritanceEnabled())
+
+	//check for env var ExperimentalEnvVar set to anything
+	defer env.Patch(t, ExperimentalEnvVar, "foo")()
+	Check(t, !isExplicitSelectorInheritanceEnabled())
+
+	//check for env var ExperimentalEnvVar set to ExperimentalSelectorExplicit
+	defer env.Patch(t, ExperimentalEnvVar, ExperimentalSelectorExplicit)()
+	Check(t, isExplicitSelectorInheritanceEnabled())
+}