feat(roboll#344): use selectorInherited for subhelm selectors

README.md

@@ -648,21 +648,21 @@ When composing helmfiles you can use selectors from the command line as well as
 ```yaml
 helmfiles:
 - apps/*/helmfile.yaml
-- apps/a-helmfile.yaml:
+- path: apps/a-helmfile.yaml
     selectors:          # list of selectors
     - name=prometheus      
     - tier=frontend    
-- apps/b-helmfile.yaml: # no selector, so all releases are used
-    selectors: {}
-- apps/c-helmfile.yaml: # parent selector to be used or cli selector for the initial helmfile
-    selectors: inherits
+- path: apps/b-helmfile.yaml # no selector, so all releases are used
+    selectors: []
+- path: apps/c-helmfile.yaml # parent selector to be used or cli selector for the initial helmfile
+    selectorsInherited: true
 ```
 * When a selector is specified, only this selector applies and the parents or CLI selectors are ignored.
 * When not selector is specified there are 2 modes for the selector inheritance because we would like to change the current inheritance behavior (see [issue #344](https://github.com/roboll/helmfile/issues/344)  ).
   * Legacy mode, sub-helmfiles without selectors inherit selectors from their parent helmfile. The initial helmfiles inherit from the command line selectors. 
-  * explicit mode, sub-helmfile without selectors do not inherit from their parent or the CLI selector. If you want them to inherit from their parent selector then use `selectors: inherits`. To enable this explicit mode you need to set the following environment variable `HELMFILE_EXPERIMENTAL=explicit-selector-inheritance` (see [experimental](#experimental-features)).
-* Using `selector: {}` will for all releases to be used regardless of the parent selector or cli for the initial helmfile
-* using `selector: inherits` make the sub-helmfile selects release with the parent selector or the cli for the initial helmfile
+  * explicit mode, sub-helmfile without selectors do not inherit from their parent or the CLI selector. If you want them to inherit from their parent selector then use `selectorsInherited: true`. To enable this explicit mode you need to set the following environment variable `HELMFILE_EXPERIMENTAL=explicit-selector-inheritance` (see [experimental](#experimental-features)).
+* Using `selector: []` will select all releases regardless of the parent selector or cli for the initial helmfile
+* using `selectorsInherited: true` make the sub-helmfile selects releases with the parent selector or the cli for the initial helmfile. You cannot specify an explicit selector while using `selectorsInherited: true`
 
 ## Importing values from any source
 

pkg/app/app.go

@@ -2,13 +2,15 @@ package app
 
 import (
 	"fmt"
-	"github.com/roboll/helmfile/helmexec"
-	"github.com/roboll/helmfile/state"
-	"go.uber.org/zap"
 	"io/ioutil"
 	"log"
 	"os"
 	"os/signal"
+
+	"github.com/roboll/helmfile/helmexec"
+	"github.com/roboll/helmfile/state"
+	"go.uber.org/zap"
+
 	"path/filepath"
 	"sort"
 	"syscall"
@@ -159,7 +161,7 @@ func (a *App) VisitDesiredStates(fileOrDir string, selector []string, converge f
 			noMatchInSubHelmfiles := true
 			for _, m := range st.Helmfiles {
 				//assign parent selector to sub helm selector in legacy mode or do not inherit in experimental mode
-				if (m.Selectors == nil && !isExplicitSelectorInheritanceEnabled()) || m.Inherits {
+				if (m.Selectors == nil && !isExplicitSelectorInheritanceEnabled()) || m.SelectorsInherited {
 					m.Selectors = selector
 				}
 				if err := a.VisitDesiredStates(m.Path, m.Selectors, converge); err != nil {

pkg/app/app_test.go

@@ -336,7 +336,7 @@ helmfiles:
   - name=zipkin      
 - helmfile.d/b*.yaml
 - path: helmfile.d/c*.yaml
-  selectors: {}
+  selectors: []
 `,
 		"/path/to/helmfile.d/a1.yaml": `
 releases:
@@ -473,7 +473,7 @@ releases:
 		"/path/to/helmfile.d/a.yaml": `
 helmfiles:
 - path: b*.yaml
-  selectors: inherits
+  selectorsInherited: true
 releases:
 - name: zipkin
   chart: stable/zipkin

state/create_test.go

@@ -267,15 +267,15 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) {
     - name=zorba
     - foo=bar
 - path: path/prefix/empty/selector.yaml
-  selectors: {}
+  selectors: []
 - path: path/prefix/inherits/selector.yaml
-  selectors: inherits
+  selectorsInherited: true
 `),
 			wantErr: false,
-			helmfiles: []SubHelmfileSpec{{Path: "simple/helmfile.yaml", Selectors: nil, Inherits: false},
-				{Path: "path/prefix/selector.yaml", Selectors: []string{"name=zorba", "foo=bar"}, Inherits: false},
-				{Path: "path/prefix/empty/selector.yaml", Selectors: []string{}, Inherits: false},
-				{Path: "path/prefix/inherits/selector.yaml", Selectors: nil, Inherits: true},
+			helmfiles: []SubHelmfileSpec{{Path: "simple/helmfile.yaml", Selectors: nil, SelectorsInherited: false},
+				{Path: "path/prefix/selector.yaml", Selectors: []string{"name=zorba", "foo=bar"}, SelectorsInherited: false},
+				{Path: "path/prefix/empty/selector.yaml", Selectors: []string{}, SelectorsInherited: false},
+				{Path: "path/prefix/inherits/selector.yaml", Selectors: nil, SelectorsInherited: true},
 			},
 		},
 		{
@@ -319,6 +319,16 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) {
 `),
 			wantErr: true,
 		},
+		{
+			path: "failing7/selector",
+			content: []byte(`helmfiles:
+- path: foo/bar
+  selectors:
+  - foo=bar
+  selectorsInherited: true
+`),
+			wantErr: true,
+    },
 	}
 	for _, test := range tests {
 		st, err := createFromYaml(test.content, test.path, DefaultEnv, logger)

state/state.go

@@ -58,13 +58,11 @@ type HelmState struct {
 
 // SubHelmfileSpec defines the subhelmfile path and options
 type SubHelmfileSpec struct {
-	Path      string   //path or glob pattern for the sub helmfiles
-	Selectors []string //chosen selectors for the sub helmfiles
-	Inherits  bool     //do the sub helmfiles inherits from parent selectors
+	Path               string   //path or glob pattern for the sub helmfiles
+	Selectors          []string //chosen selectors for the sub helmfiles
+	SelectorsInherited bool     //do the sub helmfiles inherits from parent selectors
 }
 
-const InheritsYamlValue = "inherits"
-
 // HelmSpec to defines helmDefault values
 type HelmSpec struct {
 	KubeContext     string   `yaml:"kubeContext"`
@@ -1400,7 +1398,7 @@ func escape(value string) string {
 }
 
 //UnmarshalYAML will unmarshal the helmfile yaml section and fill the SubHelmfileSpec structure
-//this is required to keep allowing string scalar for defining helmfile (maybe)
+//this is required to keep allowing string scalar for defining helmfile
 func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(interface{}) error) error {
 
 	var tmp interface{}
@@ -1409,82 +1407,29 @@ func (hf *SubHelmfileSpec) UnmarshalYAML(unmarshal func(interface{}) error) erro
 	}
 
 	switch i := tmp.(type) {
-	case string: // single path definition without sub items
+	case string: // single path definition without sub items, legacy sub helmfile definition
 		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
-				if key == "path" {
-					hf.Path = v.(string)
-				} else if key == "selectors" {
-					if err := extractSelectorContent(hf, v); err != nil {
-						return err
-					}
-				}
-			default:
-				return fmt.Errorf("Expecting a \"string\" scalar for the helmfile collection but got: %v", key)
-			}
+		var subHelmfileSpecTmp struct {
+			Path               string   `yaml:"path"`
+			Selectors          []string `yaml:"selectors"`
+			SelectorsInherited bool     `yaml:"selectorsInherited"`
+		}
+		if err := unmarshal(&subHelmfileSpecTmp); err != nil {
+			return err
 		}
+		hf.Path = subHelmfileSpecTmp.Path
+		hf.Selectors = subHelmfileSpecTmp.Selectors
+		hf.SelectorsInherited = subHelmfileSpecTmp.SelectorsInherited
 	}
 	//since we cannot make sur the "console" string can be red after the "path" we must check we don't have
 	//a SubHelmfileSpec with only selector and no path
 	if hf.Selectors != nil && hf.Path == "" {
 		return fmt.Errorf("found 'selectors' definition without path: %v", hf.Selectors)
 	}
-
-	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" {
-					return extractSelectorContent(hf, v)
-				} else { //not 'selectors' so error
-					return fmt.Errorf("Expecting a \"selectors\" mapping but got: %v", key)
-				}
-			default: //we where expecting a selector but go something else
-				return fmt.Errorf("Expecting a \"selectors\" mapping for string [-%v] but got: %v of type %T", hf.Path, key, key)
-			}
-		}
-	default:
-		return fmt.Errorf("Expecting a \"selectors\" mapping for string [-%v] but got: %v of type %T", hf.Path, value, value)
-	}
-	return nil
-}
-
-func extractSelectorContent(hf *SubHelmfileSpec, value interface{}) error {
-	switch selectors := value.(type) {
-	case string: //check the string is `inherits` else error
-		if selectors == InheritsYamlValue {
-			hf.Inherits = true
-		} else {
-			return fmt.Errorf("Expecting a list of string, an empty {} or '%v' but got: %v", InheritsYamlValue, selectors)
-		}
-	case []interface{}: //expect an array if strings
-		for _, sel := range selectors {
-			switch selValue := sel.(type) {
-			case string:
-				hf.Selectors = append(hf.Selectors, selValue)
-			default:
-				return fmt.Errorf("Expecting a string, but got: %v", selValue)
-			}
-		}
-	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 list of strings or and empty {} mapping [-%v] but got: [%v] of type [%T] ", hf.Path, selectors, selectors)
+	//also exclude SelectorsInherited to true and explicit selectors
+	if hf.SelectorsInherited && len(hf.Selectors) > 0 {
+		return fmt.Errorf("You cannot use 'SelectorsInherited: true' along with and explicit selector for path: %v", hf.Path)
 	}
 	return nil
 }