feat: removes dictionary key for subhelm and uses selectorsInherited (#576)

Removed the usage of subhelmfile path as map key.
I also introduced the selectorsInherited key for explicit parent selector inheritance.

Ref #344
This commit is contained in:
sgandon 2019-05-06 03:06:32 +02:00 committed by KUOKA Yusuke
parent 4581e004b8
commit 9a820d7bf2
5 changed files with 70 additions and 146 deletions

View File

@ -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

View File

@ -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 {

View File

@ -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

View File

@ -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)
}
}

View File

@ -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
}