rfix helmfile should fail on duplicate release name after filtered by…

… labels

This is a follow-up for roboll#218, fixes the unintentional degradation that broken the use-case described in roboll#193 (comment)

main.go

@@ -397,7 +397,18 @@ func eachDesiredStateDo(c *cli.Context, converge func(*state.HelmState, helmexec
 	}
 	allSelectorNotMatched := true
 	for _, f := range desiredStateFiles {
-		state, helm, noReleases, err := loadDesiredStateFromFile(c, f)
+		yamlBuf, err := state.RenderTemplateFileToBuffer(f)
+		if err != nil {
+			return err
+		}
+		state, helm, noReleases, err := loadDesiredStateFromFile(
+			yamlBuf.Bytes(),
+			f,
+			c.GlobalString("kube-context"),
+			c.GlobalString("namespace"),
+			c.GlobalStringSlice("selector"),
+			c.App.Metadata["logger"].(*zap.SugaredLogger),
+		)
 		if err != nil {
 			return err
 		}
@@ -474,14 +485,8 @@ func directoryExistsAt(path string) bool {
 	return err == nil && fileInfo.Mode().IsDir()
 }
 
-func loadDesiredStateFromFile(c *cli.Context, file string) (*state.HelmState, helmexec.Interface, bool, error) {
-	kubeContext := c.GlobalString("kube-context")
-	namespace := c.GlobalString("namespace")
-	labels := c.GlobalStringSlice("selector")
-
-	logger := c.App.Metadata["logger"].(*zap.SugaredLogger)
-
-	st, err := state.CreateFromFile(file, logger)
+func loadDesiredStateFromFile(yaml []byte, file string, kubeContext, namespace string, labels []string, logger *zap.SugaredLogger) (*state.HelmState, helmexec.Interface, bool, error) {
+	st, err := state.CreateFromYaml(yaml, file, logger)
 	if err != nil {
 		return nil, nil, false, fmt.Errorf("failed to read %s: %v", file, err)
 	}
@@ -510,6 +515,16 @@ func loadDesiredStateFromFile(c *cli.Context, file string) (*state.HelmState, he
 		}
 	}
 
+	releaseNameCounts := map[string]int{}
+	for _, r := range st.Releases {
+		releaseNameCounts[r.Name] += 1
+	}
+	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)
+		}
+	}
+
 	sigs := make(chan os.Signal, 1)
 	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
 	go func() {

main_test.go

@@ -0,0 +1,26 @@
+package main
+
+import "testing"
+
+// See https://github.com/roboll/helmfile/issues/193
+func TestReadFromYaml_DuplicateReleaseName(t *testing.T) {
+	yamlFile := "example/path/to/yaml/file"
+	yamlContent := []byte(`releases:
+- name: myrelease1
+  chart: mychart1
+  labels:
+    stage: pre
+    foo: bar
+- name: myrelease1
+  chart: mychart2
+  labels:
+    stage: post
+`)
+	_, _, _, err := loadDesiredStateFromFile(yamlContent, yamlFile, "default", "default", []string{}, logger)
+	if err == nil {
+		t.Error("error expected but not happened")
+	}
+	if err.Error() != "duplicate release \"myrelease1\" found: there were 2 releases named \"myrelease1\" matching specified selector" {
+		t.Errorf("unexpected error happened: %v", err)
+	}
+}

state/state.go

@@ -84,17 +84,17 @@ type SetValue struct {
 	File  string `yaml:"file"`
 }
 
-// CreateFromFile loads the helmfile from disk and processes the template
-func CreateFromFile(file string, logger *zap.SugaredLogger) (*HelmState, error) {
-	yamlBuf, err := renderTemplateFileToBuffer(file)
+// CreateFromTemplateFile loads the helmfile from disk and processes the template
+func CreateFromTemplateFile(file string, logger *zap.SugaredLogger) (*HelmState, error) {
+	yamlBuf, err := RenderTemplateFileToBuffer(file)
 	if err != nil {
 		return nil, err
 	}
 
-	return readFromYaml(yamlBuf.Bytes(), file, logger)
+	return CreateFromYaml(yamlBuf.Bytes(), file, logger)
 }
 
-func readFromYaml(content []byte, file string, logger *zap.SugaredLogger) (*HelmState, error) {
+func CreateFromYaml(content []byte, file string, logger *zap.SugaredLogger) (*HelmState, error) {
 	var state HelmState
 
 	state.BaseChartPath, _ = filepath.Abs(filepath.Dir(file))
@@ -111,16 +111,6 @@ func readFromYaml(content []byte, file string, logger *zap.SugaredLogger) (*Helm
 		state.DeprecatedReleases = []ReleaseSpec{}
 	}
 
-	releaseNameCounts := map[string]int{}
-	for _, r := range state.Releases {
-		releaseNameCounts[r.Name] += 1
-	}
-	for name, c := range releaseNameCounts {
-		if c > 1 {
-			return nil, fmt.Errorf("invalid helmfile: duplicate release name found: there were %d releases named \"%s\"", c, name)
-		}
-	}
-
 	state.logger = logger
 
 	return &state, nil
@@ -144,7 +134,7 @@ func getRequiredEnv(name string) (string, error) {
 	return "", fmt.Errorf("required env var `%s` is not set", name)
 }
 
-func renderTemplateFileToBuffer(file string) (*bytes.Buffer, error) {
+func RenderTemplateFileToBuffer(file string) (*bytes.Buffer, error) {
 	content, err := ioutil.ReadFile(file)
 	if err != nil {
 		return nil, err
@@ -712,7 +702,7 @@ func (state *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, basePat
 			if _, err := os.Stat(path); os.IsNotExist(err) {
 				return nil, err
 			}
-			yamlBuf, err := renderTemplateFileToBuffer(path)
+			yamlBuf, err := RenderTemplateFileToBuffer(path)
 			if err != nil {
 				return nil, err
 			}

state/state_test.go

@@ -19,7 +19,7 @@ func TestReadFromYaml(t *testing.T) {
   namespace: mynamespace
   chart: mychart
 `)
-	state, err := readFromYaml(yamlContent, yamlFile, logger)
+	state, err := CreateFromYaml(yamlContent, yamlFile, logger)
 	if err != nil {
 		t.Errorf("unxpected error: %v", err)
 	}
@@ -42,7 +42,7 @@ func TestReadFromYaml_StrictUnmarshalling(t *testing.T) {
   namespace: mynamespace
   releases: mychart
 `)
-	_, err := readFromYaml(yamlContent, yamlFile, logger)
+	_, err := CreateFromYaml(yamlContent, yamlFile, logger)
 	if err == nil {
 		t.Error("expected an error for wrong key 'releases' which is not in struct")
 	}
@@ -54,7 +54,7 @@ func TestReadFromYaml_DeprecatedReleaseReferences(t *testing.T) {
 - name: myrelease
   chart: mychart
 `)
-	state, err := readFromYaml(yamlContent, yamlFile, logger)
+	state, err := CreateFromYaml(yamlContent, yamlFile, logger)
 	if err != nil {
 		t.Errorf("unxpected error: %v", err)
 	}
@@ -76,7 +76,7 @@ releases:
 - name: myrelease2
   chart: mychart2
 `)
-	_, err := readFromYaml(yamlContent, yamlFile, logger)
+	_, err := CreateFromYaml(yamlContent, yamlFile, logger)
 	if err == nil {
 		t.Error("expected error")
 	}
@@ -112,7 +112,7 @@ func TestReadFromYaml_FilterReleasesOnLabels(t *testing.T) {
 		{LabelFilter{positiveLabels: [][]string{[]string{"tier", "frontend"}}, negativeLabels: [][]string{[]string{"foo", "bar"}}},
 			[]bool{false, true, false}},
 	}
-	state, err := readFromYaml(yamlContent, yamlFile, logger)
+	state, err := CreateFromYaml(yamlContent, yamlFile, logger)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -151,7 +151,7 @@ func TestReadFromYaml_FilterNegatives(t *testing.T) {
 		{LabelFilter{negativeLabels: [][]string{[]string{"stage", "pre"}, []string{"stage", "post"}}},
 			[]bool{false, false, true}},
 	}
-	state, err := readFromYaml(yamlContent, yamlFile, logger)
+	state, err := CreateFromYaml(yamlContent, yamlFile, logger)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -164,29 +164,6 @@ func TestReadFromYaml_FilterNegatives(t *testing.T) {
 	}
 }
 
-// See https://github.com/roboll/helmfile/issues/193
-func TestReadFromYaml_DuplicateReleaseName(t *testing.T) {
-	yamlFile := "example/path/to/yaml/file"
-	yamlContent := []byte(`releases:
-- name: myrelease1
-  chart: mychart1
-  labels:
-    stage: pre
-    foo: bar
-- name: myrelease1
-  chart: mychart2
-  labels:
-    stage: post
-`)
-	_, err := readFromYaml(yamlContent, yamlFile, logger)
-	if err == nil {
-		t.Error("error expected but not happened")
-	}
-	if err.Error() != "invalid helmfile: duplicate release name found: there were 2 releases named \"myrelease1\"" {
-		t.Errorf("unexpected error happened: %v", err)
-	}
-}
-
 func TestLabelParsing(t *testing.T) {
 	cases := []struct {
 		labelString    string