diff --git a/pkg/app/app.go b/pkg/app/app.go index cdc31745..95a0bf3d 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -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) }) @@ -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(), @@ -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) }) @@ -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() { @@ -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) diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index f2e5f50c..3266f509 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -205,9 +205,8 @@ releases: error: ``, selectors: []string{"app=test"}, // Issue #2309: --kube-context is now added to template flags + // Issue #1003: --include-needs should only include direct dependencies, not transitive templated: []exectest.Release{ - // TODO: Turned out we can't differentiate needs vs transitive needs in this case :thinking: - {Name: "logging", Flags: []string{"--kube-context", "default", "--namespace", "kube-system"}}, {Name: "kubernetes-external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "kube-system"}}, {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, diff --git a/test/integration/run.sh b/test/integration/run.sh index 80099707..946e595e 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -138,6 +138,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2431.sh . ${dir}/test-cases/kubedog-tracking.sh +. ${dir}/test-cases/include-needs-transitive.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- all_tests_passed diff --git a/test/integration/test-cases/include-needs-transitive.sh b/test/integration/test-cases/include-needs-transitive.sh new file mode 100755 index 00000000..74e33f6a --- /dev/null +++ b/test/integration/test-cases/include-needs-transitive.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash + +# Test case for issue #1003: --include-needs should only include direct dependencies +# while --include-transitive-needs should include both direct and transitive dependencies + +test_case_dir="${cases_dir}/include-needs-transitive" +input_dir="${test_case_dir}/input" +output_dir="${test_case_dir}/output" + +tmp_dir=$(mktemp -d) +trap "rm -rf ${tmp_dir}" EXIT + +test_start "include-needs vs include-transitive-needs" + +info "https://github.com/helmfile/helmfile/issues/1003" + +# Test 1: --include-needs should only include direct dependencies +info "Test 1: template with --include-needs should only include direct dependencies" +template_output="${tmp_dir}/include-needs.log" +${helmfile} -f ${input_dir}/helmfile.yaml -l name=release3 template --include-needs > ${template_output} 2>&1 +code=$? +if [ ${code} -ne 0 ]; then + cat ${template_output} + fail "helmfile template with --include-needs should not fail" +fi + +# Should include release2 (direct dependency) and release3 (selected) +# Should NOT include release1 (transitive dependency) +if grep -q "release1" ${template_output}; then + fail "--include-needs should NOT include transitive dependency release1" +fi + +if ! grep -q "release2" ${template_output}; then + fail "--include-needs should include direct dependency release2" +fi + +if ! grep -q "release3" ${template_output}; then + fail "--include-needs should include selected release release3" +fi + +info "PASS: --include-needs only includes direct dependencies" + +# Test 2: --include-transitive-needs should include all dependencies +info "Test 2: template with --include-transitive-needs should include all dependencies" +template_output="${tmp_dir}/include-transitive-needs.log" +${helmfile} -f ${input_dir}/helmfile.yaml -l name=release3 template --include-transitive-needs > ${template_output} 2>&1 +code=$? +if [ ${code} -ne 0 ]; then + cat ${template_output} + fail "helmfile template with --include-transitive-needs should not fail" +fi + +# Should include all: release1, release2, and release3 +if ! grep -q "release1" ${template_output}; then + fail "--include-transitive-needs should include transitive dependency release1" +fi + +if ! grep -q "release2" ${template_output}; then + fail "--include-transitive-needs should include direct dependency release2" +fi + +if ! grep -q "release3" ${template_output}; then + fail "--include-transitive-needs should include selected release release3" +fi + +info "PASS: --include-transitive-needs includes all dependencies" + +# Test 3: Verify same behavior for lint command +info "Test 3: lint with --include-needs should only include direct dependencies" +lint_output="${tmp_dir}/lint-include-needs.log" +${helmfile} -f ${input_dir}/helmfile.yaml -l name=release3 lint --include-needs > ${lint_output} 2>&1 +code=$? +if [ ${code} -ne 0 ]; then + cat ${lint_output} + fail "helmfile lint with --include-needs should not fail" +fi + +if grep -q "release1" ${lint_output}; then + fail "lint with --include-needs should NOT include transitive dependency release1" +fi + +info "PASS: lint with --include-needs only includes direct dependencies" + +# Test 4: Verify same behavior for diff command +info "Test 4: diff with --include-needs should only include direct dependencies" +diff_output="${tmp_dir}/diff-include-needs.log" +${helmfile} -f ${input_dir}/helmfile.yaml -l name=release3 diff --include-needs > ${diff_output} 2>&1 +code=$? +if [ ${code} -ne 0 ]; then + cat ${diff_output} + fail "helmfile diff with --include-needs should not fail" +fi + +if grep -q "release1" ${diff_output}; then + fail "diff with --include-needs should NOT include transitive dependency release1" +fi + +info "PASS: diff with --include-needs only includes direct dependencies" + +test_pass "include-needs vs include-transitive-needs" diff --git a/test/integration/test-cases/include-needs-transitive/README.md b/test/integration/test-cases/include-needs-transitive/README.md new file mode 100644 index 00000000..013ee54b --- /dev/null +++ b/test/integration/test-cases/include-needs-transitive/README.md @@ -0,0 +1,35 @@ +# Test Case: include-needs vs include-transitive-needs + +This test case validates the fix for issue #1003. + +## Issue +`--include-needs` flag was incorrectly including transitive dependencies when it should only include direct dependencies. + +## Expected Behavior + +### --include-needs +When selecting `release3` with `--include-needs`: +- **Should include**: `release2` (direct dependency) and `release3` (selected release) +- **Should NOT include**: `release1` (transitive dependency of release3 through release2) + +### --include-transitive-needs +When selecting `release3` with `--include-transitive-needs`: +- **Should include**: `release1`, `release2`, and `release3` (all dependencies in the chain) + +## Dependency Chain +``` +release1 (no dependencies) + ↑ +release2 (needs release1) + ↑ +release3 (needs release2) +``` + +## Tests Performed +1. `helmfile template --include-needs` - verifies only direct dependencies are included +2. `helmfile template --include-transitive-needs` - verifies all dependencies are included +3. `helmfile lint --include-needs` - verifies consistency across commands +4. `helmfile diff --include-needs` - verifies consistency across commands + +## References +- Issue: https://github.com/helmfile/helmfile/issues/1003 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..bf4b6a11 --- /dev/null +++ b/test/integration/test-cases/include-needs-transitive/input/helmfile.yaml @@ -0,0 +1,23 @@ +repositories: + - name: incubator + url: https://charts.helm.sh/incubator + +releases: + # release1 has no dependencies - this is a transitive dependency of release3 + - name: release1 + chart: incubator/raw + namespace: default + + # release2 depends on release1 - this is a direct dependency of release3 + - name: release2 + chart: incubator/raw + namespace: default + needs: + - default/release1 + + # release3 depends on release2 (and transitively on release1) + - name: release3 + chart: incubator/raw + namespace: default + needs: + - default/release2