From 346e318fd0b171e01b0eb6152ef6c23939b48214 Mon Sep 17 00:00:00 2001 From: Christoph Petrausch <263448+hikhvar@users.noreply.github.com> Date: Mon, 10 Jan 2022 09:24:07 +0100 Subject: [PATCH] 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 --- pkg/app/app.go | 9 ++++++++- pkg/app/app_test.go | 32 ++++++++++++++++++++++++-------- pkg/app/formatters.go | 4 ++-- pkg/state/state.go | 43 +++++++++++++++++++++++++++---------------- 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 4b0beeb1..8dcbd385 100644 --- a/pkg/app/app.go +++ b/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, diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 60bfa97f..549926b8 100644 --- a/pkg/app/app_test.go +++ b/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) } diff --git a/pkg/app/formatters.go b/pkg/app/formatters.go index b285a1f6..7ec9d886 100644 --- a/pkg/app/formatters.go +++ b/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()) diff --git a/pkg/state/state.go b/pkg/state/state.go index 41b42a09..29345d38 100644 --- a/pkg/state/state.go +++ b/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)