diff --git a/pkg/app/app.go b/pkg/app/app.go index cdc31745..1e7c5887 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -131,7 +131,7 @@ func (a *App) Deps(c DepsConfigProvider) error { return a.ForEachState(func(run *Run) (_ bool, errs []error) { errs = run.Deps(c) return - }, c.IncludeTransitiveNeeds(), SetFilter(true)) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), SetFilter(true)) } func (a *App) Repos(c ReposConfigProvider) error { @@ -143,7 +143,7 @@ func (a *App) Repos(c ReposConfigProvider) error { } return - }, c.IncludeTransitiveNeeds(), SetFilter(true)) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), SetFilter(true)) } func (a *App) Diff(c DiffConfigProvider) error { @@ -169,7 +169,8 @@ func (a *App) Diff(c DiffConfigProvider) error { IncludeCRDs: &includeCRDs, Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { msg, matched, affected, errs = a.diff(run, c) }) @@ -200,7 +201,7 @@ func (a *App) Diff(c DiffConfigProvider) error { } return matched, criticalErrs - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -240,7 +241,8 @@ func (a *App) Template(c TemplateConfigProvider) error { SkipCleanup: c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), Set: c.Set(), Values: c.Values(), KubeVersion: c.KubeVersion(), @@ -254,17 +256,19 @@ func (a *App) Template(c TemplateConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) } func (a *App) WriteValues(c WriteValuesConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { prepErr := run.withPreparedCharts("write-values", state.ChartPrepareOptions{ - SkipRepos: c.SkipRefresh() || c.SkipDeps(), - SkipRefresh: c.SkipRefresh(), - SkipDeps: c.SkipDeps(), - SkipCleanup: c.SkipCleanup(), - Concurrency: c.Concurrency(), + SkipRepos: c.SkipRefresh() || c.SkipDeps(), + SkipRefresh: c.SkipRefresh(), + SkipDeps: c.SkipDeps(), + SkipCleanup: c.SkipCleanup(), + IncludeNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), + Concurrency: c.Concurrency(), }, func() { ok, errs = a.writeValues(run, c) }) @@ -274,7 +278,7 @@ func (a *App) WriteValues(c WriteValuesConfigProvider) error { } return - }, c.IncludeTransitiveNeeds(), SetFilter(true)) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), SetFilter(true)) } type MultiError struct { @@ -317,7 +321,8 @@ func (a *App) Lint(c LintConfigProvider) error { SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { ok, lintErrs, errs = a.lint(run, c) }) @@ -331,7 +336,7 @@ func (a *App) Lint(c LintConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -358,6 +363,7 @@ func (a *App) Unittest(c UnittestConfigProvider) error { SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), + IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { ok, unittestErrs, errs = a.unittest(run, c) @@ -372,7 +378,7 @@ func (a *App) Unittest(c UnittestConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -402,7 +408,7 @@ func (a *App) Fetch(c FetchConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) Sync(c SyncConfigProvider) error { @@ -417,7 +423,8 @@ func (a *App) Sync(c SyncConfigProvider) error { WaitRetries: c.WaitRetries(), WaitForJobs: c.WaitForJobs(), IncludeCRDs: &includeCRDs, - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), Validate: c.Validate(), Concurrency: c.Concurrency(), }, func() { @@ -429,7 +436,7 @@ func (a *App) Sync(c SyncConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) } func (a *App) Apply(c ApplyConfigProvider) error { @@ -455,7 +462,8 @@ func (a *App) Apply(c ApplyConfigProvider) error { SkipCleanup: c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { matched, updated, es := a.apply(run, c) @@ -471,7 +479,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { } return - }, c.IncludeTransitiveNeeds(), opts...) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), opts...) if err != nil { return err @@ -501,7 +509,7 @@ func (a *App) Status(c StatusesConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) Destroy(c DestroyConfigProvider) error { @@ -524,7 +532,7 @@ func (a *App) Destroy(c DestroyConfigProvider) error { ok, errs = a.delete(run, true, c) } return - }, false, SetReverse(true)) + }, false, false, SetReverse(true)) } func (a *App) Test(c TestConfigProvider) error { @@ -549,7 +557,7 @@ func (a *App) Test(c TestConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) PrintDAGState(c DAGConfigProvider) error { @@ -566,7 +574,7 @@ func (a *App) PrintDAGState(c DAGConfigProvider) error { } }) return ok, errs - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) PrintState(c StateConfigProvider) error { @@ -619,7 +627,7 @@ func (a *App) PrintState(c StateConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) dag(r *Run) error { @@ -667,7 +675,7 @@ func (a *App) ListReleases(c ListConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) close(releasesChan) @@ -1211,7 +1219,7 @@ var ( } ) -func (a *App) ForEachState(do func(*Run) (bool, []error), includeTransitiveNeeds bool, o ...LoadOption) error { +func (a *App) ForEachState(do func(*Run) (bool, []error), includeNeeds bool, includeTransitiveNeeds bool, o ...LoadOption) error { ctx := NewContext() err := a.visitStatesWithSelectorsAndRemoteSupportWithContext(a.FileOrDir, func(st *state.HelmState) (bool, []error) { helm, err := a.getHelm(st) @@ -1224,7 +1232,7 @@ func (a *App) ForEachState(do func(*Run) (bool, []error), includeTransitiveNeeds return false, []error{err} } return do(run) - }, includeTransitiveNeeds, &ctx, o...) + }, includeNeeds, includeTransitiveNeeds, &ctx, o...) return err } @@ -1328,7 +1336,7 @@ type Opts struct { DAGEnabled bool } -func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir string, converge func(*state.HelmState) (bool, []error), includeTransitiveNeeds bool, sharedCtx *Context, opt ...LoadOption) error { +func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir string, converge func(*state.HelmState) (bool, []error), includeNeeds bool, includeTransitiveNeeds bool, sharedCtx *Context, opt ...LoadOption) error { opts := LoadOpts{ Selectors: a.Selectors, } @@ -1361,7 +1369,7 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir stri _, err := converge(st) return err }, - includeTransitiveNeeds) + includeNeeds, includeTransitiveNeeds) } } @@ -1382,9 +1390,9 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir stri return a.visitStatesWithContext(fileOrDir, opts, fHelmStatsWithOverrides, sharedCtx) } -func processFilteredReleases(st *state.HelmState, converge func(st *state.HelmState) []error, includeTransitiveNeeds bool) (bool, []error) { +func processFilteredReleases(st *state.HelmState, converge func(st *state.HelmState) []error, includeNeeds bool, includeTransitiveNeeds bool) (bool, []error) { if len(st.Selectors) > 0 { - err := st.FilterReleases(includeTransitiveNeeds) + err := st.FilterReleases(includeNeeds, includeTransitiveNeeds) if err != nil { return false, []error{err} } @@ -1430,11 +1438,11 @@ func checkDuplicates(releases []state.ReleaseSpec) error { return nil } -func (a *App) Wrap(converge func(*state.HelmState, helmexec.Interface) []error) func(st *state.HelmState, helm helmexec.Interface, includeTransitiveNeeds bool) (bool, []error) { - return func(st *state.HelmState, helm helmexec.Interface, includeTransitiveNeeds bool) (bool, []error) { +func (a *App) Wrap(converge func(*state.HelmState, helmexec.Interface) []error) func(st *state.HelmState, helm helmexec.Interface, includeNeeds bool, includeTransitiveNeeds bool) (bool, []error) { + return func(st *state.HelmState, helm helmexec.Interface, includeNeeds bool, includeTransitiveNeeds bool) (bool, []error) { return processFilteredReleases(st, func(st *state.HelmState) []error { return converge(st, helm) - }, includeTransitiveNeeds) + }, includeNeeds, includeTransitiveNeeds) } } @@ -1523,8 +1531,8 @@ func (a *App) findDesiredStateFiles(specifiedPath string, opts LoadOpts) ([]stri return files, nil } -func (a *App) getSelectedReleases(r *Run, includeTransitiveNeeds bool) ([]state.ReleaseSpec, []state.ReleaseSpec, error) { - selected, err := r.state.GetSelectedReleases(includeTransitiveNeeds) +func (a *App) getSelectedReleases(r *Run, includeNeeds bool, includeTransitiveNeeds bool) ([]state.ReleaseSpec, []state.ReleaseSpec, error) { + selected, err := r.state.GetSelectedReleases(includeNeeds, includeTransitiveNeeds) if err != nil { return nil, nil, err } @@ -1589,7 +1597,11 @@ func (a *App) getSelectedReleases(r *Run, includeTransitiveNeeds bool) ([]state. extra = " matching " + strings.Join(r.state.Selectors, ",") } - a.Logger.Debugf("%d release(s)%s found in %s\n", len(selected), extra, r.state.FilePath) + matchedOnly, err := r.state.GetSelectedReleases(false, false) + if err != nil { + return nil, nil, err + } + a.Logger.Debugf("%d release(s)%s found in %s\n", len(matchedOnly), extra, r.state.FilePath) return selected, deduplicated, nil } @@ -1600,7 +1612,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { helm.SetExtraArgs(GetArgs(c.Args(), r.state)...) - selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, c.IncludeTransitiveNeeds()) + selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return false, false, []error{err} } @@ -1804,7 +1816,7 @@ func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error affectedReleases := state.AffectedReleases{} - toSync, _, err := a.getSelectedReleases(r, false) + toSync, _, err := a.getSelectedReleases(r, false, false) if err != nil { return false, []error{err} } @@ -2024,7 +2036,7 @@ func (a *App) status(r *Run, c StatusesConfigProvider) (bool, []error) { allReleases := st.Releases - selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, false) + selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, false, false) if err != nil { return false, []error{err} } @@ -2073,7 +2085,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { st := r.state helm := r.helm - selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, c.IncludeTransitiveNeeds()) + selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return false, []error{err} } @@ -2287,7 +2299,7 @@ func (a *App) template(r *Run, c TemplateConfigProvider) (bool, []error) { func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state.HelmState) []error) (bool, []error) { st := r.state - selectedReleases, deduplicated, err := a.getSelectedReleases(r, false) + selectedReleases, deduplicated, err := a.getSelectedReleases(r, false, false) if err != nil { return false, []error{err} } @@ -2342,10 +2354,11 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state if len(toRender) > 0 { // toRender already contains the direct and transitive needs depending on the DAG options. - // That's why we don't pass in `IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds()` here. + // That's why we don't pass in `IncludeNeeds` or `IncludeTransitiveNeeds` here. // Otherwise, in case include-needs=true, it will include the needs of needs, which results in unexpectedly introducing transitive needs, // even if include-transitive-needs=true is unspecified. - if _, errs := withDAG(st, r.helm, a.Logger, state.PlanOptions{SelectedReleases: toRender, Reverse: false, SkipNeeds: c.SkipNeeds(), IncludeNeeds: includeNeeds}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { + // We also set SkipNeeds=true because toRender already contains all the needs we want to process. + if _, errs := withDAG(st, r.helm, a.Logger, state.PlanOptions{SelectedReleases: toRender, Reverse: false, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { rels = append(rels, subst.Releases...) return nil })); len(errs) > 0 { @@ -2387,7 +2400,7 @@ func (a *App) test(r *Run, c TestConfigProvider) []error { st := r.state - toTest, _, err := a.getSelectedReleases(r, false) + toTest, _, err := a.getSelectedReleases(r, false, false) if err != nil { return []error{err} } @@ -2409,7 +2422,7 @@ func (a *App) writeValues(r *Run, c WriteValuesConfigProvider) (bool, []error) { st := r.state helm := r.helm - toRender, _, err := a.getSelectedReleases(r, false) + toRender, _, err := a.getSelectedReleases(r, false, false) if err != nil { return false, []error{err} } diff --git a/pkg/app/app_sequential_test.go b/pkg/app/app_sequential_test.go index 101a4b5c..57823216 100644 --- a/pkg/app/app_sequential_test.go +++ b/pkg/app/app_sequential_test.go @@ -57,6 +57,7 @@ releases: err = app.ForEachState( Noop, false, + false, SetFilter(true), ) if err != nil { @@ -192,7 +193,8 @@ releases: err = app.ForEachState( noop, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -405,7 +407,8 @@ releases: err = app.ForEachState( failingConverge, false, - SetFilter(true), + + false, SetFilter(true), ) if err == nil { @@ -472,7 +475,8 @@ replicaCount: 3 err = app.ForEachState( captureState, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index ffc500fb..ce40b48c 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -113,7 +113,8 @@ releases: err := app.ForEachState( noop, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Errorf("unexpected error: %v", err) @@ -167,7 +168,8 @@ BAZ: 4 err := app.ForEachState( Noop, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Errorf("unexpected error: %v", err) @@ -211,7 +213,8 @@ releases: err := app.ForEachState( Noop, false, - SetFilter(true), + + false, SetFilter(true), ) if err == nil { t.Fatal("expected error did not occur") @@ -298,7 +301,8 @@ func TestUpdateStrategyParamValidation(t *testing.T) { err := app.ForEachState( Noop, false, - SetFilter(true), + + false, SetFilter(true), ) if c.isValid && err != nil { @@ -350,7 +354,8 @@ releases: err := app.ForEachState( Noop, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Errorf("unexpected error: %v", err) @@ -404,7 +409,8 @@ releases: err := app.ForEachState( Noop, false, - SetFilter(true), + + false, SetFilter(true), ) if testcase.expectErr && err == nil { t.Fatal("expected error did not occur") @@ -472,7 +478,8 @@ releases: err := app.ForEachState( Noop, false, - SetFilter(true), + + false, SetFilter(true), ) if testcase.expectErr && err == nil { t.Errorf("error expected but not happened for name=%s", testcase.name) @@ -528,7 +535,8 @@ releases: err := app.ForEachState( Noop, false, - SetFilter(true), + + false, SetFilter(true), ) if testcase.expectErr && err == nil { t.Errorf("error expected but not happened for environment=%s", testcase.name) @@ -643,7 +651,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if testcase.expectErr { if err == nil { @@ -885,7 +894,8 @@ func runFilterSubHelmFilesTests(testcases []struct { err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if testcase.expectErr { if err == nil { @@ -975,7 +985,8 @@ ns: INLINE_NS err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { @@ -1073,6 +1084,7 @@ releases: err := app.ForEachState( collectReleases, false, + false, SetReverse(testcase.reverse), SetFilter(true), ) @@ -1140,7 +1152,8 @@ bar: "bar1" err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1262,7 +1275,8 @@ x: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1314,7 +1328,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1371,7 +1386,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1421,7 +1437,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { @@ -1464,7 +1481,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) expected := "in ./helmfile.yaml: duplicate release \"foo\" found in namespace \"foo\" in kubecontext \"default\": there were 2 releases named \"foo\" matching specified selector" @@ -1511,7 +1529,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) expected := "in ./helmfile.yaml: duplicate release \"foo\" found in namespace \"foo\" in kubecontext \"default\": there were 2 releases named \"foo\" matching specified selector" @@ -2607,22 +2626,27 @@ func (a applyConfig) TrackLogs() bool { type depsConfig struct { skipRepos bool + includeNeeds bool includeTransitiveNeeds bool } -func (d depsConfig) SkipRepos() bool { - return d.skipRepos +func (c depsConfig) SkipRepos() bool { + return c.skipRepos } -func (d depsConfig) IncludeTransitiveNeeds() bool { - return d.includeTransitiveNeeds +func (c depsConfig) IncludeNeeds() bool { + return c.includeNeeds } -func (d depsConfig) Args() string { +func (c depsConfig) IncludeTransitiveNeeds() bool { + return c.includeTransitiveNeeds +} + +func (c depsConfig) Args() string { return "" } -func (d depsConfig) Concurrency() int { +func (c depsConfig) Concurrency() int { return 2 } @@ -4097,6 +4121,7 @@ releases: depsErr := app.Deps(depsConfig{ skipRepos: false, + includeNeeds: false, includeTransitiveNeeds: false, }) switch { @@ -4331,7 +4356,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4404,7 +4430,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4616,7 +4643,8 @@ func TestRenderYamlEnvVar(t *testing.T) { return false, nil }, false, - SetFilter(true), + + false, SetFilter(true), ) if tc.expectErr { diff --git a/pkg/app/config.go b/pkg/app/config.go index 03bdc627..b64296cd 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -31,6 +31,7 @@ type ConfigProvider interface { type DepsConfigProvider interface { Args() string SkipRepos() bool + IncludeNeeds() bool IncludeTransitiveNeeds() bool concurrencyConfig @@ -38,6 +39,7 @@ type DepsConfigProvider interface { type ReposConfigProvider interface { Args() string + IncludeNeeds() bool IncludeTransitiveNeeds() bool } @@ -279,6 +281,7 @@ type WriteValuesConfigProvider interface { SkipDeps() bool SkipRefresh() bool SkipCleanup() bool + IncludeNeeds() bool IncludeTransitiveNeeds() bool concurrencyConfig diff --git a/pkg/app/print_env.go b/pkg/app/print_env.go index 17421e30..987be159 100644 --- a/pkg/app/print_env.go +++ b/pkg/app/print_env.go @@ -73,7 +73,7 @@ func (a *App) PrintEnv(c PrintEnvConfigProvider) error { firstDoc = false return false, nil - }, false) + }, false, false) // Close JSON array if c.Output() == "json" { diff --git a/pkg/app/run.go b/pkg/app/run.go index 10bd281d..589bba9b 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -137,7 +137,7 @@ func (r *Run) Deps(c DepsConfigProvider) []error { r.helm.SetExtraArgs(GetArgs(c.Args(), r.state)...) - return r.state.UpdateDeps(r.helm, c.IncludeTransitiveNeeds()) + return r.state.UpdateDeps(r.helm, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) } func (r *Run) Repos(c ReposConfigProvider) error { diff --git a/pkg/app/testdata/testapply_2/include-transitive-needs=true/log b/pkg/app/testdata/testapply_2/include-transitive-needs=true/log index a4405c6a..df204aec 100644 --- a/pkg/app/testdata/testapply_2/include-transitive-needs=true/log +++ b/pkg/app/testdata/testapply_2/include-transitive-needs=true/log @@ -22,22 +22,18 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 19: merged environment: &{default map[] map[] map[]} -3 release(s) matching name=serviceA found in helmfile.yaml.gotmpl +1 release(s) matching name=serviceA found in helmfile.yaml.gotmpl Affected releases are: serviceA (my/chart) UPDATED serviceB (my/chart) UPDATED serviceC (my/chart) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 1 groups of releases in this order: GROUP RELEASES 1 default//serviceA -2 default//serviceB -3 default//serviceC -invoking preapply hooks for releases in group 1/3: default//serviceA -invoking preapply hooks for releases in group 2/3: default//serviceB -invoking preapply hooks for releases in group 3/3: default//serviceC +invoking preapply hooks for releases in group 1/1: default//serviceA processing 3 groups of releases in this order: GROUP RELEASES 1 default//serviceC diff --git a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true/log b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true/log index 62337d68..0b0bad62 100644 --- a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true/log +++ b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true/log @@ -33,28 +33,23 @@ Affected releases are: kubernetes-external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/default/my-release 2 default/default/external-secrets -3 default/kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/default/my-release -invoking preapply hooks for releases in group 2/3: default/default/external-secrets -invoking preapply hooks for releases in group 3/3: default/kube-system/kubernetes-external-secrets -processing 3 groups of releases in this order: +invoking preapply hooks for releases in group 1/2: default/default/my-release +invoking preapply hooks for releases in group 2/2: default/default/external-secrets +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/kubernetes-external-secrets -2 default/default/external-secrets -3 default/default/my-release +1 default/default/external-secrets +2 default/default/my-release -processing releases in group 1/3: default/kube-system/kubernetes-external-secrets -processing releases in group 2/3: default/default/external-secrets -processing releases in group 3/3: default/default/my-release +processing releases in group 1/2: default/default/external-secrets +processing releases in group 2/2: default/default/my-release UPDATED RELEASES: -NAME NAMESPACE CHART VERSION DURATION -kubernetes-external-secrets kube-system incubator/raw 3.1.0 0s -external-secrets default incubator/raw 3.1.0 0s -my-release default incubator/raw 3.1.0 0s +NAME NAMESPACE CHART VERSION DURATION +external-secrets default incubator/raw 3.1.0 0s +my-release default incubator/raw 3.1.0 0s diff --git a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log index 77069b44..6af67687 100644 --- a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log +++ b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log @@ -32,15 +32,13 @@ Affected releases are: external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/default/my-release 2 default/default/external-secrets -3 default/kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/default/my-release -invoking preapply hooks for releases in group 2/3: default/default/external-secrets -invoking preapply hooks for releases in group 3/3: default/kube-system/kubernetes-external-secrets +invoking preapply hooks for releases in group 1/2: default/default/my-release +invoking preapply hooks for releases in group 2/2: default/default/external-secrets processing 2 groups of releases in this order: GROUP RELEASES 1 default/default/external-secrets diff --git a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log index 1a00d70c..f00d2c4f 100644 --- a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log +++ b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log @@ -36,20 +36,20 @@ Affected releases are: kubernetes-external-secrets (incubator/raw) DELETED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/default/my-release 2 default/default/external-secrets -3 default/kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/default/my-release -invoking preapply hooks for releases in group 2/3: default/default/external-secrets -invoking preapply hooks for releases in group 3/3: default/kube-system/kubernetes-external-secrets -processing 1 groups of releases in this order: +invoking preapply hooks for releases in group 1/2: default/default/my-release +invoking preapply hooks for releases in group 2/2: default/default/external-secrets +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/kubernetes-external-secrets +1 default/default/my-release +2 default/default/external-secrets -processing releases in group 1/1: default/kube-system/kubernetes-external-secrets +processing releases in group 1/2: default/default/my-release +processing releases in group 2/2: default/default/external-secrets processing 2 groups of releases in this order: GROUP RELEASES 1 default/default/external-secrets @@ -64,8 +64,3 @@ NAME NAMESPACE CHART VERSION DURATION external-secrets default incubator/raw 3.1.0 0s my-release default incubator/raw 3.1.0 0s - -DELETED RELEASES: -NAME NAMESPACE DURATION -kubernetes-external-secrets kube-system 0s - diff --git a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log index 58c602da..d0d92835 100644 --- a/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log +++ b/pkg/app/testdata/testapply_2/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log @@ -35,15 +35,13 @@ Affected releases are: external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/default/my-release 2 default/default/external-secrets -3 default/kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/default/my-release -invoking preapply hooks for releases in group 2/3: default/default/external-secrets -invoking preapply hooks for releases in group 3/3: default/kube-system/kubernetes-external-secrets +invoking preapply hooks for releases in group 1/2: default/default/my-release +invoking preapply hooks for releases in group 2/2: default/default/external-secrets processing 2 groups of releases in this order: GROUP RELEASES 1 default/default/external-secrets diff --git a/pkg/app/testdata/testapply_2/skip-needs=true_with_no_diff_on_a_release/log b/pkg/app/testdata/testapply_2/skip-needs=true_with_no_diff_on_a_release/log index 17ac4bf2..0fb7610c 100644 --- a/pkg/app/testdata/testapply_2/skip-needs=true_with_no_diff_on_a_release/log +++ b/pkg/app/testdata/testapply_2/skip-needs=true_with_no_diff_on_a_release/log @@ -38,11 +38,13 @@ GROUP RELEASES invoking preapply hooks for releases in group 1/2: default/default/my-release invoking preapply hooks for releases in group 2/2: default/default/external-secrets -processing 1 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES 1 default/default/external-secrets +2 default/default/my-release -processing releases in group 1/1: default/default/external-secrets +processing releases in group 1/2: default/default/external-secrets +processing releases in group 2/2: default/default/my-release UPDATED RELEASES: NAME NAMESPACE CHART VERSION DURATION diff --git a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true/log b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true/log index 3706c59d..dc06cef0 100644 --- a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true/log +++ b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true/log @@ -33,28 +33,23 @@ Affected releases are: kubernetes-external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/my-release 2 default/external-secrets -3 kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/my-release -invoking preapply hooks for releases in group 2/3: default/external-secrets -invoking preapply hooks for releases in group 3/3: kube-system/kubernetes-external-secrets -processing 3 groups of releases in this order: +invoking preapply hooks for releases in group 1/2: default/my-release +invoking preapply hooks for releases in group 2/2: default/external-secrets +processing 2 groups of releases in this order: GROUP RELEASES -1 kube-system/kubernetes-external-secrets -2 default/external-secrets -3 default/my-release +1 default/external-secrets +2 default/my-release -processing releases in group 1/3: kube-system/kubernetes-external-secrets -processing releases in group 2/3: default/external-secrets -processing releases in group 3/3: default/my-release +processing releases in group 1/2: default/external-secrets +processing releases in group 2/2: default/my-release UPDATED RELEASES: -NAME NAMESPACE CHART VERSION DURATION -kubernetes-external-secrets kube-system incubator/raw 3.1.0 0s -external-secrets default incubator/raw 3.1.0 0s -my-release default incubator/raw 3.1.0 0s +NAME NAMESPACE CHART VERSION DURATION +external-secrets default incubator/raw 3.1.0 0s +my-release default incubator/raw 3.1.0 0s diff --git a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log index 7f9b3818..a01ac5c9 100644 --- a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log +++ b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_but_no_diff_on_needed_release/log @@ -32,15 +32,13 @@ Affected releases are: external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/my-release 2 default/external-secrets -3 kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/my-release -invoking preapply hooks for releases in group 2/3: default/external-secrets -invoking preapply hooks for releases in group 3/3: kube-system/kubernetes-external-secrets +invoking preapply hooks for releases in group 1/2: default/my-release +invoking preapply hooks for releases in group 2/2: default/external-secrets processing 2 groups of releases in this order: GROUP RELEASES 1 default/external-secrets diff --git a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log index 0144f44a..6bd8dc32 100644 --- a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log +++ b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_installed_but_disabled_release/log @@ -36,20 +36,20 @@ Affected releases are: kubernetes-external-secrets (incubator/raw) DELETED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/my-release 2 default/external-secrets -3 kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/my-release -invoking preapply hooks for releases in group 2/3: default/external-secrets -invoking preapply hooks for releases in group 3/3: kube-system/kubernetes-external-secrets -processing 1 groups of releases in this order: +invoking preapply hooks for releases in group 1/2: default/my-release +invoking preapply hooks for releases in group 2/2: default/external-secrets +processing 2 groups of releases in this order: GROUP RELEASES -1 kube-system/kubernetes-external-secrets +1 default/my-release +2 default/external-secrets -processing releases in group 1/1: kube-system/kubernetes-external-secrets +processing releases in group 1/2: default/my-release +processing releases in group 2/2: default/external-secrets processing 2 groups of releases in this order: GROUP RELEASES 1 default/external-secrets @@ -64,8 +64,3 @@ NAME NAMESPACE CHART VERSION DURATION external-secrets default incubator/raw 3.1.0 0s my-release default incubator/raw 3.1.0 0s - -DELETED RELEASES: -NAME NAMESPACE DURATION -kubernetes-external-secrets kube-system 0s - diff --git a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log index b67a15d7..a603737b 100644 --- a/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log +++ b/pkg/app/testdata/testapply_3/skip-needs=false_include-needs=true_with_not_installed_and_disabled_release/log @@ -35,15 +35,13 @@ Affected releases are: external-secrets (incubator/raw) UPDATED my-release (incubator/raw) UPDATED -invoking preapply hooks for 3 groups of releases in this order: +invoking preapply hooks for 2 groups of releases in this order: GROUP RELEASES 1 default/my-release 2 default/external-secrets -3 kube-system/kubernetes-external-secrets -invoking preapply hooks for releases in group 1/3: default/my-release -invoking preapply hooks for releases in group 2/3: default/external-secrets -invoking preapply hooks for releases in group 3/3: kube-system/kubernetes-external-secrets +invoking preapply hooks for releases in group 1/2: default/my-release +invoking preapply hooks for releases in group 2/2: default/external-secrets processing 2 groups of releases in this order: GROUP RELEASES 1 default/external-secrets diff --git a/pkg/app/testdata/testapply_3/skip-needs=true_with_no_diff_on_a_release/log b/pkg/app/testdata/testapply_3/skip-needs=true_with_no_diff_on_a_release/log index 47aff850..48848dc8 100644 --- a/pkg/app/testdata/testapply_3/skip-needs=true_with_no_diff_on_a_release/log +++ b/pkg/app/testdata/testapply_3/skip-needs=true_with_no_diff_on_a_release/log @@ -38,11 +38,13 @@ GROUP RELEASES invoking preapply hooks for releases in group 1/2: default/my-release invoking preapply hooks for releases in group 2/2: default/external-secrets -processing 1 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES 1 default/external-secrets +2 default/my-release -processing releases in group 1/1: default/external-secrets +processing releases in group 1/2: default/external-secrets +processing releases in group 2/2: default/my-release UPDATED RELEASES: NAME NAMESPACE CHART VERSION DURATION diff --git a/pkg/config/deps.go b/pkg/config/deps.go index f4dd4608..fd1e3e91 100644 --- a/pkg/config/deps.go +++ b/pkg/config/deps.go @@ -32,6 +32,11 @@ func (d *DepsImpl) SkipRepos() bool { return d.DepsOptions.SkipRepos } +// IncludeNeeds returns the includeNeeds +func (d *DepsImpl) IncludeNeeds() bool { + return false +} + // IncludeTransitiveNeeds returns the includeTransitiveNeeds func (d *DepsImpl) IncludeTransitiveNeeds() bool { return false diff --git a/pkg/config/repos.go b/pkg/config/repos.go index 090b8cdf..3d9fe1bd 100644 --- a/pkg/config/repos.go +++ b/pkg/config/repos.go @@ -22,6 +22,11 @@ func NewReposImpl(g *GlobalImpl, b *ReposOptions) *ReposImpl { } } +// IncludeNeeds returns the include needs +func (r *ReposImpl) IncludeNeeds() bool { + return false +} + // IncludeTransitiveNeeds returns the include transitive needs func (r *ReposImpl) IncludeTransitiveNeeds() bool { return false diff --git a/pkg/config/write-values.go b/pkg/config/write-values.go index 5e5c2ffe..8da5aa2f 100644 --- a/pkg/config/write-values.go +++ b/pkg/config/write-values.go @@ -51,6 +51,11 @@ func (c *WriteValuesImpl) SkipCleanup() bool { return false } +// IncludeNeeds returns the include needs +func (c *WriteValuesImpl) IncludeNeeds() bool { + return false +} + // IncludeTransitiveNeeds returns the include transitive needs func (c *WriteValuesImpl) IncludeTransitiveNeeds() bool { return false @@ -60,3 +65,13 @@ func (c *WriteValuesImpl) IncludeTransitiveNeeds() bool { func (c *WriteValuesImpl) OutputFileTemplate() string { return c.WriteValuesOptions.OutputFileTemplate } + +// SkipDeps returns the skip deps +func (c *WriteValuesImpl) SkipDeps() bool { + return false +} + +// SkipRefresh returns the skip refresh +func (c *WriteValuesImpl) SkipRefresh() bool { + return false +} diff --git a/pkg/state/selector_include_needs_test.go b/pkg/state/selector_include_needs_test.go new file mode 100644 index 00000000..917cc8c5 --- /dev/null +++ b/pkg/state/selector_include_needs_test.go @@ -0,0 +1,346 @@ +package state + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +// TestIncludeNeedsVsIncludeTransitiveNeeds demonstrates the difference between +// --include-needs and --include-transitive-needs flags. +// +// Behavior Summary: +// 1. --include-needs: Includes only DIRECT dependencies (immediate needs) of selected releases +// 2. --include-transitive-needs: Includes ALL dependencies including transitive ones (needs of needs) +// +// Example dependency graph: +// +// appA -> appB -> appC +// appA -> appD +// +// When selecting appA with: +// - No flags: Only appA (fails if needs are not satisfied) +// - --include-needs: appA, appB, appD (only direct needs) +// - --include-transitive-needs: appA, appB, appC, appD (all needs including transitive) +func TestIncludeNeedsVsIncludeTransitiveNeeds(t *testing.T) { + type testcase struct { + name string + selector []string + includeNeeds bool + includeTransitiveNeeds bool + want []string + } + + // Dependency graph: + // appA needs [appB, appD] + // appB needs [appC] + // appC has no needs + // appD has no needs + // appE is independent (not in dependency chain) + testcases := []testcase{ + { + name: "no include flags - only selected release", + selector: []string{"name=appA"}, + includeNeeds: false, + includeTransitiveNeeds: false, + want: []string{"appA"}, + }, + { + name: "include-needs only - direct dependencies (appB, appD)", + selector: []string{"name=appA"}, + includeNeeds: true, + includeTransitiveNeeds: false, + want: []string{"appA", "appB", "appD"}, + }, + { + name: "include-transitive-needs - all dependencies including transitive (appB, appC, appD)", + selector: []string{"name=appA"}, + includeNeeds: false, // Note: includeTransitiveNeeds implies includeNeeds + includeTransitiveNeeds: true, + want: []string{"appA", "appB", "appC", "appD"}, + }, + { + name: "include-needs AND include-transitive-needs - same as include-transitive-needs alone", + selector: []string{"name=appA"}, + includeNeeds: true, + includeTransitiveNeeds: true, + want: []string{"appA", "appB", "appC", "appD"}, + }, + { + name: "include-needs on leaf release (appC) - no dependencies to include", + selector: []string{"name=appC"}, + includeNeeds: true, + includeTransitiveNeeds: false, + want: []string{"appC"}, + }, + { + name: "include-transitive-needs on middle release (appB) - includes appC", + selector: []string{"name=appB"}, + includeNeeds: false, + includeTransitiveNeeds: true, + want: []string{"appB", "appC"}, + }, + { + name: "include-needs on middle release (appB) - includes only appC (direct need)", + selector: []string{"name=appB"}, + includeNeeds: true, + includeTransitiveNeeds: false, + want: []string{"appB", "appC"}, + }, + } + + example := []byte(`releases: +- name: appA + namespace: default + chart: stable/testchart + needs: + - appB + - appD +- name: appB + namespace: default + chart: stable/testchart + needs: + - appC +- name: appC + namespace: default + chart: stable/testchart +- name: appD + namespace: default + chart: stable/testchart +- name: appE + namespace: default + chart: stable/testchart +`) + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + state := stateTestEnv{ + Files: map[string]string{ + "/helmfile.yaml": string(example), + }, + WorkDir: "/", + }.MustLoadState(t, "/helmfile.yaml", "default") + + var err error + state.Selectors = tc.selector + state.Releases, err = state.GetReleasesWithOverrides() + if err != nil { + t.Fatalf("GetReleasesWithOverrides failed: %v", err) + } + state.Releases = state.GetReleasesWithLabels() + + // GetSelectedReleases(includeNeeds, includeTransitiveNeeds) + rs, err := state.GetSelectedReleases(tc.includeNeeds, tc.includeTransitiveNeeds) + if err != nil { + t.Fatalf("GetSelectedReleases failed: %v", err) + } + + var got []string + for _, r := range rs { + got = append(got, r.Name) + } + + if d := cmp.Diff(tc.want, got); d != "" { + t.Errorf("unexpected releases: want (-), got (+):\n%s", d) + } + }) + } +} + +// TestIncludeNeedsWithDeepTransitiveChain tests a deeper transitive dependency chain +// to ensure --include-needs only includes direct dependencies. +// +// Dependency graph: app1 -> app2 -> app3 -> app4 +// +// With --include-needs on app1: should include app1, app2 (direct only) +// With --include-transitive-needs on app1: should include app1, app2, app3, app4 +func TestIncludeNeedsWithDeepTransitiveChain(t *testing.T) { + type testcase struct { + name string + selector []string + includeNeeds bool + includeTransitiveNeeds bool + want []string + } + + testcases := []testcase{ + { + name: "include-needs on deep chain - direct dependency only", + selector: []string{"name=app1"}, + includeNeeds: true, + includeTransitiveNeeds: false, + want: []string{"app1", "app2"}, + }, + { + name: "include-transitive-needs on deep chain - all dependencies", + selector: []string{"name=app1"}, + includeNeeds: false, + includeTransitiveNeeds: true, + want: []string{"app1", "app2", "app3", "app4"}, + }, + { + name: "include-needs from middle of chain", + selector: []string{"name=app2"}, + includeNeeds: true, + includeTransitiveNeeds: false, + want: []string{"app2", "app3"}, + }, + { + name: "include-transitive-needs from middle of chain", + selector: []string{"name=app2"}, + includeNeeds: false, + includeTransitiveNeeds: true, + want: []string{"app2", "app3", "app4"}, + }, + } + + example := []byte(`releases: +- name: app1 + namespace: default + chart: stable/testchart + needs: + - app2 +- name: app2 + namespace: default + chart: stable/testchart + needs: + - app3 +- name: app3 + namespace: default + chart: stable/testchart + needs: + - app4 +- name: app4 + namespace: default + chart: stable/testchart +`) + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + state := stateTestEnv{ + Files: map[string]string{ + "/helmfile.yaml": string(example), + }, + WorkDir: "/", + }.MustLoadState(t, "/helmfile.yaml", "default") + + var err error + state.Selectors = tc.selector + state.Releases, err = state.GetReleasesWithOverrides() + if err != nil { + t.Fatalf("GetReleasesWithOverrides failed: %v", err) + } + state.Releases = state.GetReleasesWithLabels() + + rs, err := state.GetSelectedReleases(tc.includeNeeds, tc.includeTransitiveNeeds) + if err != nil { + t.Fatalf("GetSelectedReleases failed: %v", err) + } + + var got []string + for _, r := range rs { + got = append(got, r.Name) + } + + if d := cmp.Diff(tc.want, got); d != "" { + t.Errorf("unexpected releases: want (-), got (+):\n%s", d) + } + }) + } +} + +// TestIncludeNeedsWithMultipleDirectNeeds tests that --include-needs includes +// all direct needs but not transitive needs of those direct needs. +// +// Dependency graph: +// +// frontend -> [backend-api, backend-worker] +// backend-api -> database +// backend-worker -> database +// database -> cache +func TestIncludeNeedsWithMultipleDirectNeeds(t *testing.T) { + type testcase struct { + name string + selector []string + includeNeeds bool + includeTransitiveNeeds bool + want []string + } + + testcases := []testcase{ + { + name: "include-needs - direct needs only (backend-api, backend-worker)", + selector: []string{"name=frontend"}, + includeNeeds: true, + includeTransitiveNeeds: false, + want: []string{"frontend", "backend-api", "backend-worker"}, + }, + { + name: "include-transitive-needs - all needs (backend-api, backend-worker, database, cache)", + selector: []string{"name=frontend"}, + includeNeeds: false, + includeTransitiveNeeds: true, + want: []string{"frontend", "backend-api", "backend-worker", "database", "cache"}, + }, + } + + example := []byte(`releases: +- name: frontend + namespace: default + chart: stable/testchart + needs: + - backend-api + - backend-worker +- name: backend-api + namespace: default + chart: stable/testchart + needs: + - database +- name: backend-worker + namespace: default + chart: stable/testchart + needs: + - database +- name: database + namespace: default + chart: stable/testchart + needs: + - cache +- name: cache + namespace: default + chart: stable/testchart +`) + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + state := stateTestEnv{ + Files: map[string]string{ + "/helmfile.yaml": string(example), + }, + WorkDir: "/", + }.MustLoadState(t, "/helmfile.yaml", "default") + + var err error + state.Selectors = tc.selector + state.Releases, err = state.GetReleasesWithOverrides() + if err != nil { + t.Fatalf("GetReleasesWithOverrides failed: %v", err) + } + state.Releases = state.GetReleasesWithLabels() + + rs, err := state.GetSelectedReleases(tc.includeNeeds, tc.includeTransitiveNeeds) + if err != nil { + t.Fatalf("GetSelectedReleases failed: %v", err) + } + + var got []string + for _, r := range rs { + got = append(got, r.Name) + } + + if d := cmp.Diff(tc.want, got); d != "" { + t.Errorf("unexpected releases: want (-), got (+):\n%s", d) + } + }) + } +} diff --git a/pkg/state/selector_test.go b/pkg/state/selector_test.go index e955aed4..35ea4ab0 100644 --- a/pkg/state/selector_test.go +++ b/pkg/state/selector_test.go @@ -117,7 +117,7 @@ func TestSelectReleasesWithOverrides(t *testing.T) { } state.Releases = state.GetReleasesWithLabels() - rs, err := state.GetSelectedReleases(false) + rs, err := state.GetSelectedReleases(false, false) if err != nil { t.Fatalf("%s %s: %v", tc.selector, tc.subject, err) } @@ -192,7 +192,7 @@ func TestSelectReleasesWithOverridesWithIncludedTransitives(t *testing.T) { } state.Releases = state.GetReleasesWithLabels() - rs, err := state.GetSelectedReleases(tc.includeTransitiveNeeds) + rs, err := state.GetSelectedReleases(tc.includeTransitiveNeeds, tc.includeTransitiveNeeds) if err != nil { t.Fatalf("%s %s: %v", tc.selector, tc.subject, err) } diff --git a/pkg/state/state.go b/pkg/state/state.go index 5f929fd5..ba227ae8 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1346,6 +1346,7 @@ type ChartPrepareOptions struct { WaitForJobs bool OutputDir string OutputDirTemplate string + IncludeNeeds bool IncludeTransitiveNeeds bool Concurrency int KubeVersion string @@ -1822,7 +1823,7 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre } *st = *updated } - selected, err := st.GetSelectedReleases(opts.IncludeTransitiveNeeds) + selected, err := st.GetSelectedReleases(opts.IncludeNeeds, opts.IncludeTransitiveNeeds) if err != nil { return nil, []error{err} } @@ -2945,16 +2946,16 @@ func (st *HelmState) GetReleasesWithLabels() []ReleaseSpec { return rs } -func (st *HelmState) SelectReleases(includeTransitiveNeeds bool) ([]Release, error) { +func (st *HelmState) SelectReleases(includeNeeds bool, includeTransitiveNeeds bool) ([]Release, error) { values := st.Values() - rs, err := markExcludedReleases(st.Releases, st.Selectors, values, includeTransitiveNeeds) + rs, err := markExcludedReleases(st.Releases, st.Selectors, values, includeNeeds, includeTransitiveNeeds) if err != nil { return nil, err } return rs, nil } -func markExcludedReleases(releases []ReleaseSpec, selectors []string, values map[string]any, includeTransitiveNeeds bool) ([]Release, error) { +func markExcludedReleases(releases []ReleaseSpec, selectors []string, values map[string]any, includeNeeds bool, includeTransitiveNeeds bool) ([]Release, error) { var filteredReleases []Release filters := []ReleaseFilter{} for _, label := range selectors { @@ -2986,6 +2987,8 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, values map } if includeTransitiveNeeds { unmarkNeedsAndTransitives(filteredReleases, releases) + } else if includeNeeds { + unmarkNeedsDirectOnly(filteredReleases) } return filteredReleases, nil } @@ -3043,6 +3046,35 @@ func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []Release unmarkReleases(needsWithTranstives, filteredReleases) } +func unmarkNeedsDirectOnly(filteredReleases []Release) { + directNeeds := collectDirectNeedsOnly(filteredReleases) + unmarkReleasesByNeedID(directNeeds, filteredReleases) +} + +func collectDirectNeedsOnly(filteredReleases []Release) map[string]struct{} { + directNeeds := map[string]struct{}{} + for _, r := range filteredReleases { + if !r.Filtered { + for _, id := range r.ReleaseSpec.Needs { + directNeeds[id] = struct{}{} + } + } + } + return directNeeds +} + +func unmarkReleasesByNeedID(toUnmark map[string]struct{}, releases []Release) { + for needID := range toUnmark { + parts := strings.Split(needID, "/") + needName := parts[len(parts)-1] + for i, r := range releases { + if r.Name == needName { + releases[i].Filtered = false + } + } + } +} + func collectAllNeedsWithTransitives(filteredReleases []Release, allReleases []ReleaseSpec) map[string]struct{} { needsWithTranstives := map[string]struct{}{} for _, r := range filteredReleases { @@ -3076,8 +3108,8 @@ func collectNeedsWithTransitives(release ReleaseSpec, allReleases []ReleaseSpec, } } -func (st *HelmState) GetSelectedReleases(includeTransitiveNeeds bool) ([]ReleaseSpec, error) { - filteredReleases, err := st.SelectReleases(includeTransitiveNeeds) +func (st *HelmState) GetSelectedReleases(includeNeeds bool, includeTransitiveNeeds bool) ([]ReleaseSpec, error) { + filteredReleases, err := st.SelectReleases(includeNeeds, includeTransitiveNeeds) if err != nil { return nil, err } @@ -3092,8 +3124,8 @@ func (st *HelmState) GetSelectedReleases(includeTransitiveNeeds bool) ([]Release } // FilterReleases allows for the execution of helm commands against a subset of the releases in the helmfile. -func (st *HelmState) FilterReleases(includeTransitiveNeeds bool) error { - releases, err := st.GetSelectedReleases(includeTransitiveNeeds) +func (st *HelmState) FilterReleases(includeNeeds bool, includeTransitiveNeeds bool) error { + releases, err := st.GetSelectedReleases(includeNeeds, includeTransitiveNeeds) if err != nil { return err } @@ -3173,7 +3205,7 @@ func (st *HelmState) ResolveDeps() (*HelmState, error) { } // UpdateDeps wrapper for updating dependencies on the releases -func (st *HelmState) UpdateDeps(helm helmexec.Interface, includeTransitiveNeeds bool) []error { +func (st *HelmState) UpdateDeps(helm helmexec.Interface, includeNeeds bool, includeTransitiveNeeds bool) []error { var selected []ReleaseSpec if len(st.Selectors) > 0 { @@ -3181,7 +3213,7 @@ func (st *HelmState) UpdateDeps(helm helmexec.Interface, includeTransitiveNeeds // This and releasesNeedCharts ensures that we run operations like helm-dep-build and prepare-hook calls only on // releases that are (1) selected by the selectors and (2) to be installed. - selected, err = st.GetSelectedReleases(includeTransitiveNeeds) + selected, err = st.GetSelectedReleases(includeNeeds, includeTransitiveNeeds) if err != nil { return []error{err} } diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index 53646bc9..0f0befbf 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -99,7 +99,7 @@ type PlanOptions struct { } func (st *HelmState) PlanReleases(opts PlanOptions) ([][]Release, error) { - marked, err := st.SelectReleases(opts.IncludeTransitiveNeeds) + marked, err := st.SelectReleases(opts.IncludeNeeds, opts.IncludeTransitiveNeeds) if err != nil { return nil, err } @@ -132,6 +132,12 @@ func SortedReleaseGroups(releases []Release, opts PlanOptions) ([][]Release, err func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Release, error) { idToReleases := map[string][]Release{} idToIndex := map[string]int{} + nameToID := map[string]string{} + + for _, r := range releases { + id := ReleaseToID(&r.ReleaseSpec) + nameToID[r.Name] = id + } d := dag.New() for i, r := range releases { @@ -143,7 +149,11 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas var needs []string for i := 0; i < len(r.Needs); i++ { n := r.Needs[i] - needs = append(needs, n) + if fullID, ok := nameToID[n]; ok { + needs = append(needs, fullID) + } else { + needs = append(needs, n) + } } d.Add(id, dag.Dependencies(needs)) } @@ -154,17 +164,22 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas } var selectedReleaseIDs []string + for _, r := range releases { + if !r.Filtered { + id := ReleaseToID(&r.ReleaseSpec) + selectedReleaseIDs = append(selectedReleaseIDs, id) + } + } - for _, r := range opts.SelectedReleases { - release := r - id := ReleaseToID(&release) - selectedReleaseIDs = append(selectedReleaseIDs, id) + skipDepValidation := opts.SkipNeeds + if opts.IncludeNeeds && !opts.IncludeTransitiveNeeds { + skipDepValidation = true } plan, err := d.Plan(dag.SortOptions{ Only: selectedReleaseIDs, - WithDependencies: opts.IncludeNeeds, - WithoutDependencies: opts.SkipNeeds, + WithDependencies: false, + WithoutDependencies: skipDepValidation, }) if err != nil { if ude, ok := err.(*dag.UnhandledDependencyError); ok { diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 5de789b9..9251e9e9 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -2589,7 +2589,7 @@ generated: 2019-05-16T15:42:45.50486+09:00 }) fs.Cwd = basePath state = injectFs(state, fs) - errs := state.UpdateDeps(helm, false) + errs := state.UpdateDeps(helm, false, false) want := []string{"/example", "./example", generatedDir} if !reflect.DeepEqual(helm.Charts, want) { @@ -3058,7 +3058,7 @@ func TestHelmState_NoReleaseMatched(t *testing.T) { RenderedValues: map[string]any{}, } state.Selectors = []string{tt.labels} - errs := state.FilterReleases(false) + errs := state.FilterReleases(false, false) if (errs != nil) != tt.wantErr { t.Errorf("ReleaseStatuses() for %s error = %v, wantErr %v", tt.name, errs, tt.wantErr) return diff --git a/test/integration/run.sh b/test/integration/run.sh index 80099707..e88df567 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -137,6 +137,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2424-sequential-values-paths.sh . ${dir}/test-cases/issue-2431.sh . ${dir}/test-cases/kubedog-tracking.sh +. ${dir}/test-cases/include-needs-transitive.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/include-needs-transitive.sh b/test/integration/test-cases/include-needs-transitive.sh new file mode 100644 index 00000000..ee8e10d2 --- /dev/null +++ b/test/integration/test-cases/include-needs-transitive.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash + +# Test case for --include-needs vs --include-transitive-needs +# This test verifies that: +# 1. --include-needs includes only direct dependencies +# 2. --include-transitive-needs includes all transitive dependencies +# 3. Log message shows correct count of releases matching selector (not including needs) + +include_needs_case_input_dir="${cases_dir}/include-needs-transitive/input" +include_needs_tmp=$(mktemp -d) + +test_start "include-needs vs include-transitive-needs" + +# Test 1: --include-needs should include only direct dependencies +info "Testing --include-needs includes only direct dependencies" +${helmfile} -f ${include_needs_case_input_dir}/helmfile.yaml -l name=service-a template --include-needs > ${include_needs_tmp}/include-needs.log 2>&1 || fail "helmfile template --include-needs should not fail" + +# Verify that service-a, service-b are included in the output (service-b is direct need of service-a) +# service-c should NOT be included (it's transitive, not direct) +include_needs_output=$(cat ${include_needs_tmp}/include-needs.log) + +if echo "${include_needs_output}" | grep -q "name: service-a" && \ + echo "${include_needs_output}" | grep -q "name: service-b" && \ + ! echo "${include_needs_output}" | grep -q "name: service-c"; then + info "--include-needs correctly includes only direct dependencies (service-a, service-b)" +else + cat ${include_needs_tmp}/include-needs.log + fail "--include-needs should include only service-a and service-b (direct need), not service-c (transitive)" +fi + +# Verify log shows "1 release(s) matching name=service-a" (not 2 or 3) +if echo "${include_needs_output}" | grep -q "1 release(s) matching name=service-a"; then + info "Log correctly shows 1 release matching selector" +else + cat ${include_needs_tmp}/include-needs.log + fail "Log should show '1 release(s) matching name=service-a', not including needs count" +fi + +# Test 2: --include-transitive-needs should include all transitive dependencies +info "Testing --include-transitive-needs includes all transitive dependencies" +${helmfile} -f ${include_needs_case_input_dir}/helmfile.yaml -l name=service-a template --include-transitive-needs > ${include_needs_tmp}/include-transitive-needs.log 2>&1 || fail "helmfile template --include-transitive-needs should not fail" + +# Verify that service-a, service-b, service-c are all included +transitive_output=$(cat ${include_needs_tmp}/include-transitive-needs.log) + +if echo "${transitive_output}" | grep -q "name: service-a" && \ + echo "${transitive_output}" | grep -q "name: service-b" && \ + echo "${transitive_output}" | grep -q "name: service-c"; then + info "--include-transitive-needs correctly includes all transitive dependencies (service-a, service-b, service-c)" +else + cat ${include_needs_tmp}/include-transitive-needs.log + fail "--include-transitive-needs should include service-a, service-b, and service-c (transitive)" +fi + +# Verify log still shows "1 release(s) matching name=service-a" (selector match, not total) +if echo "${transitive_output}" | grep -q "1 release(s) matching name=service-a"; then + info "Log correctly shows 1 release matching selector (not including transitive needs)" +else + cat ${include_needs_tmp}/include-transitive-needs.log + fail "Log should show '1 release(s) matching name=service-a', not including needs count" +fi + +# Test 3: Verify service-d is never included (not in dependency chain) +if ! echo "${include_needs_output}" | grep -q "name: service-d" && \ + ! echo "${transitive_output}" | grep -q "name: service-d"; then + info "service-d correctly not included (not in dependency chain)" +else + fail "service-d should never be included as it's not in the dependency chain" +fi + +# Cleanup +rm -rf ${include_needs_tmp} + +test_pass "include-needs vs include-transitive-needs" diff --git a/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml b/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml new file mode 100644 index 00000000..0a432fb1 --- /dev/null +++ b/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml @@ -0,0 +1,60 @@ +releases: + - name: service-a + chart: ../../../charts/raw + namespace: helmfile-tests + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }} + needs: + - service-b + + - name: service-b + chart: ../../../charts/raw + namespace: helmfile-tests + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }} + needs: + - service-c + + - name: service-c + chart: ../../../charts/raw + namespace: helmfile-tests + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }} + + - name: service-d + chart: ../../../charts/raw + namespace: helmfile-tests + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }}