Correct enabled property in helmfile list (#1921)

Use the value of the `condition` field instead of the `installed` field of a release in the `enabled` column of helmfile list.
The value of the `installed` field is shown in a new `installed` column.

Fixes #1920

Co-authored-by: Yusuke Kuoka <ykuoka@gmail.com>

pkg/app/app.go

@@ -61,6 +61,7 @@ type HelmRelease struct {
 	Name      string `json:"name"`
 	Namespace string `json:"namespace"`
 	Enabled   bool   `json:"enabled"`
+	Installed bool   `json:"installed"`
 	Labels    string `json:"labels"`
 	Chart     string `json:"chart"`
 	Version   string `json:"version"`
@@ -575,11 +576,17 @@ func (a *App) ListReleases(c ListConfigProvider) error {
 				}
 				labels = strings.Trim(labels, ",")
 
+				enabled, err := state.ConditionEnabled(r, run.state.Values())
+				if err != nil {
+					panic(err)
+				}
+
 				installed := r.Installed == nil || *r.Installed
 				releases = append(releases, &HelmRelease{
 					Name:      r.Name,
 					Namespace: r.Namespace,
-					Enabled:   installed,
+					Installed: installed,
+					Enabled:   enabled,
 					Labels:    labels,
 					Chart:     r.Chart,
 					Version:   r.Version,

pkg/app/app_test.go

@@ -16,15 +16,18 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+
 	"github.com/roboll/helmfile/pkg/remote"
 
-	"github.com/roboll/helmfile/pkg/exectest"
 	"gotest.tools/v3/assert"
 
+	"github.com/roboll/helmfile/pkg/exectest"
+
+	"github.com/variantdev/vals"
+
 	"github.com/roboll/helmfile/pkg/helmexec"
 	"github.com/roboll/helmfile/pkg/state"
 	"github.com/roboll/helmfile/pkg/testhelper"
-	"github.com/variantdev/vals"
 
 	"go.uber.org/zap"
 	"gotest.tools/v3/env"
@@ -4803,6 +4806,11 @@ func TestList(t *testing.T) {
 		"/path/to/helmfile.d/first.yaml": `
 commonLabels:
   common: label
+environments:
+  default:
+    values:
+     - myrelease2:
+         enabled: false
 releases:
 - name: myrelease1
   chart: mychart1
@@ -4811,6 +4819,7 @@ releases:
     id: myrelease1
 - name: myrelease2
   chart: mychart1
+  condition: myrelease2.enabled
 `,
 		"/path/to/helmfile.d/second.yaml": `
 releases:
@@ -4846,18 +4855,24 @@ releases:
 		assert.NilError(t, err)
 	})
 
-	expected := `NAME      	NAMESPACE	ENABLED	LABELS                    	CHART   	VERSION
-myrelease1	         	false  	common:label,id:myrelease1	mychart1	       
-myrelease2	         	true   	common:label              	mychart1	       
-myrelease3	         	true   	                          	mychart1	       
-myrelease4	         	true   	id:myrelease1             	mychart1	       
+	expected := `NAME      	NAMESPACE	ENABLED	INSTALLED	LABELS                    	CHART   	VERSION
+myrelease1	         	true   	false    	common:label,id:myrelease1	mychart1	       
+myrelease2	         	false  	true     	common:label              	mychart1	       
+myrelease3	         	true   	true     	                          	mychart1	       
+myrelease4	         	true   	true     	id:myrelease1             	mychart1	       
 `
+
 	assert.Equal(t, expected, out)
 }
 
 func TestListWithJsonOutput(t *testing.T) {
 	files := map[string]string{
 		"/path/to/helmfile.d/first.yaml": `
+environments:
+  default:
+    values:
+     - myrelease2:
+         enabled: false
 releases:
 - name: myrelease1
   chart: mychart1
@@ -4866,6 +4881,7 @@ releases:
     id: myrelease1
 - name: myrelease2
   chart: mychart1
+  condition: myrelease2.enabled
 `,
 		"/path/to/helmfile.d/second.yaml": `
 releases:
@@ -4903,7 +4919,7 @@ releases:
 		assert.NilError(t, err)
 	})
 
-	expected := `[{"name":"myrelease1","namespace":"","enabled":false,"labels":"id:myrelease1","chart":"mychart1","version":""},{"name":"myrelease2","namespace":"","enabled":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease3","namespace":"","enabled":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease4","namespace":"","enabled":true,"labels":"id:myrelease1","chart":"mychart1","version":""}]
+	expected := `[{"name":"myrelease1","namespace":"","enabled":true,"installed":false,"labels":"id:myrelease1","chart":"mychart1","version":""},{"name":"myrelease2","namespace":"","enabled":false,"installed":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease3","namespace":"","enabled":true,"installed":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease4","namespace":"","enabled":true,"installed":true,"labels":"id:myrelease1","chart":"mychart1","version":""}]
 `
 	assert.Equal(t, expected, out)
 }

pkg/app/formatters.go

@@ -9,10 +9,10 @@ import (
 
 func FormatAsTable(releases []*HelmRelease) error {
 	table := uitable.New()
-	table.AddRow("NAME", "NAMESPACE", "ENABLED", "LABELS", "CHART", "VERSION")
+	table.AddRow("NAME", "NAMESPACE", "ENABLED", "INSTALLED", "LABELS", "CHART", "VERSION")
 
 	for _, r := range releases {
-		table.AddRow(r.Name, r.Namespace, fmt.Sprintf("%t", r.Enabled), r.Labels, r.Chart, r.Version)
+		table.AddRow(r.Name, r.Namespace, fmt.Sprintf("%t", r.Enabled), fmt.Sprintf("%t", r.Installed), r.Labels, r.Chart, r.Version)
 	}
 
 	fmt.Println(table.String())

pkg/state/state.go

@@ -2075,25 +2075,13 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, commonLabe
 			}
 		}
 		var conditionMatch bool
-		if len(r.Condition) > 0 {
-			conditionSplit := strings.Split(r.Condition, ".")
-			if len(conditionSplit) != 2 {
-				return nil, fmt.Errorf("Condition value must be in the form 'foo.enabled' where 'foo' can be modified as necessary")
-			}
-			if v, ok := values[conditionSplit[0]]; ok {
-				if v == nil {
-					panic(fmt.Sprintf("environment values field '%s' is nil", conditionSplit[0]))
-				}
-				if v.(map[string]interface{})["enabled"] == true {
-					conditionMatch = true
-				}
-			} else {
-				panic(fmt.Sprintf("environment values does not contain field '%s'", conditionSplit[0]))
-			}
+		conditionMatch, err := ConditionEnabled(r, values)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse condition in release %s: %w", r.Name, err)
 		}
 		res := Release{
 			ReleaseSpec: r,
-			Filtered:    (len(filters) > 0 && !filterMatch) || (len(r.Condition) > 0 && !conditionMatch),
+			Filtered:    (len(filters) > 0 && !filterMatch) || (!conditionMatch),
 		}
 		filteredReleases = append(filteredReleases, res)
 	}
@@ -2103,6 +2091,29 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, commonLabe
 	return filteredReleases, nil
 }
 
+func ConditionEnabled(r ReleaseSpec, values map[string]interface{}) (bool, error) {
+	var conditionMatch bool
+	if len(r.Condition) == 0 {
+		return true, nil
+	}
+	conditionSplit := strings.Split(r.Condition, ".")
+	if len(conditionSplit) != 2 {
+		return false, fmt.Errorf("Condition value must be in the form 'foo.enabled' where 'foo' can be modified as necessary")
+	}
+	if v, ok := values[conditionSplit[0]]; ok {
+		if v == nil {
+			panic(fmt.Sprintf("environment values field '%s' is nil", conditionSplit[0]))
+		}
+		if v.(map[string]interface{})["enabled"] == true {
+			conditionMatch = true
+		}
+	} else {
+		panic(fmt.Sprintf("environment values does not contain field '%s'", conditionSplit[0]))
+	}
+
+	return conditionMatch, nil
+}
+
 func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []ReleaseSpec) {
 	needsWithTranstives := collectAllNeedsWithTransitives(filteredReleases, allReleases)
 	unmarkReleases(needsWithTranstives, filteredReleases)