From d9f09fd218fc7f053ea6b3bbd64403482779bb25 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 15 Mar 2026 09:10:25 +0800 Subject: [PATCH] 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