Respect release filter in lint and status (#1672)

Ref #1190
Fixes #1651

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

main.go

@@ -5,9 +5,8 @@ import (
 	"os"
 	"strings"
 
-	"github.com/roboll/helmfile/pkg/app/version"
-
 	"github.com/roboll/helmfile/pkg/app"
+	"github.com/roboll/helmfile/pkg/app/version"
 	"github.com/roboll/helmfile/pkg/helmexec"
 	"github.com/roboll/helmfile/pkg/maputil"
 	"github.com/roboll/helmfile/pkg/state"
@@ -126,8 +125,8 @@ func main() {
 					Usage: `skip running "helm repo update" before running "helm dependency build"`,
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Deps(c)
+				return a.Deps(c)
 			}),
 		},
 		{
@@ -140,8 +139,8 @@ func main() {
 					Usage: "pass args to helm exec",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Repos(c)
+				return a.Repos(c)
 			}),
 		},
 		{
@@ -167,8 +166,8 @@ func main() {
 					Usage: "maximum number of concurrent helm processes to run, 0 is unlimited",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.DeprecatedSyncCharts(c)
+				return a.DeprecatedSyncCharts(c)
 			}),
 		},
 		{
@@ -219,8 +218,8 @@ func main() {
 					Usage: "output NUM lines of context around changes",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Diff(c)
+				return a.Diff(c)
 			}),
 		},
 		{
@@ -270,8 +269,8 @@ func main() {
 					Usage: "Stop cleaning up temporary values generated by helmfile and helm-secrets. Useful for debugging. Don't use in production for security",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Template(c)
+				return a.Template(c)
 			}),
 		},
 		{
@@ -300,8 +299,8 @@ func main() {
 					Usage: `skip running "helm repo update" and "helm dependency build"`,
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.WriteValues(c)
+				return a.WriteValues(c)
 			}),
 		},
 		{
@@ -331,8 +330,8 @@ func main() {
 					Usage: `skip running "helm repo update" and "helm dependency build"`,
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Lint(c)
+				return a.Lint(c)
 			}),
 		},
 		{
@@ -392,8 +391,8 @@ func main() {
 					Usage: `Override helmDefaults.waitForJobs setting "helm upgrade --install --wait-for-jobs"`,
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Sync(c)
+				return a.Sync(c)
 			}),
 		},
 		{
@@ -468,8 +467,8 @@ func main() {
 					Usage: `Override helmDefaults.waitForJobs setting "helm upgrade --install --wait-for-jobs"`,
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Apply(c)
+				return a.Apply(c)
 			}),
 		},
 		{
@@ -487,8 +486,8 @@ func main() {
 					Usage: "pass args to helm exec",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Status(c)
+				return a.Status(c)
 			}),
 		},
 		{
@@ -510,8 +509,8 @@ func main() {
 					Usage: "purge releases i.e. free release names and histories",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Delete(c)
+				return a.Delete(c)
 			}),
 		},
 		{
@@ -529,8 +528,8 @@ func main() {
 					Usage: "pass args to helm exec",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Destroy(c)
+				return a.Destroy(c)
 			}),
 		},
 		{
@@ -561,8 +560,8 @@ func main() {
 					Usage: "maximum number of concurrent helm processes to run, 0 is unlimited",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.Test(c)
+				return a.Test(c)
 			}),
 		},
 		{
@@ -574,8 +573,8 @@ func main() {
 					Usage: "Read all the values files for every release and embed into the output helmfile.yaml",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.PrintState(c)
+				return a.PrintState(c)
 			}),
 		},
 		{
@@ -592,8 +591,8 @@ func main() {
 					Usage: "Keep temporary directory",
 				},
 			},
-			Action: action(func(run *app.App, c configImpl) error {
+			Action: action(func(a *app.App, c configImpl) error {
-				return run.ListReleases(c)
+				return a.ListReleases(c)
 			}),
 		},
 		{

pkg/app/app.go

@@ -275,14 +275,14 @@ func (a *App) WriteValues(c WriteValuesConfigProvider) error {
 }
 
 func (a *App) Lint(c LintConfigProvider) error {
-	return a.ForEachState(func(run *Run) (_ bool, errs []error) {
+	return a.ForEachState(func(run *Run) (ok bool, errs []error) {
 		// `helm lint` on helm v2 and v3 does not support remote charts, that we need to set `forceDownload=true` here
 		prepErr := run.withPreparedCharts("lint", state.ChartPrepareOptions{
 			ForceDownload: true,
 			SkipRepos:     c.SkipDeps(),
 			SkipDeps:      c.SkipDeps(),
 		}, func() {
-			errs = run.Lint(c)
+			ok, errs = a.lint(run, c)
 		})
 
 		if prepErr != nil {
@@ -376,12 +376,12 @@ func (a *App) Apply(c ApplyConfigProvider) error {
 }
 
 func (a *App) Status(c StatusesConfigProvider) error {
-	return a.ForEachState(func(run *Run) (_ bool, errs []error) {
+	return a.ForEachState(func(run *Run) (ok bool, errs []error) {
 		err := run.withPreparedCharts("status", state.ChartPrepareOptions{
 			SkipRepos: true,
 			SkipDeps:  true,
 		}, func() {
-			errs = run.Status(c)
+			ok, errs = a.status(run, c)
 		})
 
 		if err != nil {
@@ -1307,6 +1307,184 @@ Do you really want to delete?
 	return true, errs
 }
 
+func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) {
+	st := r.state
+
+	allReleases := st.GetReleasesWithOverrides()
+
+	toDiff, err := a.getSelectedReleases(r)
+	if err != nil {
+		return nil, false, false, []error{err}
+	}
+
+	if len(toDiff) == 0 {
+		return nil, false, false, nil
+	}
+
+	// Do build deps and prepare only on selected releases so that we won't waste time
+	// on running various helm commands on unnecessary releases
+	st.Releases = toDiff
+
+	r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
+
+	opts := &state.DiffOpts{
+		Context: c.Context(),
+		NoColor: c.NoColor(),
+		Set:     c.Set(),
+	}
+
+	// Validate all releases for missing `needs` targets
+	st.Releases = allReleases
+
+	if _, err := st.PlanReleases(false); err != nil {
+		return nil, false, false, []error{err}
+	}
+
+	// Diff only targeted releases
+
+	st.Releases = toDiff
+
+	filtered := &Run{
+		state: st,
+		helm:  r.helm,
+		ctx:   r.ctx,
+		Ask:   r.Ask,
+	}
+
+	infoMsg, updated, deleted, errs := filtered.diff(true, c.DetailedExitcode(), c, opts)
+
+	return infoMsg, true, len(deleted) > 0 || len(updated) > 0, errs
+}
+
+func (a *App) lint(r *Run, c LintConfigProvider) (bool, []error) {
+	st := r.state
+	helm := r.helm
+
+	allReleases := st.GetReleasesWithOverrides()
+
+	selectedReleases, err := a.getSelectedReleases(r)
+	if err != nil {
+		return false, []error{err}
+	}
+	if len(selectedReleases) == 0 {
+		return false, nil
+	}
+
+	// Do build deps and prepare only on selected releases so that we won't waste time
+	// on running various helm commands on unnecessary releases
+	st.Releases = selectedReleases
+
+	releasesToRender := map[string]state.ReleaseSpec{}
+	for _, r := range selectedReleases {
+		id := state.ReleaseToID(&r)
+		if r.Installed != nil && !*r.Installed {
+			continue
+		}
+		releasesToRender[id] = r
+	}
+
+	var errs []error
+
+	// Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies
+	st.Releases = allReleases
+
+	args := argparser.GetArgs(c.Args(), st)
+
+	// Reset the extra args if already set, not to break `helm fetch` by adding the args intended for `lint`
+	helm.SetExtraArgs()
+
+	if len(args) > 0 {
+		helm.SetExtraArgs(args...)
+	}
+
+	if len(releasesToRender) > 0 {
+		_, templateErrs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error {
+			var rs []state.ReleaseSpec
+
+			for _, r := range subst.Releases {
+				if r2, ok := releasesToRender[state.ReleaseToID(&r)]; ok {
+					rs = append(rs, r2)
+				}
+			}
+
+			subst.Releases = rs
+
+			opts := &state.LintOpts{
+				Set: c.Set(),
+			}
+			return subst.LintReleases(helm, c.Values(), args, c.Concurrency(), opts)
+		}))
+
+		if templateErrs != nil && len(templateErrs) > 0 {
+			errs = append(errs, templateErrs...)
+		}
+	}
+	return true, errs
+}
+
+func (a *App) status(r *Run, c StatusesConfigProvider) (bool, []error) {
+	st := r.state
+	helm := r.helm
+
+	allReleases := st.GetReleasesWithOverrides()
+
+	selectedReleases, err := a.getSelectedReleases(r)
+	if err != nil {
+		return false, []error{err}
+	}
+	if len(selectedReleases) == 0 {
+		return false, nil
+	}
+
+	// Do build deps and prepare only on selected releases so that we won't waste time
+	// on running various helm commands on unnecessary releases
+	st.Releases = selectedReleases
+
+	releasesToRender := map[string]state.ReleaseSpec{}
+	for _, r := range selectedReleases {
+		id := state.ReleaseToID(&r)
+		if r.Installed != nil && !*r.Installed {
+			continue
+		}
+		releasesToRender[id] = r
+	}
+
+	var errs []error
+
+	// Traverse DAG of all the releases so that we don't suffer from false-positive missing dependencies
+	st.Releases = allReleases
+
+	args := argparser.GetArgs(c.Args(), st)
+
+	// Reset the extra args if already set, not to break `helm fetch` by adding the args intended for `lint`
+	helm.SetExtraArgs()
+
+	if len(args) > 0 {
+		helm.SetExtraArgs(args...)
+	}
+
+	if len(releasesToRender) > 0 {
+		_, templateErrs := withDAG(st, helm, a.Logger, false, a.Wrap(func(subst *state.HelmState, helm helmexec.Interface) []error {
+			var rs []state.ReleaseSpec
+
+			for _, r := range subst.Releases {
+				if r2, ok := releasesToRender[state.ReleaseToID(&r)]; ok {
+					rs = append(rs, r2)
+				}
+			}
+
+			subst.Releases = rs
+
+			return subst.ReleaseStatuses(helm, c.Concurrency())
+		}))
+
+		if templateErrs != nil && len(templateErrs) > 0 {
+			errs = append(errs, templateErrs...)
+		}
+	}
+	return true, errs
+}
+
 func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) {
 	st := r.state
 	helm := r.helm
@@ -1508,6 +1686,31 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) {
 	return true, errs
 }
 
+func (a *App) test(r *Run, c TestConfigProvider) []error {
+	cleanup := c.Cleanup()
+	timeout := c.Timeout()
+	concurrency := c.Concurrency()
+
+	st := r.state
+
+	toTest, err := a.getSelectedReleases(r)
+	if err != nil {
+		return []error{err}
+	}
+
+	if len(toTest) == 0 {
+		return nil
+	}
+
+	// Do test only on selected releases, because that's what the user intended
+	// with conditions and selectors
+	st.Releases = toTest
+
+	r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
+
+	return st.TestReleases(r.helm, cleanup, timeout, concurrency, state.Logs(c.Logs()))
+}
+
 func (a *App) writeValues(r *Run, c WriteValuesConfigProvider) (bool, []error) {
 	st := r.state
 	helm := r.helm

pkg/app/run.go

@@ -117,105 +117,10 @@ func (r *Run) DeprecatedSyncCharts(c DeprecatedChartsConfigProvider) []error {
 	return errs
 }
 
-func (r *Run) Status(c StatusesConfigProvider) []error {
+func (r *Run) diff(triggerCleanupEvent bool, detailedExitCode bool, c DiffConfigProvider, diffOpts *state.DiffOpts) (*string, map[string]state.ReleaseSpec, map[string]state.ReleaseSpec, []error) {
-	workers := c.Concurrency()
-
-	r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
-
-	return r.state.ReleaseStatuses(r.helm, workers)
-}
-
-func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) {
-	st := r.state
-
-	allReleases := st.GetReleasesWithOverrides()
-
-	toDiff, err := a.getSelectedReleases(r)
-	if err != nil {
-		return nil, false, false, []error{err}
-	}
-
-	if len(toDiff) == 0 {
-		return nil, false, false, nil
-	}
-
-	// Do build deps and prepare only on selected releases so that we won't waste time
-	// on running various helm commands on unnecessary releases
-	st.Releases = toDiff
-
-	r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
-
-	opts := &state.DiffOpts{
-		Context: c.Context(),
-		NoColor: c.NoColor(),
-		Set:     c.Set(),
-	}
-
-	// Validate all releases for missing `needs` targets
-	st.Releases = allReleases
-
-	if _, err := st.PlanReleases(false); err != nil {
-		return nil, false, false, []error{err}
-	}
-
-	// Diff only targeted releases
-
-	st.Releases = toDiff
-
-	filtered := &Run{
-		state: st,
-		helm:  r.helm,
-		ctx:   r.ctx,
-		Ask:   r.Ask,
-	}
-
-	infoMsg, updated, deleted, errs := filtered.diff(true, c.DetailedExitcode(), c, opts)
-
-	return infoMsg, true, len(deleted) > 0 || len(updated) > 0, errs
-}
-
-func (a *App) test(r *Run, c TestConfigProvider) []error {
-	cleanup := c.Cleanup()
-	timeout := c.Timeout()
-	concurrency := c.Concurrency()
-
-	st := r.state
-
-	toTest, err := a.getSelectedReleases(r)
-	if err != nil {
-		return []error{err}
-	}
-
-	if len(toTest) == 0 {
-		return nil
-	}
-
-	// Do test only on selected releases, because that's what the user intended
-	// with conditions and selectors
-	st.Releases = toTest
-
-	r.helm.SetExtraArgs(argparser.GetArgs(c.Args(), r.state)...)
-
-	return st.TestReleases(r.helm, cleanup, timeout, concurrency, state.Logs(c.Logs()))
-}
-
-func (r *Run) Lint(c LintConfigProvider) []error {
 	st := r.state
 	helm := r.helm
 
-	values := c.Values()
-	args := argparser.GetArgs(c.Args(), st)
-	workers := c.Concurrency()
-	opts := &state.LintOpts{
-		Set: c.Set(),
-	}
-	return st.LintReleases(helm, values, args, workers, opts)
-}
-
-func (run *Run) diff(triggerCleanupEvent bool, detailedExitCode bool, c DiffConfigProvider, diffOpts *state.DiffOpts) (*string, map[string]state.ReleaseSpec, map[string]state.ReleaseSpec, []error) {
-	st := run.state
-	helm := run.helm
-
 	var changedReleases []state.ReleaseSpec
 	var deletingReleases []state.ReleaseSpec
 	var planningErrs []error

test/integration/environment.values.yaml

@@ -1 +1,3 @@
 mysecret: MYSECRET
+raw2:
+  enabled: false

test/integration/happypath.yaml

@@ -46,3 +46,11 @@ releases:
             path: /metadata/annotations
             value:
               foo: bar
+
+  - name: raw2
+    chart: center/incubator/raw
+    version: 0.2.3
+    condition: raw2.enabled
+    values:
+      - mysecret: {{ .Environment.Values.mysecret }}
+      - values.yaml