diff --git a/pkg/app/app.go b/pkg/app/app.go index 1ee1cb48..5ad9a7aa 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() []error { msg, matched, affected, errs = a.diff(run, c) return errs @@ -201,7 +202,7 @@ func (a *App) Diff(c DiffConfigProvider) error { } return matched, criticalErrs - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -241,7 +242,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(), @@ -256,17 +258,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() []error { ok, errs = a.writeValues(run, c) return errs @@ -277,7 +281,7 @@ func (a *App) WriteValues(c WriteValuesConfigProvider) error { } return - }, c.IncludeTransitiveNeeds(), SetFilter(true)) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), SetFilter(true)) } type MultiError struct { @@ -320,7 +324,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() []error { ok, lintErrs, errs = a.lint(run, c) return append(errs, lintErrs...) @@ -335,7 +340,7 @@ func (a *App) Lint(c LintConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -362,6 +367,7 @@ func (a *App) Unittest(c UnittestConfigProvider) error { SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), + IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() []error { ok, unittestErrs, errs = a.unittest(run, c) @@ -377,7 +383,7 @@ func (a *App) Unittest(c UnittestConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -409,7 +415,7 @@ func (a *App) Fetch(c FetchConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) Sync(c SyncConfigProvider) error { @@ -424,7 +430,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() []error { @@ -437,7 +444,7 @@ func (a *App) Sync(c SyncConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) } func (a *App) Apply(c ApplyConfigProvider) error { @@ -463,7 +470,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() []error { matched, updated, es := a.apply(run, c) @@ -480,7 +488,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { } return - }, c.IncludeTransitiveNeeds(), opts...) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), opts...) if err != nil { return err @@ -511,7 +519,7 @@ func (a *App) Status(c StatusesConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) Destroy(c DestroyConfigProvider) error { @@ -535,7 +543,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 { @@ -561,7 +569,7 @@ func (a *App) Test(c TestConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) PrintDAGState(c DAGConfigProvider) error { @@ -579,7 +587,7 @@ func (a *App) PrintDAGState(c DAGConfigProvider) error { return errs }) return ok, errs - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) PrintState(c StateConfigProvider) error { @@ -633,7 +641,7 @@ func (a *App) PrintState(c StateConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) } func (a *App) dag(r *Run) error { @@ -685,7 +693,7 @@ func (a *App) ListReleases(c ListConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) close(releasesChan) @@ -1254,7 +1262,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) @@ -1267,7 +1275,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 } @@ -1371,7 +1379,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, } @@ -1402,7 +1410,7 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir stri _, err := converge(st) return err }, - includeTransitiveNeeds) + includeNeeds, includeTransitiveNeeds) } } @@ -1423,9 +1431,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} } @@ -1471,11 +1479,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) } } @@ -1564,8 +1572,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 } @@ -1630,7 +1638,17 @@ 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) + // When needs are included, we need a separate count of just the selector-matched releases + // for the debug log, so we don't count the included needs in the "found" message. + matchCount := len(selected) + if includeNeeds || includeTransitiveNeeds { + matchedOnly, err := r.state.GetSelectedReleases(false, false) + if err != nil { + return nil, nil, err + } + matchCount = len(matchedOnly) + } + a.Logger.Debugf("%d release(s)%s found in %s\n", matchCount, extra, r.state.FilePath) return selected, deduplicated, nil } @@ -1641,7 +1659,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} } @@ -1850,7 +1868,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} } @@ -2070,7 +2088,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} } @@ -2119,7 +2137,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} } @@ -2337,7 +2355,12 @@ 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) + includeNeeds := c.IncludeNeeds() + if c.IncludeTransitiveNeeds() { + includeNeeds = true + } + + selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, includeNeeds, c.IncludeTransitiveNeeds()) if err != nil { return false, []error{err} } @@ -2349,19 +2372,14 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state // Without this, `PlanReleases` conflates duplicates and return both in `batches`, // even if we provided `SelectedReleases: selectedReleases`. // See https://github.com/roboll/helmfile/issues/1818 for more context. - st.Releases = deduplicated - - includeNeeds := c.IncludeNeeds() - if c.IncludeTransitiveNeeds() { - includeNeeds = true - } + st.Releases = selectedAndNeededReleases batches, err := st.PlanReleases(state.PlanOptions{ Reverse: false, SelectedReleases: selectedReleases, - IncludeNeeds: includeNeeds, - IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), - SkipNeeds: c.SkipNeeds(), + IncludeNeeds: false, + IncludeTransitiveNeeds: false, + SkipNeeds: c.SkipNeeds() || includeNeeds, }) if err != nil { @@ -2392,10 +2410,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 set SkipNeeds when needs are included because toRender already has the correct set of releases. + if _, errs := withDAG(st, r.helm, a.Logger, state.PlanOptions{SelectedReleases: toRender, Reverse: false, SkipNeeds: c.SkipNeeds() || includeNeeds}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { rels = append(rels, subst.Releases...) return nil })); len(errs) > 0 { @@ -2437,7 +2456,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} } @@ -2459,7 +2478,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_diff_test.go b/pkg/app/app_diff_test.go index 5b9e8d0b..e888ee19 100644 --- a/pkg/app/app_diff_test.go +++ b/pkg/app/app_diff_test.go @@ -175,8 +175,6 @@ releases: error: ``, selectors: []string{"app=test"}, diffed: []exectest.Release{ - // TODO: Turned out we can't differentiate needs vs transitive needs in this case :thinking: - {Name: "logging", Flags: []string{"--kube-context", "default", "--namespace", "kube-system", "--reset-values"}}, {Name: "kubernetes-external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "kube-system", "--reset-values"}}, {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default", "--reset-values"}}, {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default", "--reset-values"}}, diff --git a/pkg/app/app_lint_test.go b/pkg/app/app_lint_test.go index f9b10836..6aa3f36b 100644 --- a/pkg/app/app_lint_test.go +++ b/pkg/app/app_lint_test.go @@ -1,21 +1,18 @@ package app import ( - "os" - "path/filepath" - "strings" "sync" "testing" "github.com/google/go-cmp/cmp" "github.com/helmfile/vals" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" "github.com/helmfile/helmfile/pkg/exectest" ffs "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" + "github.com/helmfile/helmfile/pkg/testhelper" ) func TestLint(t *testing.T) { @@ -144,30 +141,7 @@ releases: require.Equal(t, wantLints, helm.Linted) }) - testNameComponents := strings.Split(t.Name(), "/") - testBaseName := strings.ToLower( - strings.ReplaceAll( - testNameComponents[len(testNameComponents)-1], - " ", - "_", - ), - ) - wantLogFileDir := filepath.Join("testdata", "app_lint_test") - wantLogFile := filepath.Join(wantLogFileDir, testBaseName) - wantLogData, err := os.ReadFile(wantLogFile) - updateLogFile := err != nil - wantLog := string(wantLogData) - gotLog := bs.String() - if updateLogFile { - if err := os.MkdirAll(wantLogFileDir, 0755); err != nil { - t.Fatalf("unable to create directory %q: %v", wantLogFileDir, err) - } - if err := os.WriteFile(wantLogFile, bs.Bytes(), 0644); err != nil { - t.Fatalf("unable to update lint log snapshot: %v", err) - } - } - - assert.Equal(t, wantLog, gotLog) + testhelper.RequireLog(t, "app_lint_test", bs) } t.Run("fail on unselected need by default", func(t *testing.T) { @@ -199,8 +173,6 @@ releases: error: ``, selectors: []string{"app=test"}, linted: []exectest.Release{ - // TODO: Turned out we can't differentiate needs vs transitive needs in this case :thinking: - {Name: "logging", Flags: []string{"--namespace", "kube-system"}}, {Name: "kubernetes-external-secrets", Flags: []string{"--namespace", "kube-system"}}, {Name: "external-secrets", Flags: []string{"--namespace", "default"}}, {Name: "my-release", Flags: []string{"--namespace", "default"}}, 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_template_test.go b/pkg/app/app_template_test.go index f2e5f50c..eabed44c 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -206,8 +206,6 @@ releases: selectors: []string{"app=test"}, // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - // TODO: Turned out we can't differentiate needs vs transitive needs in this case :thinking: - {Name: "logging", Flags: []string{"--kube-context", "default", "--namespace", "kube-system"}}, {Name: "kubernetes-external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "kube-system"}}, {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index aa792b33..f1bd20f7 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 { @@ -968,7 +977,8 @@ func runFilterSubHelmFilesTests(testcases []struct { err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if testcase.expectErr { if err == nil { @@ -1058,7 +1068,8 @@ ns: INLINE_NS err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { @@ -1156,6 +1167,7 @@ releases: err := app.ForEachState( collectReleases, false, + false, SetReverse(testcase.reverse), SetFilter(true), ) @@ -1223,7 +1235,8 @@ bar: "bar1" err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1345,7 +1358,8 @@ x: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1458,7 +1472,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1515,7 +1530,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -1565,7 +1581,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { @@ -1608,7 +1625,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" @@ -1655,7 +1673,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" @@ -2762,22 +2781,27 @@ func (a applyConfig) Description() string { 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 } @@ -4464,6 +4488,7 @@ releases: depsErr := app.Deps(depsConfig{ skipRepos: false, + includeNeeds: false, includeTransitiveNeeds: false, }) switch { @@ -4698,7 +4723,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4771,7 +4797,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4983,7 +5010,8 @@ func TestRenderYamlEnvVar(t *testing.T) { return false, nil }, false, - SetFilter(true), + + false, SetFilter(true), ) if tc.expectErr { diff --git a/pkg/app/app_unittest_test.go b/pkg/app/app_unittest_test.go index f5977048..cf55d57d 100644 --- a/pkg/app/app_unittest_test.go +++ b/pkg/app/app_unittest_test.go @@ -190,12 +190,10 @@ releases: }) t.Run("with dedicated flags", func(t *testing.T) { - // --color is skipped on Helm 4 due to flag parsing issues - expectedFlags := []string{"--namespace", "kube-system", "--failfast"} + expectedFlags := []string{"--namespace", "kube-system", "--failfast", "--debugPlugin", "--file", "tests/logging/*_test.yaml"} if !exectest.IsHelm4Enabled() { - expectedFlags = append(expectedFlags, "--color") + expectedFlags = []string{"--namespace", "kube-system", "--failfast", "--color", "--debugPlugin", "--file", "tests/logging/*_test.yaml"} } - expectedFlags = append(expectedFlags, "--debugPlugin", "--file", "tests/logging/*_test.yaml") check(t, testcase{ fields: fields{ @@ -231,7 +229,6 @@ releases: }, selectors: []string{"app=test"}, unittested: []exectest.Release{ - {Name: "logging", Flags: []string{"--namespace", "kube-system", "--file", "tests/logging/*_test.yaml"}}, {Name: "kubernetes-external-secrets", Flags: []string{"--namespace", "kube-system", "--file", "tests/secrets/*_test.yaml"}}, {Name: "external-secrets", Flags: []string{"--namespace", "default", "--file", "tests/external/*_test.yaml"}}, {Name: "my-release", Flags: []string{"--namespace", "default", "--file", "tests/myrelease/*_test.yaml"}}, diff --git a/pkg/app/config.go b/pkg/app/config.go index 9b92e498..4c263d6d 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 } @@ -285,6 +287,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 91e25bb5..27ea2ae1 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -144,7 +144,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/app_diff_test/include-needs b/pkg/app/testdata/app_diff_test/include-needs index 2e043346..ed985d09 100644 --- a/pkg/app/testdata/app_diff_test/include-needs +++ b/pkg/app/testdata/app_diff_test/include-needs @@ -2,14 +2,12 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 2 release(s) matching app=test found in helmfile.yaml -processing 4 groups of releases in this order: +processing 3 groups of releases in this order: GROUP RELEASES -1 default/kube-system/logging -2 default/kube-system/kubernetes-external-secrets -3 default/default/external-secrets -4 default/default/my-release +1 default/kube-system/kubernetes-external-secrets +2 default/default/external-secrets +3 default/default/my-release -processing releases in group 1/4: default/kube-system/logging -processing releases in group 2/4: default/kube-system/kubernetes-external-secrets -processing releases in group 3/4: default/default/external-secrets -processing releases in group 4/4: 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 diff --git a/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_direct_need b/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_direct_need index 538e191f..11b5a896 100644 --- a/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_direct_need +++ b/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_direct_need @@ -2,14 +2,9 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test2 found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 +1 default//test2 -processing releases in group 1/2: default/kube-system/disabled -processing releases in group 2/2: default//test2 +processing releases in group 1/1: default//test2 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs -Affected releases are: - disabled (incubator/raw) DELETED - diff --git a/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_transitive_need index 374768b4..b8b28277 100644 --- a/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_diff_test/include-needs_should_not_fail_on_disabled_transitive_need @@ -2,16 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs -Affected releases are: - disabled (incubator/raw) DELETED - diff --git a/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need b/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need index 538e191f..b6c63520 100644 --- a/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need +++ b/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need @@ -2,13 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test2 found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 +1 default//test2 -processing releases in group 1/2: default/kube-system/disabled -processing releases in group 2/2: default//test2 +processing releases in group 1/1: default//test2 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs Affected releases are: disabled (incubator/raw) DELETED diff --git a/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need index 374768b4..94cc476c 100644 --- a/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_diff_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need @@ -2,15 +2,13 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs Affected releases are: disabled (incubator/raw) DELETED diff --git a/pkg/app/testdata/app_diff_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_diff_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need index 374768b4..94cc476c 100644 --- a/pkg/app/testdata/app_diff_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_diff_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need @@ -2,15 +2,13 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs Affected releases are: disabled (incubator/raw) DELETED diff --git a/pkg/app/testdata/app_diff_test_1/delete_bar_when_foo_needs_bar_with_include-needs b/pkg/app/testdata/app_diff_test_1/delete_bar_when_foo_needs_bar_with_include-needs index d0e53b18..423dcd31 100644 --- a/pkg/app/testdata/app_diff_test_1/delete_bar_when_foo_needs_bar_with_include-needs +++ b/pkg/app/testdata/app_diff_test_1/delete_bar_when_foo_needs_bar_with_include-needs @@ -2,13 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release foo needs bar, but bar is not installed due to installed: false. Either mark bar as installed or remove bar from foo's needs 2 release(s) found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default//bar -2 default//foo +1 default//foo -processing releases in group 1/2: default//bar -processing releases in group 2/2: default//foo +processing releases in group 1/1: default//foo WARNING: release foo needs bar, but bar is not installed due to installed: false. Either mark bar as installed or remove bar from foo's needs Affected releases are: bar (mychart2) DELETED diff --git a/pkg/app/testdata/app_diff_test_1/delete_foo_when_bar_needs_foo_with_include-needs b/pkg/app/testdata/app_diff_test_1/delete_foo_when_bar_needs_foo_with_include-needs index 3ba64bfa..66b8126d 100644 --- a/pkg/app/testdata/app_diff_test_1/delete_foo_when_bar_needs_foo_with_include-needs +++ b/pkg/app/testdata/app_diff_test_1/delete_foo_when_bar_needs_foo_with_include-needs @@ -2,13 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release bar needs foo, but foo is not installed due to installed: false. Either mark foo as installed or remove foo from bar's needs 2 release(s) found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default//foo -2 default//bar +1 default//bar -processing releases in group 1/2: default//foo -processing releases in group 2/2: default//bar +processing releases in group 1/1: default//bar WARNING: release bar needs foo, but foo is not installed due to installed: false. Either mark foo as installed or remove foo from bar's needs Affected releases are: bar (mychart2) UPDATED diff --git a/pkg/app/testdata/app_diff_test_2/delete_bar_when_foo_needs_bar_with_include-needs b/pkg/app/testdata/app_diff_test_2/delete_bar_when_foo_needs_bar_with_include-needs index 0df4dc20..a6718797 100644 --- a/pkg/app/testdata/app_diff_test_2/delete_bar_when_foo_needs_bar_with_include-needs +++ b/pkg/app/testdata/app_diff_test_2/delete_bar_when_foo_needs_bar_with_include-needs @@ -2,13 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release foo needs bar, but bar is not installed due to installed: false. Either mark bar as installed or remove bar from foo's needs 2 release(s) found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 bar -2 foo +1 foo -processing releases in group 1/2: bar -processing releases in group 2/2: foo +processing releases in group 1/1: foo WARNING: release foo needs bar, but bar is not installed due to installed: false. Either mark bar as installed or remove bar from foo's needs Affected releases are: bar (mychart2) DELETED diff --git a/pkg/app/testdata/app_lint_test/include-needs b/pkg/app/testdata/app_lint_test/include-needs index 2e043346..ed985d09 100644 --- a/pkg/app/testdata/app_lint_test/include-needs +++ b/pkg/app/testdata/app_lint_test/include-needs @@ -2,14 +2,12 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 2 release(s) matching app=test found in helmfile.yaml -processing 4 groups of releases in this order: +processing 3 groups of releases in this order: GROUP RELEASES -1 default/kube-system/logging -2 default/kube-system/kubernetes-external-secrets -3 default/default/external-secrets -4 default/default/my-release +1 default/kube-system/kubernetes-external-secrets +2 default/default/external-secrets +3 default/default/my-release -processing releases in group 1/4: default/kube-system/logging -processing releases in group 2/4: default/kube-system/kubernetes-external-secrets -processing releases in group 3/4: default/default/external-secrets -processing releases in group 4/4: 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 diff --git a/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_direct_need b/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_direct_need index 3c08554b..3c22226f 100644 --- a/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_direct_need +++ b/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_direct_need @@ -2,10 +2,8 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test2 found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 +1 default//test2 -processing releases in group 1/2: default/kube-system/disabled -processing releases in group 2/2: default//test2 +processing releases in group 1/1: default//test2 diff --git a/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_transitive_need index 8f55648a..3a218076 100644 --- a/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_lint_test/include-needs_should_not_fail_on_disabled_transitive_need @@ -2,12 +2,10 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 diff --git a/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need b/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need index 3c08554b..3c22226f 100644 --- a/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need +++ b/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need @@ -2,10 +2,8 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test2 found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 +1 default//test2 -processing releases in group 1/2: default/kube-system/disabled -processing releases in group 2/2: default//test2 +processing releases in group 1/1: default//test2 diff --git a/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need index 8f55648a..3a218076 100644 --- a/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_lint_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need @@ -2,12 +2,10 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 diff --git a/pkg/app/testdata/app_lint_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_lint_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need index 8f55648a..3a218076 100644 --- a/pkg/app/testdata/app_lint_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_lint_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need @@ -2,12 +2,10 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 diff --git a/pkg/app/testdata/app_template_test/include-needs b/pkg/app/testdata/app_template_test/include-needs index 2e043346..ed985d09 100644 --- a/pkg/app/testdata/app_template_test/include-needs +++ b/pkg/app/testdata/app_template_test/include-needs @@ -2,14 +2,12 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 2 release(s) matching app=test found in helmfile.yaml -processing 4 groups of releases in this order: +processing 3 groups of releases in this order: GROUP RELEASES -1 default/kube-system/logging -2 default/kube-system/kubernetes-external-secrets -3 default/default/external-secrets -4 default/default/my-release +1 default/kube-system/kubernetes-external-secrets +2 default/default/external-secrets +3 default/default/my-release -processing releases in group 1/4: default/kube-system/logging -processing releases in group 2/4: default/kube-system/kubernetes-external-secrets -processing releases in group 3/4: default/default/external-secrets -processing releases in group 4/4: 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 diff --git a/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_direct_need b/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_direct_need index 0b468ae0..11b5a896 100644 --- a/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_direct_need +++ b/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_direct_need @@ -2,11 +2,9 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test2 found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 +1 default//test2 -processing releases in group 1/2: default/kube-system/disabled -processing releases in group 2/2: default//test2 +processing releases in group 1/1: default//test2 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs diff --git a/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_transitive_need index 3bb8007e..b8b28277 100644 --- a/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_template_test/include-needs_should_not_fail_on_disabled_transitive_need @@ -2,13 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs diff --git a/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need b/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need index 0b468ae0..11b5a896 100644 --- a/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need +++ b/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_direct_need @@ -2,11 +2,9 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test2 found in helmfile.yaml -processing 2 groups of releases in this order: +processing 1 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 +1 default//test2 -processing releases in group 1/2: default/kube-system/disabled -processing releases in group 2/2: default//test2 +processing releases in group 1/1: default//test2 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs diff --git a/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need index 3bb8007e..b8b28277 100644 --- a/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_template_test/include-needs_with_include-transitive-needs_should_not_fail_on_disabled_transitive_need @@ -2,13 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs diff --git a/pkg/app/testdata/app_template_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need b/pkg/app/testdata/app_template_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need index 3bb8007e..b8b28277 100644 --- a/pkg/app/testdata/app_template_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need +++ b/pkg/app/testdata/app_template_test/include-transitive-needs_should_not_fail_on_disabled_transitive_need @@ -2,13 +2,11 @@ merged environment: &{default map[] map[] map[]} WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs 1 release(s) matching name=test3 found in helmfile.yaml -processing 3 groups of releases in this order: +processing 2 groups of releases in this order: GROUP RELEASES -1 default/kube-system/disabled -2 default//test2 -3 default//test3 +1 default//test2 +2 default//test3 -processing releases in group 1/3: default/kube-system/disabled -processing releases in group 2/3: default//test2 -processing releases in group 3/3: default//test3 +processing releases in group 1/2: default//test2 +processing releases in group 2/2: default//test3 WARNING: release test2 needs disabled, but disabled is not installed due to installed: false. Either mark disabled as installed or remove disabled from test2's needs diff --git a/pkg/app/testdata/app_unittest_test/include-needs b/pkg/app/testdata/app_unittest_test/include-needs index f7f03251..f328fb5b 100644 --- a/pkg/app/testdata/app_unittest_test/include-needs +++ b/pkg/app/testdata/app_unittest_test/include-needs @@ -1,14 +1,12 @@ merged environment: &{default map[] map[] map[]} 2 release(s) matching app=test found in helmfile.yaml -processing 4 groups of releases in this order: +processing 3 groups of releases in this order: GROUP RELEASES -1 default/kube-system/logging -2 default/kube-system/kubernetes-external-secrets -3 default/default/external-secrets -4 default/default/my-release +1 default/kube-system/kubernetes-external-secrets +2 default/default/external-secrets +3 default/default/my-release -processing releases in group 1/4: default/kube-system/logging -processing releases in group 2/4: default/kube-system/kubernetes-external-secrets -processing releases in group 3/4: default/default/external-secrets -processing releases in group 4/4: 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 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..d00bbb0d 100644 --- a/pkg/app/testdata/testapply_2/include-transitive-needs=true/log +++ b/pkg/app/testdata/testapply_2/include-transitive-needs=true/log @@ -22,7 +22,7 @@ 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 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..fcfda083 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 diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index a2153576..dbc218a1 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -526,8 +526,13 @@ func (helm *execer) List(context HelmContext, filter string, flags ...string) (s // // This fixes it by removing the header from the v3 output, so that the output is formatted the same as that of v2. lines := strings.Split(string(out), "\n") - lines = lines[1:] - out = []byte(strings.Join(lines, "\n")) + var filtered []string + for _, line := range lines[1:] { + if strings.TrimSpace(line) != "" { + filtered = append(filtered, line) + } + } + out = []byte(strings.Join(filtered, "\n")) helm.info(out) return string(out), err } @@ -1136,7 +1141,8 @@ func resolveOciChart(ociChart string) (ociChartURL, ociChartTag string) { func (helm *execer) ShowChart(chartPath string) (chart.Metadata, error) { var helmArgs = []string{"show", "chart", chartPath} - out, error := helm.exec(helmArgs, map[string]string{}, nil) + enableLiveOutput := false + out, error := helm.exec(helmArgs, map[string]string{}, &enableLiveOutput) if error != nil { return chart.Metadata{}, error } 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..fe6ff245 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) } @@ -139,20 +139,37 @@ func TestSelectReleasesWithOverridesWithIncludedTransitives(t *testing.T) { subject string selector []string want []string + includeNeeds bool includeTransitiveNeeds bool } testcases := []testcase{ { - subject: "include transitives is false", + subject: "no needs inclusion", selector: []string{"name=serviceA"}, want: []string{"serviceA"}, + includeNeeds: false, includeTransitiveNeeds: false, }, { - subject: "include transitives is true", + subject: "include direct needs only", + selector: []string{"name=serviceA"}, + want: []string{"serviceA", "serviceB"}, + includeNeeds: true, + includeTransitiveNeeds: false, + }, + { + subject: "include transitive needs", selector: []string{"name=serviceA"}, want: []string{"serviceA", "serviceB", "serviceC"}, + includeNeeds: false, + includeTransitiveNeeds: true, + }, + { + subject: "include both direct and transitive needs", + selector: []string{"name=serviceA"}, + want: []string{"serviceA", "serviceB", "serviceC"}, + includeNeeds: true, includeTransitiveNeeds: true, }, } @@ -192,7 +209,7 @@ func TestSelectReleasesWithOverridesWithIncludedTransitives(t *testing.T) { } state.Releases = state.GetReleasesWithLabels() - rs, err := state.GetSelectedReleases(tc.includeTransitiveNeeds) + rs, err := state.GetSelectedReleases(tc.includeNeeds, tc.includeTransitiveNeeds) if err != nil { t.Fatalf("%s %s: %v", tc.selector, tc.subject, err) } @@ -208,3 +225,53 @@ func TestSelectReleasesWithOverridesWithIncludedTransitives(t *testing.T) { } } } + +func TestSelectReleasesWithIncludeNeedsCrossNamespace(t *testing.T) { + // When multiple releases share the same name across different namespaces, + // includeNeeds should resolve the correct dependency using fully-qualified IDs + // rather than name-based lookup (which would be ambiguous). + example := []byte(`releases: +- name: frontend + namespace: team-a + chart: stable/testchart + needs: + - team-a/backend +- name: backend + namespace: team-a + chart: stable/testchart +- name: backend + namespace: team-b + chart: stable/testchart +`) + + state := stateTestEnv{ + Files: map[string]string{ + "/helmfile.yaml": string(example), + }, + WorkDir: "/", + }.MustLoadState(t, "/helmfile.yaml", "default") + + state.Selectors = []string{"name=frontend"} + var err error + state.Releases, err = state.GetReleasesWithOverrides() + if err != nil { + t.Fatal(err) + } + state.Releases = state.GetReleasesWithLabels() + + rs, err := state.GetSelectedReleases(true, false) + if err != nil { + t.Fatal(err) + } + + var got []string + for _, r := range rs { + got = append(got, r.Namespace+"/"+r.Name) + } + + // Should include team-a/backend (direct need) but NOT team-b/backend + want := []string{"team-a/frontend", "team-a/backend"} + if d := cmp.Diff(want, got); d != "" { + t.Errorf("cross-namespace include-needs: %s", d) + } +} diff --git a/pkg/state/state.go b/pkg/state/state.go index f51a0dd1..dbf0ad9f 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -857,10 +857,18 @@ func (st *HelmState) isReleaseInstalled(context helmexec.HelmContext, helm helme func (st *HelmState) DetectReleasesToBeDeletedForSync(helm helmexec.Interface, releases []ReleaseSpec) ([]ReleaseSpec, error) { deleted := []ReleaseSpec{} + checked := make(map[string]bool) + for i := range releases { release := releases[i] if !release.Desired() { + id := ReleaseToID(&release) + if checked[id] { + continue + } + checked[id] = true + installed, err := st.isReleaseInstalled(st.createHelmContext(&release, 0), helm, release) if err != nil { return nil, err @@ -1350,6 +1358,7 @@ type ChartPrepareOptions struct { WaitForJobs bool OutputDir string OutputDirTemplate string + IncludeNeeds bool IncludeTransitiveNeeds bool Concurrency int KubeVersion string @@ -1833,7 +1842,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} } @@ -2956,16 +2965,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 { @@ -2997,6 +3006,8 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, values map } if includeTransitiveNeeds { unmarkNeedsAndTransitives(filteredReleases, releases) + } else if includeNeeds { + unmarkNeedsDirectOnly(filteredReleases) } return filteredReleases, nil } @@ -3054,11 +3065,48 @@ 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 _, need := range r.ReleaseSpec.Needs { + // After ApplyOverrides/reformat(), need IDs are already fully-qualified + // (matching ReleaseToID format), so we collect them as-is. + directNeeds[need] = struct{}{} + } + } + } + return directNeeds +} + +func unmarkReleasesByNeedID(toUnmark map[string]struct{}, releases []Release) { + for i := range releases { + releaseID := ReleaseToID(&releases[i].ReleaseSpec) + if _, ok := toUnmark[releaseID]; ok { + if releases[i].Desired() { + releases[i].Filtered = false + } + } + } +} + func collectAllNeedsWithTransitives(filteredReleases []Release, allReleases []ReleaseSpec) map[string]struct{} { + // Build an ID→ReleaseSpec map for efficient lookup by fully-qualified ID, + // avoiding cross-namespace ambiguity from name-based matching. + idToRelease := map[string]ReleaseSpec{} + for _, r := range allReleases { + idToRelease[ReleaseToID(&r)] = r + } + needsWithTranstives := map[string]struct{}{} for _, r := range filteredReleases { if !r.Filtered { - collectNeedsWithTransitives(r.ReleaseSpec, allReleases, needsWithTranstives) + collectNeedsWithTransitives(r.ReleaseSpec, idToRelease, needsWithTranstives) } } return needsWithTranstives @@ -3072,23 +3120,22 @@ func unmarkReleases(toUnmark map[string]struct{}, releases []Release) { } } -func collectNeedsWithTransitives(release ReleaseSpec, allReleases []ReleaseSpec, needsWithTranstives map[string]struct{}) { +func collectNeedsWithTransitives(release ReleaseSpec, idToRelease map[string]ReleaseSpec, needsWithTranstives map[string]struct{}) { for _, id := range release.Needs { if _, exists := needsWithTranstives[id]; !exists { needsWithTranstives[id] = struct{}{} - releaseParts := strings.Split(id, "/") - releaseName := releaseParts[len(releaseParts)-1] - for _, r := range allReleases { - if r.Name == releaseName { - collectNeedsWithTransitives(r, allReleases, needsWithTranstives) - } + // After ApplyOverrides/reformat(), need IDs are already fully-qualified + // (matching ReleaseToID format), so we look up by full ID to avoid + // cross-namespace ambiguity when multiple releases share the same name. + if r, ok := idToRelease[id]; ok { + collectNeedsWithTransitives(r, idToRelease, needsWithTranstives) } } } } -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 } @@ -3103,8 +3150,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 } @@ -3184,7 +3231,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 { @@ -3192,7 +3239,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..70e0d4a3 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 } @@ -140,10 +140,11 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas idToReleases[id] = append(idToReleases[id], r) idToIndex[id] = i + // After ApplyOverrides/reformat(), need IDs are already fully-qualified + // (matching ReleaseToID format), so we pass them as-is. var needs []string for i := 0; i < len(r.Needs); i++ { - n := r.Needs[i] - needs = append(needs, n) + needs = append(needs, r.Needs[i]) } d.Add(id, dag.Dependencies(needs)) } @@ -154,17 +155,43 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas } var selectedReleaseIDs []string + if len(opts.SelectedReleases) > 0 { + // When SelectedReleases is explicitly provided (e.g., by the delete/destroy path), + // use it directly to determine which releases to plan. + for _, r := range opts.SelectedReleases { + release := r + id := ReleaseToID(&release) + selectedReleaseIDs = append(selectedReleaseIDs, id) + } + } else { + // Otherwise, use the Filtered flag set by SelectReleases/markExcludedReleases + 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) + // When SelectedReleases is explicitly provided, use the DAG's dependency + // inclusion (WithDependencies) since markExcludedReleases doesn't apply. + // When using the Filtered flag path (no SelectedReleases), needs are already + // handled by markExcludedReleases, so WithDependencies stays false. + withDeps := false + skipDepValidation := opts.SkipNeeds + if len(opts.SelectedReleases) > 0 { + withDeps = opts.IncludeNeeds + skipDepValidation = opts.SkipNeeds + } else { + if opts.IncludeNeeds && !opts.IncludeTransitiveNeeds { + skipDepValidation = true + } } plan, err := d.Plan(dag.SortOptions{ Only: selectedReleaseIDs, - WithDependencies: opts.IncludeNeeds, - WithoutDependencies: opts.SkipNeeds, + WithDependencies: withDeps, + 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 0d10624a..97632775 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -3007,7 +3007,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) { @@ -3476,7 +3476,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 14d4b435..5a2f1d99 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -142,6 +142,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2431.sh . ${dir}/test-cases/issue-2544.sh . ${dir}/test-cases/kubedog-tracking.sh +. ${dir}/test-cases/include-needs-transitive.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/diff-args/output/apply-live-stderr b/test/integration/test-cases/diff-args/output/apply-live-stderr index 7f789424..f8ac5385 100644 --- a/test/integration/test-cases/diff-args/output/apply-live-stderr +++ b/test/integration/test-cases/diff-args/output/apply-live-stderr @@ -4,7 +4,6 @@ Listing releases matching ^uninstalled$ Upgrading release=installed, chart=../../../charts/httpbin, namespace=helmfile-tests Listing releases matching ^installed$ - UPDATED RELEASES: NAME NAMESPACE CHART VERSION DURATION diff --git a/test/integration/test-cases/diff-args/output/apply-live-stderr-helm4 b/test/integration/test-cases/diff-args/output/apply-live-stderr-helm4 index 7f789424..f8ac5385 100644 --- a/test/integration/test-cases/diff-args/output/apply-live-stderr-helm4 +++ b/test/integration/test-cases/diff-args/output/apply-live-stderr-helm4 @@ -4,7 +4,6 @@ Listing releases matching ^uninstalled$ Upgrading release=installed, chart=../../../charts/httpbin, namespace=helmfile-tests Listing releases matching ^installed$ - UPDATED RELEASES: NAME NAMESPACE CHART VERSION DURATION diff --git a/test/integration/test-cases/diff-args/output/apply-stderr b/test/integration/test-cases/diff-args/output/apply-stderr index 6a5025e3..86c52fa9 100644 --- a/test/integration/test-cases/diff-args/output/apply-stderr +++ b/test/integration/test-cases/diff-args/output/apply-stderr @@ -10,7 +10,6 @@ TEST SUITE: None Listing releases matching ^installed$ - UPDATED RELEASES: NAME NAMESPACE CHART VERSION DURATION diff --git a/test/integration/test-cases/diff-args/output/apply-stderr-helm4 b/test/integration/test-cases/diff-args/output/apply-stderr-helm4 index 088d699a..c14a22b5 100644 --- a/test/integration/test-cases/diff-args/output/apply-stderr-helm4 +++ b/test/integration/test-cases/diff-args/output/apply-stderr-helm4 @@ -11,7 +11,6 @@ TEST SUITE: None Listing releases matching ^installed$ - UPDATED RELEASES: NAME NAMESPACE CHART VERSION DURATION 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..cf1f7806 --- /dev/null +++ b/test/integration/test-cases/include-needs-transitive.sh @@ -0,0 +1,58 @@ +#!/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 + +# 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 + +# 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 }}