diff --git a/README.md b/README.md index 0d982940..6155f88a 100644 --- a/README.md +++ b/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 diff --git a/pkg/app/app.go b/pkg/app/app.go index d787df19..6d5d3516 100644 --- a/pkg/app/app.go +++ b/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 { diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 713df53d..99fe80be 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2,15 +2,16 @@ package app import ( "fmt" - "github.com/roboll/helmfile/helmexec" - "github.com/roboll/helmfile/state" - "gotest.tools/env" "io/ioutil" "os" "path/filepath" "reflect" "strings" "testing" + + "github.com/roboll/helmfile/helmexec" + "github.com/roboll/helmfile/state" + "gotest.tools/env" ) type testFs struct { @@ -329,13 +330,13 @@ func TestVisitDesiredStatesWithReleasesFiltered_EmbeddedSelectors(t *testing.T) files := map[string]string{ "/path/to/helmfile.yaml": ` helmfiles: -- helmfile.d/a*.yaml: - selectors: - - name=prometheus - - name=zipkin +- path: helmfile.d/a*.yaml + selectors: + - name=prometheus + - name=zipkin - helmfile.d/b*.yaml -- helmfile.d/c*.yaml: - selectors: {} +- path: helmfile.d/c*.yaml + selectors: [] `, "/path/to/helmfile.d/a1.yaml": ` releases: @@ -462,17 +463,17 @@ func TestVisitDesiredStatesWithReleasesFiltered_InheritedSelectors_inherits(t *t "/path/to/helmfile.yaml": ` helmfiles: - helmfile.d/a*.yaml -- helmfile.d/a*.yaml: - selectors: - - select=foo +- path: helmfile.d/a*.yaml + selectors: + - select=foo releases: - name: mongodb chart: stable/mongodb `, "/path/to/helmfile.d/a.yaml": ` helmfiles: -- b*.yaml: - selectors: inherits +- path: b*.yaml + selectorsInherited: true releases: - name: zipkin chart: stable/zipkin diff --git a/state/create_test.go b/state/create_test.go index d66efed3..d99d9b62 100644 --- a/state/create_test.go +++ b/state/create_test.go @@ -7,6 +7,7 @@ import ( "testing" . "gotest.tools/assert" + "gotest.tools/assert/cmp" ) func TestReadFromYaml(t *testing.T) { @@ -261,45 +262,26 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) { path: "working/selector", content: []byte(`helmfiles: - simple/helmfile.yaml -- simple/helmfile/with/semicolon.yaml: -- two/selectors.yaml: - selectors: - - name=foo - - name=bar -- empty/selector.yaml: - selectors: {} -- inherits/selector.yaml: - selectors: inherits - path: path/prefix/selector.yaml selectors: - 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"}, - {Path: "simple/helmfile/with/semicolon.yaml", Selectors: nil, Inherits: false}, - {Path: "two/selectors.yaml", Selectors: []string{"name=foo", "name=bar"}, Inherits: false}, - {Path: "empty/selector.yaml", Selectors: []string{}, Inherits: false}, - {Path: "inherits/selector.yaml", Selectors: nil, Inherits: true}, - {Path: "path/prefix/selector.yaml", Selectors: []string{"name=zorba"}, 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}, }, }, - { - path: "failing1/selector", - content: []byte(`helmfiles: -- failing1/helmfile.yaml: foo -`), - wantErr: true, - }, { path: "failing2/selector", content: []byte(`helmfiles: -- failing2/helmfile.yaml: +- path: failing2/helmfile.yaml wrongkey: `), wantErr: true, @@ -307,7 +289,7 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) { { path: "failing3/selector", content: []byte(`helmfiles: -- failing3/helmfile.yaml: +- path: failing3/helmfile.yaml selectors: foo `), wantErr: true, @@ -315,7 +297,7 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) { { path: "failing4/selector", content: []byte(`helmfiles: -- failing4/helmfile.yaml: +- path: failing4/helmfile.yaml selectors: `), wantErr: true, @@ -323,17 +305,9 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) { { path: "failing4/selector", content: []byte(`helmfiles: -- failing4/helmfile.yaml: +- path: failing4/helmfile.yaml selectors: - colon: not-authorized -`), - wantErr: true, - }, - { - path: "failing5/selector", - content: []byte(`helmfiles: -- selectors: - - colon: not-authorized `), wantErr: true, }, @@ -342,6 +316,16 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) { content: []byte(`helmfiles: - selectors: - whatever +`), + wantErr: true, + }, + { + path: "failing7/selector", + content: []byte(`helmfiles: +- path: foo/bar + selectors: + - foo=bar + selectorsInherited: true `), wantErr: true, }, @@ -355,6 +339,6 @@ func TestReadFromYaml_Helmfiles_Selectors(t *testing.T) { t.Error("unexpected error:", err) } } - DeepEqual(t, st.Helmfiles, test.helmfiles) + Assert(t, cmp.DeepEqual(st.Helmfiles, test.helmfiles), "for path %v", test.path) } } diff --git a/state/state.go b/state/state.go index 2a74763b..84cee14c 100644 --- a/state/state.go +++ b/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,90 +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 - } - } else { - hf.Path = key - //get the selectors if something is specified - if v != nil { //we have a path, now compute the selector - if err := extractSelector(hf, v); err != nil { - return err - } - } //else it is a path with ending semi-colon without anything else below and it is fine - } - 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 }