fix: unexpected `no releases found for any helmfiles` on `helmfiles:` config (#318)

Fixes #315
Fixes #316

app_test.go

@@ -0,0 +1,108 @@
+package main
+
+import (
+	"fmt"
+	"github.com/roboll/helmfile/helmexec"
+	"github.com/roboll/helmfile/state"
+	"os"
+	"testing"
+)
+
+// See https://github.com/roboll/helmfile/issues/193
+func TestFindAndIterateOverDesiredStates(t *testing.T) {
+	absPaths := map[string]string{
+		".": "/path/to",
+		"/path/to/helmfile.d": "/path/to/helmfile.d",
+	}
+	dirs := map[string]bool{
+		"helmfile.d": true,
+	}
+	files := map[string]string{
+		"helmfile.yaml": `
+helmfiles:
+- helmfile.d/a*.yaml
+- helmfile.d/b*.yaml
+`,
+		"/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/b.yaml": `
+releases:
+- name: grafana
+  chart: stable/grafana
+`,
+	}
+	globMatches := map[string][]string{
+		"/path/to/helmfile.d/a*.yaml": []string{"/path/to/helmfile.d/a1.yaml", "/path/to/helmfile.d/a2.yaml"},
+		"/path/to/helmfile.d/b*.yaml": []string{"/path/to/helmfile.d/b.yaml"},
+	}
+	fileExistsAt := func(path string) bool {
+		_, ok := files[path]
+		return ok
+	}
+	directoryExistsAt := func(path string) bool {
+		_, ok := dirs[path]
+		return ok
+	}
+	readFile := func(filename string) ([]byte, error) {
+		str, ok := files[filename]
+		if !ok {
+			return []byte(nil), fmt.Errorf("no file found: %s", filename)
+		}
+		return []byte(str), nil
+	}
+	glob := func(pattern string) ([]string, error) {
+		matches, ok := globMatches[pattern]
+		if !ok {
+			return []string(nil), fmt.Errorf("no file matched: %s", pattern)
+		}
+		return matches, nil
+	}
+	abs := func(path string) (string, error) {
+		a, ok := absPaths[path]
+		if !ok {
+			return "", fmt.Errorf("abs: unexpected path: %s", path)
+		}
+		return a, nil
+	}
+	app := &app{
+		readFile:          readFile,
+		glob:              glob,
+		abs:               abs,
+		fileExistsAt:      fileExistsAt,
+		directoryExistsAt: directoryExistsAt,
+		kubeContext:       "default",
+		logger:            helmexec.NewLogger(os.Stderr, "debug"),
+	}
+	noop := func(st *state.HelmState, helm helmexec.Interface) []error {
+		return []error{}
+	}
+
+	testcases := []struct {
+		name      string
+		expectErr bool
+	}{
+		{name: "prometheus", expectErr: false},
+		{name: "zipkin", expectErr: false},
+		{name: "grafana", expectErr: false},
+		{name: "elasticsearch", expectErr: true},
+	}
+
+	for _, testcase := range testcases {
+		err := app.FindAndIterateOverDesiredStates(
+			"helmfile.yaml", noop, "", []string{fmt.Sprintf("name=%s", testcase.name)}, "default",
+		)
+		if testcase.expectErr && err == nil {
+			t.Errorf("error expected but not happened for name=%s", testcase.name)
+		} else if !testcase.expectErr && err != nil {
+			t.Errorf("unexpected error for name=%s: %v", testcase.name, err)
+		}
+	}
+}

main.go

@@ -534,6 +534,16 @@ func executeDiffCommand(c *cli.Context, st *state.HelmState, helm helmexec.Inter
 	return st.DiffReleases(helm, values, workers, detailedExitCode, suppressSecrets)
 }
 
+type app struct {
+	kubeContext       string
+	logger            *zap.SugaredLogger
+	readFile          func(string) ([]byte, error)
+	glob              func(string) ([]string, error)
+	abs               func(string) (string, error)
+	fileExistsAt      func(string) bool
+	directoryExistsAt func(string) bool
+}
+
 func findAndIterateOverDesiredStatesUsingFlags(c *cli.Context, converge func(*state.HelmState, helmexec.Interface) []error) error {
 	fileOrDir := c.GlobalString("file")
 	kubeContext := c.GlobalString("kube-context")
@@ -546,101 +556,134 @@ func findAndIterateOverDesiredStatesUsingFlags(c *cli.Context, converge func(*st
 		env = state.DefaultEnv
 	}
 
-	return findAndIterateOverDesiredStates(fileOrDir, converge, kubeContext, namespace, selectors, env, logger)
+	app := &app{
+		readFile:          ioutil.ReadFile,
+		glob:              filepath.Glob,
+		abs:               filepath.Abs,
+		fileExistsAt:      fileExistsAt,
+		directoryExistsAt: directoryExistsAt,
+		kubeContext:       kubeContext,
+		logger:            logger,
+	}
+	if err := app.FindAndIterateOverDesiredStates(fileOrDir, converge, namespace, selectors, env); err != nil {
+		switch e := err.(type) {
+		case *noMatchingHelmfileError:
+			return cli.NewExitError(e.Error(), 2)
+		}
+		return err
+	}
+	return nil
 }
 
-func findAndIterateOverDesiredStates(fileOrDir string, converge func(*state.HelmState, helmexec.Interface) []error, kubeContext, namespace string, selectors []string, env string, logger *zap.SugaredLogger) error {
+type noMatchingHelmfileError struct {
-	desiredStateFiles, err := findDesiredStateFiles(fileOrDir)
+	selectors []string
+	env       string
+}
+
+func (e *noMatchingHelmfileError) Error() string {
+	return fmt.Sprintf(
+		"err: no releases found that matches specified selector(%s) and environment(%s), in any helmfile",
+		strings.Join(e.selectors, ", "),
+		e.env,
+	)
+}
+
+func (a *app) FindAndIterateOverDesiredStates(fileOrDir string, converge func(*state.HelmState, helmexec.Interface) []error, namespace string, selectors []string, env string) error {
+	desiredStateFiles, err := a.findDesiredStateFiles(fileOrDir)
 	if err != nil {
 		return err
 	}
-	noTargetFoundForAllHelmfiles := true
+	noMatchInHelmfiles := true
 	for _, f := range desiredStateFiles {
-		logger.Debugf("Processing %s", f)
+		a.logger.Debugf("Processing %s", f)
-		yamlBuf, err := tmpl.NewFileRenderer(ioutil.ReadFile, "", environment.Environment{Name: env, Values: map[string]interface{}(nil)}).RenderTemplateFileToBuffer(f)
+		yamlBuf, err := tmpl.NewFileRenderer(a.readFile, "", environment.Environment{Name: env, Values: map[string]interface{}(nil)}).RenderTemplateFileToBuffer(f)
 		if err != nil {
 			return err
 		}
 
-		st, helm, noReleasesMatchingSelector, err := loadDesiredStateFromFile(
+		st, noMatchInThisHelmfile, err := a.loadDesiredStateFromYaml(
 			yamlBuf.Bytes(),
 			f,
-			kubeContext,
 			namespace,
 			selectors,
 			env,
-			logger,
 		)
+		helm := helmexec.New(a.logger, a.kubeContext)
 
-		var noTarget bool
 		if err != nil {
 			switch stateLoadErr := err.(type) {
 			// Addresses https://github.com/roboll/helmfile/issues/279
 			case *state.StateLoadError:
 				switch stateLoadErr.Cause.(type) {
 				case *state.UndefinedEnvError:
-					noTarget = true
+					noMatchInThisHelmfile = true
 				default:
 					return err
 				}
 			default:
 				return err
 			}
-		} else if len(st.Helmfiles) > 0 {
+		}
+
+		errs := []error{}
+
+		if len(st.Helmfiles) > 0 {
+			noMatchInSubHelmfiles := true
 			for _, globPattern := range st.Helmfiles {
 				helmfileRelativePattern := st.JoinBase(globPattern)
-				matches, err := filepath.Glob(helmfileRelativePattern)
+				matches, err := a.glob(helmfileRelativePattern)
 				if err != nil {
 					return fmt.Errorf("failed processing %s: %v", globPattern, err)
 				}
 				sort.Strings(matches)
+
 				for _, m := range matches {
-					if err := findAndIterateOverDesiredStates(m, converge, kubeContext, namespace, selectors, env, logger); err != nil {
+					if err := a.FindAndIterateOverDesiredStates(m, converge, namespace, selectors, env); err != nil {
-						return fmt.Errorf("failed processing %s: %v", globPattern, err)
+						switch err.(type) {
+						case *noMatchingHelmfileError:
+
+						default:
+							return fmt.Errorf("failed processing %s: %v", globPattern, err)
+						}
+					} else {
+						noMatchInSubHelmfiles = false
 					}
 				}
 			}
-			return nil
+			noMatchInHelmfiles = noMatchInHelmfiles && noMatchInSubHelmfiles
 		} else {
-			noTarget = noReleasesMatchingSelector
+			noMatchInHelmfiles = noMatchInHelmfiles && noMatchInThisHelmfile
+			if noMatchInThisHelmfile {
+				continue
+			}
+			errs = converge(st, helm)
 		}
 
-		noTargetFoundForAllHelmfiles = noTargetFoundForAllHelmfiles && noTarget
-		if noTarget {
-			continue
-		}
-
-		errs := converge(st, helm)
 		if err := clean(st, errs); err != nil {
 			return err
 		}
 	}
-	if noTargetFoundForAllHelmfiles {
+	if noMatchInHelmfiles {
-		logger.Errorf(
+		return &noMatchingHelmfileError{selectors, env}
-			"err: no releases found that matches specified selector(%s) and environment(%s), in any helmfile",
-			strings.Join(selectors, ", "),
-			env,
-		)
-		os.Exit(2)
 	}
 	return nil
 }
 
-func findDesiredStateFiles(specifiedPath string) ([]string, error) {
+func (a *app) findDesiredStateFiles(specifiedPath string) ([]string, error) {
 	var helmfileDir string
 	if specifiedPath != "" {
-		if fileExistsAt(specifiedPath) {
+		if a.fileExistsAt(specifiedPath) {
 			return []string{specifiedPath}, nil
-		} else if directoryExistsAt(specifiedPath) {
+		} else if a.directoryExistsAt(specifiedPath) {
 			helmfileDir = specifiedPath
 		} else {
 			return []string{}, fmt.Errorf("specified state file %s is not found", specifiedPath)
 		}
 	} else {
 		var defaultFile string
-		if fileExistsAt(DefaultHelmfile) {
+		if a.fileExistsAt(DefaultHelmfile) {
 			defaultFile = DefaultHelmfile
-		} else if fileExistsAt(DeprecatedHelmfile) {
+		} else if a.fileExistsAt(DeprecatedHelmfile) {
 			log.Printf(
 				"warn: %s is being loaded: %s is deprecated in favor of %s. See https://github.com/roboll/helmfile/issues/25 for more information",
 				DeprecatedHelmfile,
@@ -650,7 +693,7 @@ func findDesiredStateFiles(specifiedPath string) ([]string, error) {
 			defaultFile = DeprecatedHelmfile
 		}
 
-		if directoryExistsAt(DefaultHelmfileDirectory) {
+		if a.directoryExistsAt(DefaultHelmfileDirectory) {
 			if defaultFile != "" {
 				return []string{}, fmt.Errorf("configuration conlict error: you can have either %s or %s, but not both", defaultFile, DefaultHelmfileDirectory)
 			}
@@ -663,7 +706,7 @@ func findDesiredStateFiles(specifiedPath string) ([]string, error) {
 		}
 	}
 
-	files, err := filepath.Glob(filepath.Join(helmfileDir, "*.yaml"))
+	files, err := a.glob(filepath.Join(helmfileDir, "*.yaml"))
 	if err != nil {
 		return []string{}, err
 	}
@@ -683,19 +726,19 @@ func directoryExistsAt(path string) bool {
 	return err == nil && fileInfo.Mode().IsDir()
 }
 
-func loadDesiredStateFromFile(yaml []byte, file string, kubeContext, namespace string, labels []string, env string, logger *zap.SugaredLogger) (*state.HelmState, helmexec.Interface, bool, error) {
+func (a *app) loadDesiredStateFromYaml(yaml []byte, file string, namespace string, labels []string, env string) (*state.HelmState, bool, error) {
-	st, err := state.CreateFromYaml(yaml, file, env, logger)
+	c := state.NewCreator(a.logger, a.readFile, a.abs)
+	st, err := c.CreateFromYaml(yaml, file, env)
 	if err != nil {
-		return nil, nil, false, err
+		return nil, false, err
 	}
 
-	if st.Context != "" {
+	if a.kubeContext != "" {
-		if kubeContext != "" {
+		if st.Context != "" {
 			log.Printf("err: Cannot use option --kube-context and set attribute context.")
 			os.Exit(1)
 		}
-
+		st.Context = a.kubeContext
-		kubeContext = st.Context
 	}
 	if namespace != "" {
 		if st.Namespace != "" {
@@ -709,7 +752,7 @@ func loadDesiredStateFromFile(yaml []byte, file string, kubeContext, namespace s
 		err = st.FilterReleases(labels)
 		if err != nil {
 			log.Print(err)
-			return nil, nil, true, nil
+			return nil, true, nil
 		}
 	}
 
@@ -719,7 +762,7 @@ func loadDesiredStateFromFile(yaml []byte, file string, kubeContext, namespace s
 	}
 	for name, c := range releaseNameCounts {
 		if c > 1 {
-			return nil, nil, false, fmt.Errorf("duplicate release \"%s\" found: there were %d releases named \"%s\" matching specified selector", name, c, name)
+			return nil, false, fmt.Errorf("duplicate release \"%s\" found: there were %d releases named \"%s\" matching specified selector", name, c, name)
 		}
 	}
 
@@ -732,7 +775,7 @@ func loadDesiredStateFromFile(yaml []byte, file string, kubeContext, namespace s
 		clean(st, errs)
 	}()
 
-	return st, helmexec.New(logger, kubeContext), len(st.Releases) == 0, nil
+	return st, len(st.Releases) == 0, nil
 }
 
 func clean(st *state.HelmState, errs []error) error {

main_test.go

@@ -1,6 +1,10 @@
 package main
 
-import "testing"
+import (
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+)
 
 // See https://github.com/roboll/helmfile/issues/193
 func TestReadFromYaml_DuplicateReleaseName(t *testing.T) {
@@ -16,7 +20,14 @@ func TestReadFromYaml_DuplicateReleaseName(t *testing.T) {
   labels:
     stage: post
 `)
-	_, _, _, err := loadDesiredStateFromFile(yamlContent, yamlFile, "default", "default", []string{}, "default", logger)
+	app := &app{
+		readFile:    ioutil.ReadFile,
+		glob:        filepath.Glob,
+		abs:         filepath.Abs,
+		kubeContext: "default",
+		logger:      logger,
+	}
+	_, _, err := app.loadDesiredStateFromYaml(yamlContent, yamlFile, "default", []string{}, "default")
 	if err == nil {
 		t.Error("error expected but not happened")
 	}

state/create.go

@@ -30,14 +30,38 @@ func (e *UndefinedEnvError) Error() string {
 	return e.msg
 }
 
-func CreateFromYaml(content []byte, file string, env string, logger *zap.SugaredLogger) (*HelmState, error) {
+func createFromYaml(content []byte, file string, env string, logger *zap.SugaredLogger) (*HelmState, error) {
-	return createFromYamlWithFileReader(content, file, env, logger, ioutil.ReadFile)
+	c := &creator{
+		logger,
+		ioutil.ReadFile,
+		filepath.Abs,
+	}
+	return c.CreateFromYaml(content, file, env)
 }
 
-func createFromYamlWithFileReader(content []byte, file string, env string, logger *zap.SugaredLogger, readFile func(string) ([]byte, error)) (*HelmState, error) {
+type creator struct {
+	logger   *zap.SugaredLogger
+	readFile func(string) ([]byte, error)
+	abs      func(string) (string, error)
+}
+
+func NewCreator(logger *zap.SugaredLogger, readFile func(string) ([]byte, error), abs func(string) (string, error)) *creator {
+	return &creator{
+		logger:   logger,
+		readFile: readFile,
+		abs:      abs,
+	}
+}
+
+func (c *creator) CreateFromYaml(content []byte, file string, env string) (*HelmState, error) {
 	var state HelmState
 
-	state.basePath, _ = filepath.Abs(filepath.Dir(file))
+	basePath, err := c.abs(filepath.Dir(file))
+	if err != nil {
+		return nil, &StateLoadError{fmt.Sprintf("failed to read %s", file), err}
+	}
+	state.basePath = basePath
+
 	if err := yaml.UnmarshalStrict(content, &state); err != nil {
 		return nil, &StateLoadError{fmt.Sprintf("failed to read %s", file), err}
 	}
@@ -51,15 +75,15 @@ func createFromYamlWithFileReader(content []byte, file string, env string, logge
 		state.DeprecatedReleases = []ReleaseSpec{}
 	}
 
-	state.logger = logger
+	state.logger = c.logger
 
-	e, err := state.loadEnv(env, readFile)
+	e, err := state.loadEnv(env, c.readFile)
 	if err != nil {
 		return nil, &StateLoadError{fmt.Sprintf("failed to read %s", file), err}
 	}
 	state.env = *e
 
-	state.readFile = readFile
+	state.readFile = c.readFile
 
 	return &state, nil
 }

state/create_test.go

@@ -2,6 +2,7 @@ package state
 
 import (
 	"fmt"
+	"path/filepath"
 	"reflect"
 	"testing"
 )
@@ -13,7 +14,7 @@ func TestReadFromYaml(t *testing.T) {
   namespace: mynamespace
   chart: mychart
 `)
-	state, err := CreateFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
+	state, err := createFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
 	if err != nil {
 		t.Errorf("unxpected error: %v", err)
 	}
@@ -36,7 +37,7 @@ func TestReadFromYaml_InexistentEnv(t *testing.T) {
   namespace: mynamespace
   chart: mychart
 `)
-	_, err := CreateFromYaml(yamlContent, yamlFile, "production", logger)
+	_, err := createFromYaml(yamlContent, yamlFile, "production", logger)
 	if err == nil {
 		t.Error("expected error")
 	}
@@ -97,7 +98,7 @@ bar: {{ readFile "bar.txt" }}
 		return nil, fmt.Errorf("unexpected filename: %s", filename)
 	}
 
-	state, err := createFromYamlWithFileReader(yamlContent, yamlFile, "production", logger, readFile)
+	state, err := NewCreator(logger, readFile, filepath.Abs).CreateFromYaml(yamlContent, yamlFile, "production")
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -125,7 +126,7 @@ func TestReadFromYaml_StrictUnmarshalling(t *testing.T) {
   namespace: mynamespace
   releases: mychart
 `)
-	_, err := CreateFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
+	_, err := createFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
 	if err == nil {
 		t.Error("expected an error for wrong key 'releases' which is not in struct")
 	}
@@ -137,7 +138,7 @@ func TestReadFromYaml_DeprecatedReleaseReferences(t *testing.T) {
 - name: myrelease
   chart: mychart
 `)
-	state, err := CreateFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
+	state, err := createFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
 	if err != nil {
 		t.Errorf("unxpected error: %v", err)
 	}
@@ -159,7 +160,7 @@ releases:
 - name: myrelease2
   chart: mychart2
 `)
-	_, err := CreateFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
+	_, err := createFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
 	if err == nil {
 		t.Error("expected error")
 	}
@@ -195,7 +196,7 @@ func TestReadFromYaml_FilterReleasesOnLabels(t *testing.T) {
 		{LabelFilter{positiveLabels: [][]string{[]string{"tier", "frontend"}}, negativeLabels: [][]string{[]string{"foo", "bar"}}},
 			[]bool{false, true, false}},
 	}
-	state, err := CreateFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
+	state, err := createFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -234,7 +235,7 @@ func TestReadFromYaml_FilterNegatives(t *testing.T) {
 		{LabelFilter{negativeLabels: [][]string{[]string{"stage", "pre"}, []string{"stage", "post"}}},
 			[]bool{false, false, true}},
 	}
-	state, err := CreateFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
+	state, err := createFromYaml(yamlContent, yamlFile, DefaultEnv, logger)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}

state/state.go

@@ -742,10 +742,8 @@ func (state *HelmState) FilterReleases(labels []string) error {
 		filteredReleases = append(filteredReleases, r)
 	}
 	state.Releases = filteredReleases
-	if len(filteredReleases) == 0 {
+	numFound := len(filteredReleases)
-		state.logger.Debugf("specified selector did not match any releases in %s\n", state.FilePath)
+	state.logger.Debugf("%d release(s) matching %s found in %s\n", numFound, strings.Join(labels, ","), state.FilePath)
-		return nil
-	}
 	return nil
 }