From 4a15d694831e9ad18c41de40c78394a8c8bab2d7 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 09:10:25 +0800 Subject: [PATCH 01/12] fix: --include-needs should only include direct dependencies Fixes #1003 Previously, --include-needs was incorrectly including transitive dependencies (dependencies of dependencies). This fix ensures that: - --include-needs only includes direct needs - --include-transitive-needs includes both direct and transitive needs Changes: - Add separate handling for direct vs transitive needs in state.go - Add IncludeNeeds field to ChartPrepareOptions - Add unmarkNeedsDirectOnly() and collectDirectNeedsOnly() functions - Update ForEachState and related functions to accept both flags - Fix incorrect usage of c.IncludeNeeds() for IncludeTransitiveNeeds - Update tests to verify the correct behavior Signed-off-by: yxxhero --- pkg/app/app.go | 78 +++++++++---------- pkg/app/app_sequential_test.go | 12 ++- pkg/app/app_test.go | 64 ++++++++++----- pkg/app/print_env.go | 2 +- pkg/app/run.go | 2 +- .../skip-needs=false_include-needs=true/log | 2 +- .../log | 2 +- .../log | 2 +- .../log | 2 +- .../skip-needs=false_include-needs=true/log | 2 +- .../log | 2 +- .../log | 2 +- .../log | 2 +- pkg/state/selector_test.go | 4 +- pkg/state/state.go | 40 +++++++--- pkg/state/state_run.go | 2 +- pkg/state/state_test.go | 4 +- 17 files changed, 135 insertions(+), 89 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index cdc31745..332bd7b2 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)) + }, false, 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)) + }, false, c.IncludeTransitiveNeeds(), SetFilter(true)) } func (a *App) Diff(c DiffConfigProvider) error { @@ -169,7 +169,7 @@ func (a *App) Diff(c DiffConfigProvider) error { IncludeCRDs: &includeCRDs, Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { msg, matched, affected, errs = a.diff(run, c) }) @@ -200,7 +200,7 @@ func (a *App) Diff(c DiffConfigProvider) error { } return matched, criticalErrs - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -240,7 +240,7 @@ func (a *App) Template(c TemplateConfigProvider) error { SkipCleanup: c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), Set: c.Set(), Values: c.Values(), KubeVersion: c.KubeVersion(), @@ -254,7 +254,7 @@ func (a *App) Template(c TemplateConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) } func (a *App) WriteValues(c WriteValuesConfigProvider) error { @@ -274,7 +274,7 @@ func (a *App) WriteValues(c WriteValuesConfigProvider) error { } return - }, c.IncludeTransitiveNeeds(), SetFilter(true)) + }, false, c.IncludeTransitiveNeeds(), SetFilter(true)) } type MultiError struct { @@ -317,7 +317,7 @@ func (a *App) Lint(c LintConfigProvider) error { SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { ok, lintErrs, errs = a.lint(run, c) }) @@ -331,7 +331,7 @@ func (a *App) Lint(c LintConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -372,7 +372,7 @@ func (a *App) Unittest(c UnittestConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return err @@ -402,7 +402,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 +417,7 @@ func (a *App) Sync(c SyncConfigProvider) error { WaitRetries: c.WaitRetries(), WaitForJobs: c.WaitForJobs(), IncludeCRDs: &includeCRDs, - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), Validate: c.Validate(), Concurrency: c.Concurrency(), }, func() { @@ -429,7 +429,7 @@ func (a *App) Sync(c SyncConfigProvider) error { } return - }, c.IncludeTransitiveNeeds()) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) } func (a *App) Apply(c ApplyConfigProvider) error { @@ -455,7 +455,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { SkipCleanup: c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { matched, updated, es := a.apply(run, c) @@ -471,7 +471,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { } return - }, c.IncludeTransitiveNeeds(), opts...) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), opts...) if err != nil { return err @@ -501,7 +501,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 +524,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 +549,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 +566,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 +619,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 +667,7 @@ func (a *App) ListReleases(c ListConfigProvider) error { } return - }, false, SetFilter(true)) + }, false, false, SetFilter(true)) close(releasesChan) @@ -1211,7 +1211,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 +1224,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 +1328,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 +1361,7 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir stri _, err := converge(st) return err }, - includeTransitiveNeeds) + includeNeeds, includeTransitiveNeeds) } } @@ -1382,9 +1382,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 +1430,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 +1523,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 } @@ -1600,7 +1600,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 +1804,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 +2024,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 +2073,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 +2287,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} } @@ -2387,7 +2387,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 +2409,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..f956abaf 100644 --- a/pkg/app/app_sequential_test.go +++ b/pkg/app/app_sequential_test.go @@ -57,7 +57,8 @@ releases: err = app.ForEachState( Noop, false, - SetFilter(true), + + false,SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -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..50e3682a 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" @@ -4331,7 +4350,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4404,7 +4424,8 @@ releases: err := app.ForEachState( collectReleases, false, - SetFilter(true), + + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4616,7 +4637,8 @@ func TestRenderYamlEnvVar(t *testing.T) { return false, nil }, false, - SetFilter(true), + + false, SetFilter(true), ) if tc.expectErr { 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..0fbfb29a 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, false, c.IncludeTransitiveNeeds()) } func (r *Run) Repos(c ReposConfigProvider) error { 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..7b29bf17 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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..03ac6329 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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..9d8772e5 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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..6ff1c0e1 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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..9cd3574f 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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..0b6c8fa5 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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..1643f8a4 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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..29c25fbe 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -2 release(s) matching app=test found in helmfile.yaml.gotmpl +3 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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 431864ea..aa18e0a5 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1342,6 +1342,7 @@ type ChartPrepareOptions struct { WaitForJobs bool OutputDir string OutputDirTemplate string + IncludeNeeds bool IncludeTransitiveNeeds bool Concurrency int KubeVersion string @@ -1818,7 +1819,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} } @@ -2941,16 +2942,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 { @@ -2982,6 +2983,8 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, values map } if includeTransitiveNeeds { unmarkNeedsAndTransitives(filteredReleases, releases) + } else if includeNeeds { + unmarkNeedsDirectOnly(filteredReleases, releases) } return filteredReleases, nil } @@ -3039,6 +3042,23 @@ func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []Release unmarkReleases(needsWithTranstives, filteredReleases) } +func unmarkNeedsDirectOnly(filteredReleases []Release, allReleases []ReleaseSpec) { + directNeeds := collectDirectNeedsOnly(filteredReleases) + unmarkReleases(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 collectAllNeedsWithTransitives(filteredReleases []Release, allReleases []ReleaseSpec) map[string]struct{} { needsWithTranstives := map[string]struct{}{} for _, r := range filteredReleases { @@ -3072,8 +3092,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 } @@ -3088,8 +3108,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 } @@ -3169,7 +3189,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 { @@ -3177,7 +3197,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..6034c258 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 } diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 9fc5d893..ea03e045 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -2504,7 +2504,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) { @@ -2973,7 +2973,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 From 849c4e4e091399d73a138170ec07bfccaad95851 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 09:30:57 +0800 Subject: [PATCH 02/12] fix: address lint errors in CI - Remove unused allReleases parameter from unmarkNeedsDirectOnly - Fix gci formatting in app_sequential_test.go Signed-off-by: yxxhero --- pkg/app/app_sequential_test.go | 10 +++++----- pkg/state/state.go | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/app/app_sequential_test.go b/pkg/app/app_sequential_test.go index f956abaf..57823216 100644 --- a/pkg/app/app_sequential_test.go +++ b/pkg/app/app_sequential_test.go @@ -57,8 +57,8 @@ releases: err = app.ForEachState( Noop, false, - - false,SetFilter(true), + false, + SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -194,7 +194,7 @@ releases: noop, false, - false,SetFilter(true), + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -408,7 +408,7 @@ releases: failingConverge, false, - false,SetFilter(true), + false, SetFilter(true), ) if err == nil { @@ -476,7 +476,7 @@ replicaCount: 3 captureState, false, - false,SetFilter(true), + false, SetFilter(true), ) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/pkg/state/state.go b/pkg/state/state.go index aa18e0a5..d700fb93 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -2984,7 +2984,7 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, values map if includeTransitiveNeeds { unmarkNeedsAndTransitives(filteredReleases, releases) } else if includeNeeds { - unmarkNeedsDirectOnly(filteredReleases, releases) + unmarkNeedsDirectOnly(filteredReleases) } return filteredReleases, nil } @@ -3042,7 +3042,7 @@ func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []Release unmarkReleases(needsWithTranstives, filteredReleases) } -func unmarkNeedsDirectOnly(filteredReleases []Release, allReleases []ReleaseSpec) { +func unmarkNeedsDirectOnly(filteredReleases []Release) { directNeeds := collectDirectNeedsOnly(filteredReleases) unmarkReleases(directNeeds, filteredReleases) } From c07a82da700c3af713636677b1812ae4a19c8bc9 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 09:50:30 +0800 Subject: [PATCH 03/12] fix: use config for includeNeeds in Deps command - Add IncludeNeeds() method to DepsConfigProvider interface - Implement IncludeNeeds() in DepsImpl - Update depsConfig in tests - Use c.IncludeNeeds() in run.go instead of hardcoded false Signed-off-by: yxxhero --- pkg/app/app_test.go | 19 +++++++++++++------ pkg/app/config.go | 1 + pkg/app/run.go | 2 +- pkg/config/deps.go | 5 +++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 50e3682a..36aaf9f5 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2626,22 +2626,28 @@ func (a applyConfig) TrackLogs() bool { type depsConfig struct { skipRepos bool + includeNeeds bool includeTransitiveNeeds bool + args []string } -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 } @@ -4116,6 +4122,7 @@ releases: depsErr := app.Deps(depsConfig{ skipRepos: false, + includeNeeds: false, includeTransitiveNeeds: false, }) switch { diff --git a/pkg/app/config.go b/pkg/app/config.go index 03bdc627..de34473f 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 diff --git a/pkg/app/run.go b/pkg/app/run.go index 0fbfb29a..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, false, c.IncludeTransitiveNeeds()) + return r.state.UpdateDeps(r.helm, c.IncludeNeeds(), c.IncludeTransitiveNeeds()) } func (r *Run) Repos(c ReposConfigProvider) error { 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 From 60214c2c7833a3f178bb90e93a9b7cb3c02837b7 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 10:00:35 +0800 Subject: [PATCH 04/12] fix: use config for IncludeNeeds in Deps and Repos commands - Add IncludeNeeds() to DepsConfigProvider and ReposConfigProvider - Update depsConfig in tests - Use c.IncludeNeeds() in run.go instead of hardcoded false Signed-off-by: yxxhero --- pkg/app/app.go | 4 ++-- pkg/app/config.go | 1 + pkg/config/repos.go | 5 +++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 332bd7b2..75ee10e5 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 - }, false, 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 - }, false, c.IncludeTransitiveNeeds(), SetFilter(true)) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), SetFilter(true)) } func (a *App) Diff(c DiffConfigProvider) error { diff --git a/pkg/app/config.go b/pkg/app/config.go index de34473f..4dc71edd 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -39,6 +39,7 @@ type DepsConfigProvider interface { type ReposConfigProvider interface { Args() string + IncludeNeeds() bool IncludeTransitiveNeeds() bool } 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 From c03a24f83d8ed45178657c6b32b4256a1fd4e36c Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 10:08:31 +0800 Subject: [PATCH 05/12] fix: remove unused args field from depsConfig Signed-off-by: yxxhero --- pkg/app/app_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 36aaf9f5..ce40b48c 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2628,7 +2628,6 @@ type depsConfig struct { skipRepos bool includeNeeds bool includeTransitiveNeeds bool - args []string } func (c depsConfig) SkipRepos() bool { From 425cf60d9a2e5700a58df1758ff87b6c7ee8d89f Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 10:27:12 +0800 Subject: [PATCH 06/12] fix: use config for IncludeNeeds in WriteValues command - Add IncludeNeeds() to WriteValuesConfigProvider interface - Implement IncludeNeeds() in WriteValuesImpl - Update WriteValues function to use IncludeNeeds and IncludeTransitiveNeeds Signed-off-by: yxxhero --- pkg/app/app.go | 14 ++++++++------ pkg/app/config.go | 1 + pkg/config/write-values.go | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 75ee10e5..2966b8dc 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -260,11 +260,13 @@ func (a *App) Template(c TemplateConfigProvider) error { 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 +276,7 @@ func (a *App) WriteValues(c WriteValuesConfigProvider) error { } return - }, false, c.IncludeTransitiveNeeds(), SetFilter(true)) + }, c.IncludeNeeds(), c.IncludeTransitiveNeeds(), SetFilter(true)) } type MultiError struct { diff --git a/pkg/app/config.go b/pkg/app/config.go index 4dc71edd..b64296cd 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -281,6 +281,7 @@ type WriteValuesConfigProvider interface { SkipDeps() bool SkipRefresh() bool SkipCleanup() bool + IncludeNeeds() bool IncludeTransitiveNeeds() bool concurrencyConfig 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 +} From d68fb89f32f6f74585bfb160cede4e4f7f55cee4 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 10:53:01 +0800 Subject: [PATCH 07/12] fix: add IncludeNeeds to all ChartPrepareOptions - Add IncludeNeeds to diff, template, lint, unittest, sync, apply ChartPrepareOptions - All ConfigProviders already have DAGConfig embedded which includes IncludeNeeds() Signed-off-by: yxxhero --- pkg/app/app.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/app/app.go b/pkg/app/app.go index 2966b8dc..3c60fbd4 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -169,6 +169,7 @@ func (a *App) Diff(c DiffConfigProvider) error { IncludeCRDs: &includeCRDs, Validate: c.Validate(), Concurrency: c.Concurrency(), + IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { msg, matched, affected, errs = a.diff(run, c) @@ -240,6 +241,7 @@ func (a *App) Template(c TemplateConfigProvider) error { SkipCleanup: c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), + IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), Set: c.Set(), Values: c.Values(), @@ -319,6 +321,7 @@ func (a *App) Lint(c LintConfigProvider) error { SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), + IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { ok, lintErrs, errs = a.lint(run, c) @@ -360,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) @@ -419,6 +423,7 @@ func (a *App) Sync(c SyncConfigProvider) error { WaitRetries: c.WaitRetries(), WaitForJobs: c.WaitForJobs(), IncludeCRDs: &includeCRDs, + IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), Validate: c.Validate(), Concurrency: c.Concurrency(), @@ -457,6 +462,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { SkipCleanup: c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), + IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), }, func() { matched, updated, es := a.apply(run, c) From d88292f0d7d5e14c5abaf880594adac02f333770 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 14:03:55 +0800 Subject: [PATCH 08/12] fix: IncludeTransitiveNeeds should also enable WithDependencies in DAG planning Signed-off-by: yxxhero --- pkg/state/state_run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index 6034c258..cdd42d6b 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -163,7 +163,7 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas plan, err := d.Plan(dag.SortOptions{ Only: selectedReleaseIDs, - WithDependencies: opts.IncludeNeeds, + WithDependencies: opts.IncludeNeeds || opts.IncludeTransitiveNeeds, WithoutDependencies: opts.SkipNeeds, }) if err != nil { From 15df67f17a65c52fa7539631a37a8be4a5e66c39 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 14:19:05 +0800 Subject: [PATCH 09/12] fix: log should show only releases matching selector, not including needs Signed-off-by: yxxhero --- pkg/app/app.go | 6 +- .../include-transitive-needs=true/log | 2 +- .../skip-needs=false_include-needs=true/log | 2 +- .../log | 2 +- .../log | 2 +- .../log | 2 +- .../skip-needs=false_include-needs=true/log | 2 +- .../log | 2 +- .../log | 2 +- .../log | 2 +- pkg/state/selector_include_needs_test.go | 346 ++++++++++++++++++ 11 files changed, 360 insertions(+), 10 deletions(-) create mode 100644 pkg/state/selector_include_needs_test.go diff --git a/pkg/app/app.go b/pkg/app/app.go index 3c60fbd4..fd947645 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1597,7 +1597,11 @@ func (a *App) getSelectedReleases(r *Run, includeNeeds bool, includeTransitiveNe 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 } 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/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 7b29bf17..62337d68 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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 03ac6329..77069b44 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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 9d8772e5..1a00d70c 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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 6ff1c0e1..58c602da 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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 9cd3574f..3706c59d 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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 0b6c8fa5..7f9b3818 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 @@ -26,7 +26,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": 23: merged environment: &{default map[] map[] map[]} -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl Affected releases are: external-secrets (incubator/raw) UPDATED 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 1643f8a4..0144f44a 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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 29c25fbe..b67a15d7 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 @@ -28,7 +28,7 @@ rendering result of "helmfile.yaml.gotmpl.part.0": merged environment: &{default map[] map[] map[]} WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs -3 release(s) matching app=test found in helmfile.yaml.gotmpl +2 release(s) matching app=test found in helmfile.yaml.gotmpl WARNING: release external-secrets needs kubernetes-external-secrets, but kubernetes-external-secrets is not installed due to installed: false. Either mark kubernetes-external-secrets as installed or remove kubernetes-external-secrets from external-secrets's needs Affected releases are: 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) + } + }) + } +} From 5236778ad83fb4d933b933e0928ea851e319dd29 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 14:31:56 +0800 Subject: [PATCH 10/12] test: add integration test for include-needs vs include-transitive-needs Signed-off-by: yxxhero --- test/integration/run.sh | 1 + .../test-cases/include-needs-transitive.sh | 74 +++++++++++++++++++ .../input/helmfile.yaml | 56 ++++++++++++++ 3 files changed, 131 insertions(+) create mode 100644 test/integration/test-cases/include-needs-transitive.sh create mode 100644 test/integration/test-cases/include-needs-transitive/input/helmfile.yaml 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..318d74ae --- /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=serviceA template --include-needs > ${include_needs_tmp}/include-needs.log 2>&1 || fail "helmfile template --include-needs should not fail" + +# Verify that serviceA, serviceB are included in the output (serviceB is direct need of serviceA) +# serviceC 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: serviceA" && \ + echo "${include_needs_output}" | grep -q "name: serviceB" && \ + ! echo "${include_needs_output}" | grep -q "name: serviceC"; then + info "--include-needs correctly includes only direct dependencies (serviceA, serviceB)" +else + cat ${include_needs_tmp}/include-needs.log + fail "--include-needs should include only serviceA and serviceB (direct need), not serviceC (transitive)" +fi + +# Verify log shows "1 release(s) matching name=serviceA" (not 2 or 3) +if echo "${include_needs_output}" | grep -q "1 release(s) matching name=serviceA"; 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=serviceA', 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=serviceA template --include-transitive-needs > ${include_needs_tmp}/include-transitive-needs.log 2>&1 || fail "helmfile template --include-transitive-needs should not fail" + +# Verify that serviceA, serviceB, serviceC are all included +transitive_output=$(cat ${include_needs_tmp}/include-transitive-needs.log) + +if echo "${transitive_output}" | grep -q "name: serviceA" && \ + echo "${transitive_output}" | grep -q "name: serviceB" && \ + echo "${transitive_output}" | grep -q "name: serviceC"; then + info "--include-transitive-needs correctly includes all transitive dependencies (serviceA, serviceB, serviceC)" +else + cat ${include_needs_tmp}/include-transitive-needs.log + fail "--include-transitive-needs should include serviceA, serviceB, and serviceC (transitive)" +fi + +# Verify log still shows "1 release(s) matching name=serviceA" (selector match, not total) +if echo "${transitive_output}" | grep -q "1 release(s) matching name=serviceA"; 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=serviceA', not including needs count" +fi + +# Test 3: Verify serviceD is never included (not in dependency chain) +if ! echo "${include_needs_output}" | grep -q "name: serviceD" && \ + ! echo "${transitive_output}" | grep -q "name: serviceD"; then + info "serviceD correctly not included (not in dependency chain)" +else + fail "serviceD 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..6d6c97b7 --- /dev/null +++ b/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml @@ -0,0 +1,56 @@ +releases: + - name: serviceA + chart: ../../../charts/raw + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }} + needs: + - serviceB + + - name: serviceB + chart: ../../../charts/raw + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }} + needs: + - serviceC + + - name: serviceC + chart: ../../../charts/raw + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }} + + - name: serviceD + chart: ../../../charts/raw + values: + - templates: + - | + apiVersion: v1 + kind: ConfigMap + metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} + data: + name: {{ .Release.Name }} From 9f275c038ec520312c6eaf00ef80aefbd8608f61 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 15:33:45 +0800 Subject: [PATCH 11/12] fix: include-needs should only include direct dependencies, not transitive Signed-off-by: yxxhero --- pkg/app/app.go | 5 ++-- .../include-transitive-needs=true/log | 8 ++--- .../skip-needs=false_include-needs=true/log | 27 +++++++---------- .../log | 8 ++--- .../log | 21 +++++--------- .../log | 8 ++--- .../log | 6 ++-- .../skip-needs=false_include-needs=true/log | 27 +++++++---------- .../log | 8 ++--- .../log | 21 +++++--------- .../log | 8 ++--- .../log | 6 ++-- pkg/state/state.go | 14 ++++++++- pkg/state/state_run.go | 29 ++++++++++++++----- 14 files changed, 98 insertions(+), 98 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index fd947645..1e7c5887 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -2354,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 { 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 d00bbb0d..df204aec 100644 --- a/pkg/app/testdata/testapply_2/include-transitive-needs=true/log +++ b/pkg/app/testdata/testapply_2/include-transitive-needs=true/log @@ -29,15 +29,11 @@ Affected releases are: 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/state/state.go b/pkg/state/state.go index d700fb93..1354cdef 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -3044,7 +3044,7 @@ func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []Release func unmarkNeedsDirectOnly(filteredReleases []Release) { directNeeds := collectDirectNeedsOnly(filteredReleases) - unmarkReleases(directNeeds, filteredReleases) + unmarkReleasesByNeedID(directNeeds, filteredReleases) } func collectDirectNeedsOnly(filteredReleases []Release) map[string]struct{} { @@ -3059,6 +3059,18 @@ func collectDirectNeedsOnly(filteredReleases []Release) map[string]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 { diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index cdd42d6b..0f0befbf 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -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 || opts.IncludeTransitiveNeeds, - WithoutDependencies: opts.SkipNeeds, + WithDependencies: false, + WithoutDependencies: skipDepValidation, }) if err != nil { if ude, ok := err.(*dag.UnhandledDependencyError); ok { From 26e063c1fdafe46f24e678fb947e5956b712a2e5 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 15:35:46 +0800 Subject: [PATCH 12/12] test: update integration test with valid Helm release names Signed-off-by: yxxhero --- .../test-cases/include-needs-transitive.sh | 52 +++++++++---------- .../input/helmfile.yaml | 16 +++--- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/test/integration/test-cases/include-needs-transitive.sh b/test/integration/test-cases/include-needs-transitive.sh index 318d74ae..ee8e10d2 100644 --- a/test/integration/test-cases/include-needs-transitive.sh +++ b/test/integration/test-cases/include-needs-transitive.sh @@ -13,59 +13,59 @@ 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=serviceA template --include-needs > ${include_needs_tmp}/include-needs.log 2>&1 || fail "helmfile template --include-needs should not fail" +${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 serviceA, serviceB are included in the output (serviceB is direct need of serviceA) -# serviceC should NOT be included (it's transitive, not direct) +# 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: serviceA" && \ - echo "${include_needs_output}" | grep -q "name: serviceB" && \ - ! echo "${include_needs_output}" | grep -q "name: serviceC"; then - info "--include-needs correctly includes only direct dependencies (serviceA, serviceB)" +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 serviceA and serviceB (direct need), not serviceC (transitive)" + 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=serviceA" (not 2 or 3) -if echo "${include_needs_output}" | grep -q "1 release(s) matching name=serviceA"; then +# 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=serviceA', not including needs count" + 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=serviceA template --include-transitive-needs > ${include_needs_tmp}/include-transitive-needs.log 2>&1 || fail "helmfile template --include-transitive-needs should not fail" +${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 serviceA, serviceB, serviceC are all included +# 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: serviceA" && \ - echo "${transitive_output}" | grep -q "name: serviceB" && \ - echo "${transitive_output}" | grep -q "name: serviceC"; then - info "--include-transitive-needs correctly includes all transitive dependencies (serviceA, serviceB, serviceC)" +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 serviceA, serviceB, and serviceC (transitive)" + fail "--include-transitive-needs should include service-a, service-b, and service-c (transitive)" fi -# Verify log still shows "1 release(s) matching name=serviceA" (selector match, not total) -if echo "${transitive_output}" | grep -q "1 release(s) matching name=serviceA"; then +# 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=serviceA', not including needs count" + fail "Log should show '1 release(s) matching name=service-a', not including needs count" fi -# Test 3: Verify serviceD is never included (not in dependency chain) -if ! echo "${include_needs_output}" | grep -q "name: serviceD" && \ - ! echo "${transitive_output}" | grep -q "name: serviceD"; then - info "serviceD correctly not included (not in dependency chain)" +# 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 "serviceD should never be included as it's not in the dependency chain" + fail "service-d should never be included as it's not in the dependency chain" fi # Cleanup diff --git a/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml b/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml index 6d6c97b7..0a432fb1 100644 --- a/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml +++ b/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml @@ -1,6 +1,7 @@ releases: - - name: serviceA + - name: service-a chart: ../../../charts/raw + namespace: helmfile-tests values: - templates: - | @@ -12,10 +13,11 @@ releases: data: name: {{ .Release.Name }} needs: - - serviceB + - service-b - - name: serviceB + - name: service-b chart: ../../../charts/raw + namespace: helmfile-tests values: - templates: - | @@ -27,10 +29,11 @@ releases: data: name: {{ .Release.Name }} needs: - - serviceC + - service-c - - name: serviceC + - name: service-c chart: ../../../charts/raw + namespace: helmfile-tests values: - templates: - | @@ -42,8 +45,9 @@ releases: data: name: {{ .Release.Name }} - - name: serviceD + - name: service-d chart: ../../../charts/raw + namespace: helmfile-tests values: - templates: - |