From 07c42474cc3d338a75b46ee0883fb19fffd7d065 Mon Sep 17 00:00:00 2001 From: KUOKA Yusuke Date: Sun, 1 Dec 2019 11:37:56 +0900 Subject: [PATCH] Do not fail due to missing env in base helmfile (#1009) When helmfile is run with `--environment NAME` and there was a base hemlfile that misses `environments`, helmfile had been trying to load env values for NAME and failing. A base helmfile is allowed to reference values from within itself, but that's optional. In other words, a base helmfile that misses the env is okay as long as it doesn't self-reference env values. So, this change allows missing env and env values while loading base helmfile. After loading, a base helmfile can fail due to referencing missing env values, but that's okay. Fixes #1008 --- pkg/app/app_test.go | 36 ++++++++++++++++++++++++++++++++++++ pkg/state/create.go | 21 +++++++++++---------- pkg/state/create_test.go | 4 ++-- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index b79fad25..db6179db 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -177,6 +177,42 @@ releases: } } +func TestVisitDesiredStatesWithReleasesFiltered_Issue1008_MissingNonDefaultEnvInBase(t *testing.T) { + files := map[string]string{ + "/path/to/base.yaml": ` +helmDefaults: + wait: true +`, + "/path/to/helmfile.yaml": ` +bases: +- base.yaml +environments: + test: +releases: +- name: zipkin + chart: stable/zipkin +`, + } + fs := testhelper.NewTestFs(files) + app := &App{ + KubeContext: "default", + Logger: helmexec.NewLogger(os.Stderr, "debug"), + Namespace: "", + Env: "test", + } + app = injectFs(app, fs) + noop := func(st *state.HelmState, helm helmexec.Interface) []error { + return []error{} + } + + err := app.VisitDesiredStatesWithReleasesFiltered( + "helmfile.yaml", noop, + ) + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + func TestVisitDesiredStatesWithReleasesFiltered_MissingEnvValuesFileHandler(t *testing.T) { testcases := []struct { name string diff --git a/pkg/state/create.go b/pkg/state/create.go index a3598191..1e09ef3d 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -116,10 +116,10 @@ func (c *StateCreator) Parse(content []byte, baseDir, file string) (*HelmState, } // LoadEnvValues loads environment values files relative to the `baseDir` -func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv *environment.Environment) (*HelmState, error) { +func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv *environment.Environment, failOnMissingEnv bool) (*HelmState, error) { state := *target - e, err := state.loadEnvValues(env, ctxEnv, c.readFile, c.glob) + e, err := state.loadEnvValues(env, failOnMissingEnv, ctxEnv, c.readFile, c.glob) if err != nil { return nil, &StateLoadError{fmt.Sprintf("failed to read %s", state.FilePath), err} } @@ -135,6 +135,7 @@ func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv *envi } // Parses YAML into HelmState, while loading environment values files relative to the `baseDir` +// evaluateBases=true means that this is NOT a base helmfile func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envName string, evaluateBases bool, envValues *environment.Environment) (*HelmState, error) { state, err := c.Parse(content, baseDir, file) if err != nil { @@ -145,14 +146,14 @@ func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envNam if len(state.Bases) > 0 { return nil, errors.New("nested `base` helmfile is unsupported. please submit a feature request if you need this!") } + } else { + state, err = c.loadBases(envValues, state, baseDir) + if err != nil { + return nil, err + } } - state, err = c.loadBases(envValues, state, baseDir) - if err != nil { - return nil, err - } - - state, err = c.LoadEnvValues(state, envName, envValues) + state, err = c.LoadEnvValues(state, envName, envValues, evaluateBases) if err != nil { return nil, err } @@ -182,7 +183,7 @@ func (c *StateCreator) loadBases(envValues *environment.Environment, st *HelmSta return layers[0], nil } -func (st *HelmState) loadEnvValues(name string, ctxEnv *environment.Environment, readFile func(string) ([]byte, error), glob func(string) ([]string, error)) (*environment.Environment, error) { +func (st *HelmState) loadEnvValues(name string, failOnMissingEnv bool, ctxEnv *environment.Environment, readFile func(string) ([]byte, error), glob func(string) ([]string, error)) (*environment.Environment, error) { envVals := map[string]interface{}{} envSpec, ok := st.Environments[name] if ok { @@ -210,7 +211,7 @@ func (st *HelmState) loadEnvValues(name string, ctxEnv *environment.Environment, return nil, err } } - } else if ctxEnv == nil && name != DefaultEnv { + } else if ctxEnv == nil && name != DefaultEnv && failOnMissingEnv { return nil, &UndefinedEnvError{msg: fmt.Sprintf("environment \"%s\" is not defined", name)} } diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index 072fa789..d70f9c8a 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -20,7 +20,7 @@ func createFromYaml(content []byte, file string, env string, logger *zap.Sugared abs: filepath.Abs, Strict: true, } - return c.ParseAndLoad(content, filepath.Dir(file), file, env, false, nil) + return c.ParseAndLoad(content, filepath.Dir(file), file, env, true, nil) } func TestReadFromYaml(t *testing.T) { @@ -108,7 +108,7 @@ bar: {{ readFile "bar.txt" }} }) testFs.Cwd = "/example/path/to" - state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, nil, nil).ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", false, nil) + state, err := NewCreator(logger, testFs.ReadFile, testFs.FileExists, testFs.Abs, testFs.Glob, nil, nil).ParseAndLoad(yamlContent, filepath.Dir(yamlFile), yamlFile, "production", true, nil) if err != nil { t.Fatalf("unexpected error: %v", err) }