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

cmd/cmd.go

@@ -27,7 +27,7 @@ func VisitAllDesiredStates(c *cli.Context, converge func(*state.HelmState, helme
 		return converge(st, helm, ctx)
 	}
 
-	err = a.VisitDesiredStates(fileOrDir, convergeWithHelmBinary)
+	err = a.VisitDesiredStates(fileOrDir, a.Selectors, convergeWithHelmBinary)
 
 	return toCliError(err)
 }

pkg/app/app.go

@@ -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 && os.Getenv(ExperimentalEnvVar) != "true" {
+					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
 

pkg/app/app_test.go

@@ -324,6 +324,132 @@ 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
+`,
+	}
+
+	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 series")
+
+	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},
+	}
+
+	os.Setenv(ExperimentalEnvVar, "true")
+	defer os.Unsetenv(ExperimentalEnvVar)
+
+	runFilterSubHelmFilesTests(desiredTestcases, files, t, "2nd series")
+
+}
+
+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{

pkg/app/constants.go

@@ -4,4 +4,5 @@ const (
 	DefaultHelmfile          = "helmfile.yaml"
 	DeprecatedHelmfile       = "charts.yaml"
 	DefaultHelmfileDirectory = "helmfile.d"
+	ExperimentalEnvVar       = "EXPERIMENTAL" // environment variable for experimental features, expecting "true" lower case
 )

state/state.go

@@ -32,13 +32,14 @@ type HelmState struct {
 	basePath           string
 	Environments       map[string]EnvironmentSpec
 	FilePath           string
-	HelmDefaults       HelmSpec         `yaml:"helmDefaults"`
-	Helmfiles          []string         `yaml:"helmfiles"`
-	DeprecatedContext  string           `yaml:"context"`
-	DeprecatedReleases []ReleaseSpec    `yaml:"charts"`
-	Namespace          string           `yaml:"namespace"`
-	Repositories       []RepositorySpec `yaml:"repositories"`
-	Releases           []ReleaseSpec    `yaml:"releases"`
+	HelmDefaults       HelmSpec          `yaml:"helmDefaults"`
+	Helmfiles          []SubHelmfileSpec `yaml:"helmfiles"`
+	DeprecatedContext  string            `yaml:"context"`
+	DeprecatedReleases []ReleaseSpec     `yaml:"charts"`
+	Namespace          string            `yaml:"namespace"`
+	Repositories       []RepositorySpec  `yaml:"repositories"`
+	Releases           []ReleaseSpec     `yaml:"releases"`
+	Selectors          []string
 
 	Templates map[string]TemplateSpec `yaml:"templates"`
 
@@ -54,6 +55,12 @@ type HelmState struct {
 	runner helmexec.Runner
 }
 
+// SubHelmfileSpec defines the subhelmfile path and options
+type SubHelmfileSpec struct {
+	Path      string
+	Selectors []string
+}
+
 // HelmSpec to defines helmDefault values
 type HelmSpec struct {
 	KubeContext     string   `yaml:"kubeContext"`
@@ -866,11 +873,11 @@ func (st *HelmState) Clean() []error {
 }
 
 // FilterReleases allows for the execution of helm commands against a subset of the releases in the helmfile.
-func (st *HelmState) FilterReleases(labels []string) error {
+func (st *HelmState) FilterReleases() error {
 	var filteredReleases []ReleaseSpec
 	releaseSet := map[string][]ReleaseSpec{}
 	filters := []ReleaseFilter{}
-	for _, label := range labels {
+	for _, label := range st.Selectors {
 		f, err := ParseLabels(label)
 		if err != nil {
 			return err
@@ -902,7 +909,7 @@ func (st *HelmState) FilterReleases(labels []string) error {
 	}
 	st.Releases = filteredReleases
 	numFound := len(filteredReleases)
-	st.logger.Debugf("%d release(s) matching %s found in %s\n", numFound, strings.Join(labels, ","), st.FilePath)
+	st.logger.Debugf("%d release(s) matching %s found in %s\n", numFound, strings.Join(st.Selectors, ","), st.FilePath)
 	return nil
 }
 
@@ -1387,3 +1394,68 @@ func escape(value string) string {
 	intermediate = strings.Replace(intermediate, "}", "\\}", -1)
 	return strings.Replace(intermediate, ",", "\\,", -1)
 }
+
+//UnmarshalYAML will unmarshal the helmfile yaml section and fill the SubHelmfileSpec structure
+//this is required to keep allowing string scalar for defining helmfile (maybe)
+func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(interface{}) error) error {
+
+	var tmp interface{}
+	if err := unmarshal(&tmp); err != nil {
+		return err
+	}
+
+	switch i := tmp.(type) {
+	case string: // single path definition without sub items
+		hf.Path = i
+	case map[interface{}]interface{}: // helmfile path with sub section
+		for k, v := range i {
+			switch key := k.(type) {
+			case string:
+				//get the path
+				hf.Path = key
+				//get the selectors
+				if err := extractSelector(hf, v); err != nil {
+					return err
+				}
+			default:
+				return fmt.Errorf("Expecting a \"string\" scalar for the helmfile collection but got: %v", key)
+			}
+		}
+	}
+
+	return nil
+}
+
+//extractSelector this will extract the selectors: from the helmfile section
+//this has been developed to only expect selectors: under the helmfiles for now.
+func extractSelector(hf *SubHelmfileSpec, value interface{}) error {
+	switch value := value.(type) {
+	case map[interface{}]interface{}:
+		for k, v := range value {
+			switch key := k.(type) {
+			case string:
+				if key == "selectors" {
+					switch selectors := v.(type) {
+					case []interface{}:
+						for _, sel := range selectors {
+							hf.Selectors = append(hf.Selectors, sel.(string))
+						}
+					case map[interface{}]interface{}:
+						if len(selectors) == 0 {
+							hf.Selectors = make([]string, 0) //allocate and non nil empty array
+						} else { //unexpected unempty map so error
+							return fmt.Errorf("unexpected unempty map in selector [-%v] but got: %v", hf.Path, selectors)
+						}
+					default:
+						return fmt.Errorf("Expecting a \"selectors\" mapping string [-%v] but got: %v", hf.Path, selectors)
+					}
+				} //else ignores other elements
+			default: //we where expecting a selector but go something else
+				return fmt.Errorf("Expecting a \"selectors\" mapping for string [-%v] but got: %v", hf.Path, key)
+			}
+		}
+	default:
+		return fmt.Errorf("Expecting a \"selectors\" mapping for string [-%v] but got: %v", hf.Path, value)
+	}
+	return nil
+}

state/state_test.go

@@ -1636,7 +1636,8 @@ func TestHelmState_NoReleaseMatched(t *testing.T) {
 				Releases: releases,
 				logger:   logger,
 			}
-			errs := state.FilterReleases([]string{tt.labels})
+			state.Selectors = []string{tt.labels}
+			errs := state.FilterReleases()
 			if (errs != nil) != tt.wantErr {
 				t.Errorf("ReleaseStatuses() for %s error = %v, wantErr %v", tt.name, errs, tt.wantErr)
 				return