From c4a828686e078f191baaa0791360da3f208de510 Mon Sep 17 00:00:00 2001 From: Aditya Menon Date: Fri, 16 Jan 2026 13:32:33 +0100 Subject: [PATCH] fix: pass --kube-context to helm template when using jsonPatches (#2363) fix: pass --kube-context to helm template when using jsonPatches (#2309) When using jsonPatches or strategicMergePatches in helmfile, the `helm template` command was not receiving the `--kube-context` flag. This caused issues when `--dry-run=server` was used (introduced in PR #2271 to support lookup() functions), because helm would connect to the wrong cluster context. Root Cause: 1. `flagsForTemplate()` did not call `appendConnectionFlags()`, unlike `flagsForUpgrade()` and `flagsForDiff()` which both include this call. 2. `processChartification()` did not include `--kube-context` when setting `chartifyOpts.TemplateArgs` for internal helm template calls. Fix: 1. Added `appendConnectionFlags()` call to `flagsForTemplate()` to ensure kube-context and other connection flags are passed to helm template. 2. Added `getKubeContext()` helper function that resolves kube-context with proper priority: release > environment > helmDefaults. 3. Modified `processChartification()` to include `--kube-context` in chartifyOpts.TemplateArgs when chartify needs to run helm template. 4. Added compatibility check for `--validate` flag to avoid Helm 4 mutual exclusion error between --validate and --dry-run (Issue #2355). Fixes #2309 Signed-off-by: Aditya Menon --- pkg/app/app_template_test.go | 52 +++-- pkg/app/app_test.go | 14 +- pkg/state/state.go | 59 +++++- pkg/state/state_test.go | 200 ++++++++++++++++++ test/integration/run.sh | 1 + .../issue-2309-kube-context-template.sh | 32 +++ .../input/helmfile.yaml.gotmpl | 30 +++ .../output/template | 15 ++ 8 files changed, 367 insertions(+), 36 deletions(-) create mode 100644 test/integration/test-cases/issue-2309-kube-context-template.sh create mode 100644 test/integration/test-cases/issue-2309-kube-context-template/input/helmfile.yaml.gotmpl create mode 100644 test/integration/test-cases/issue-2309-kube-context-template/output/template diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index 780249cc..f2e5f50c 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -188,9 +188,10 @@ releases: skipNeeds: true, }, selectors: []string{"app=test"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "external-secrets", Flags: []string{"--namespace", "default"}}, - {Name: "my-release", Flags: []string{"--namespace", "default"}}, + {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default"}}, }, }) }) @@ -203,12 +204,13 @@ releases: }, error: ``, selectors: []string{"app=test"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ // TODO: Turned out we can't differentiate needs vs transitive needs in this case :thinking: - {Name: "logging", Flags: []string{"--namespace", "kube-system"}}, - {Name: "kubernetes-external-secrets", Flags: []string{"--namespace", "kube-system"}}, - {Name: "external-secrets", Flags: []string{"--namespace", "default"}}, - {Name: "my-release", Flags: []string{"--namespace", "default"}}, + {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"}}, }, }) }) @@ -221,11 +223,12 @@ releases: }, error: ``, selectors: []string{"app=test"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "logging", Flags: []string{"--namespace", "kube-system"}}, - {Name: "kubernetes-external-secrets", Flags: []string{"--namespace", "kube-system"}}, - {Name: "external-secrets", Flags: []string{"--namespace", "default"}}, - {Name: "my-release", Flags: []string{"--namespace", "default"}}, + {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"}}, }, }) }) @@ -237,8 +240,9 @@ releases: includeNeeds: true, }, selectors: []string{"name=test2"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "test2"}, + {Name: "test2", Flags: []string{"--kube-context", "default"}}, }, }) }) @@ -250,9 +254,10 @@ releases: includeNeeds: true, }, selectors: []string{"name=test3"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "test2"}, - {Name: "test3"}, + {Name: "test2", Flags: []string{"--kube-context", "default"}}, + {Name: "test3", Flags: []string{"--kube-context", "default"}}, }, }) }) @@ -265,9 +270,10 @@ releases: includeTransitiveNeeds: true, }, selectors: []string{"name=test3"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "test2"}, - {Name: "test3"}, + {Name: "test2", Flags: []string{"--kube-context", "default"}}, + {Name: "test3", Flags: []string{"--kube-context", "default"}}, }, }) }) @@ -280,8 +286,9 @@ releases: includeTransitiveNeeds: true, }, selectors: []string{"name=test2"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "test2"}, + {Name: "test2", Flags: []string{"--kube-context", "default"}}, }, }) }) @@ -294,9 +301,10 @@ releases: includeTransitiveNeeds: true, }, selectors: []string{"name=test3"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "test2"}, - {Name: "test3"}, + {Name: "test2", Flags: []string{"--kube-context", "default"}}, + {Name: "test3", Flags: []string{"--kube-context", "default"}}, }, }) }) @@ -308,9 +316,10 @@ releases: noHooks: true, }, selectors: []string{"app=test"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "external-secrets", Flags: []string{"--namespace", "default", "--no-hooks"}}, - {Name: "my-release", Flags: []string{"--namespace", "default", "--no-hooks"}}, + {Name: "external-secrets", Flags: []string{"--kube-context", "default", "--namespace", "default", "--no-hooks"}}, + {Name: "my-release", Flags: []string{"--kube-context", "default", "--namespace", "default", "--no-hooks"}}, }, }) }) @@ -329,8 +338,9 @@ releases: showOnly: []string{"templates/resources.yaml"}, }, selectors: []string{"name=logging"}, + // Issue #2309: --kube-context is now added to template flags templated: []exectest.Release{ - {Name: "logging", Flags: []string{"--show-only", "templates/resources.yaml", "--namespace", "kube-system"}}, + {Name: "logging", Flags: []string{"--show-only", "templates/resources.yaml", "--kube-context", "default", "--namespace", "kube-system"}}, }, }) }) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index e129aa99..fce32b84 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2754,9 +2754,10 @@ releases: } var helm = &mockHelmExec{} + // Issue #2309: --kube-context is now added to template flags var wantReleases = []mockTemplates{ - {name: "myrelease1", chart: "stable/mychart1", flags: []string{"--namespace", "testNamespace", "--set", "foo=a", "--set", "bar=b", "--output-dir", "output/subdir/helmfile-[a-z0-9]{8}-myrelease1"}}, - {name: "myrelease2", chart: "stable/mychart2", flags: []string{"--namespace", "testNamespace", "--set", "foo=a", "--set", "bar=b", "--output-dir", "output/subdir/helmfile-[a-z0-9]{8}-myrelease2"}}, + {name: "myrelease1", chart: "stable/mychart1", flags: []string{"--kube-context", "default", "--namespace", "testNamespace", "--set", "foo=a", "--set", "bar=b", "--output-dir", "output/subdir/helmfile-[a-z0-9]{8}-myrelease1"}}, + {name: "myrelease2", chart: "stable/mychart2", flags: []string{"--kube-context", "default", "--namespace", "testNamespace", "--set", "foo=a", "--set", "bar=b", "--output-dir", "output/subdir/helmfile-[a-z0-9]{8}-myrelease2"}}, } var wantRepos = []mockRepo{ @@ -2806,7 +2807,8 @@ releases: t.Errorf("chart = [%v], want %v", helm.templated[i].chart, wantReleases[i].chart) } for j := range wantReleases[i].flags { - if j == 7 { + // Issue #2309: regex index changed from 7 to 9 due to added --kube-context flag + if j == 9 { matched, _ := regexp.Match(wantReleases[i].flags[j], []byte(helm.templated[i].flags[j])) if !matched { t.Errorf("HelmState.TemplateReleases() = [%v], want %v", helm.templated[i].flags[j], wantReleases[i].flags[j]) @@ -2834,8 +2836,9 @@ releases: } var helm = &mockHelmExec{} + // Issue #2309: --kube-context is now added to template flags var wantReleases = []mockTemplates{ - {name: "myrelease1", chart: "stable/mychart1", flags: []string{"--api-versions", "helmfile.test/v1", "--api-versions", "helmfile.test/v2", "--kube-version", "v1.21", "--namespace", "testNamespace", "--output-dir", "output/subdir/helmfile-[a-z0-9]{8}-myrelease1"}}, + {name: "myrelease1", chart: "stable/mychart1", flags: []string{"--api-versions", "helmfile.test/v1", "--api-versions", "helmfile.test/v2", "--kube-version", "v1.21", "--kube-context", "default", "--namespace", "testNamespace", "--output-dir", "output/subdir/helmfile-[a-z0-9]{8}-myrelease1"}}, } var buffer bytes.Buffer @@ -2874,7 +2877,8 @@ releases: t.Errorf("chart = [%v], want %v", helm.templated[i].chart, wantReleases[i].chart) } for j := range wantReleases[i].flags { - if j == 9 { + // Issue #2309: regex index changed from 9 to 11 due to added --kube-context flag + if j == 11 { matched, _ := regexp.Match(wantReleases[i].flags[j], []byte(helm.templated[i].flags[j])) if !matched { t.Errorf("HelmState.TemplateReleases() = [%v], want %v", helm.templated[i].flags[j], wantReleases[i].flags[j]) diff --git a/pkg/state/state.go b/pkg/state/state.go index 634f1e6a..701f249a 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1511,11 +1511,36 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re // The lookup() function can be used with or without patches, so we enable cluster access // for all cluster-requiring operations (diff, apply, sync, etc.) but not for offline // commands (template, lint, build, etc.) + // Issue #2309: Also pass --kube-context to chartify's internal helm template call + // Issue #2355: In Helm 4, --validate and --dry-run are mutually exclusive flags. + // When --validate is set, we skip adding --dry-run=server since --validate already + // provides server-side validation. if requiresCluster { - if chartifyOpts.TemplateArgs == "" { - chartifyOpts.TemplateArgs = "--dry-run=server" - } else if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { - chartifyOpts.TemplateArgs += " --dry-run=server" + // Get the effective kube-context for this release + kubeContext := st.getKubeContext(release) + + // Build the additional args needed for cluster-requiring commands + var additionalArgs []string + + // Add --kube-context if configured (Issue #2309) + // Note: kube-context is independent of the validate/dry-run mutual exclusion + if kubeContext != "" && !strings.Contains(chartifyOpts.TemplateArgs, "--kube-context") { + additionalArgs = append(additionalArgs, "--kube-context", kubeContext) + } + + // Add --dry-run=server if not already present (Issue #2271) + // Skip if --validate is set to avoid Helm 4 mutual exclusion error (Issue #2355) + if !opts.Validate && !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { + additionalArgs = append(additionalArgs, "--dry-run=server") + } + + // Append the additional args to TemplateArgs + if len(additionalArgs) > 0 { + if chartifyOpts.TemplateArgs == "" { + chartifyOpts.TemplateArgs = strings.Join(additionalArgs, " ") + } else { + chartifyOpts.TemplateArgs += " " + strings.Join(additionalArgs, " ") + } } } @@ -3082,14 +3107,26 @@ func (st *HelmState) appendEnableDNSFlags(flags []string, release *ReleaseSpec) return flags } +// getKubeContext returns the effective kube-context for a release. +// It follows the priority: release.KubeContext > environment.KubeContext > helmDefaults.KubeContext +// Issue #2309: This is the single source of truth for kube-context resolution +func (st *HelmState) getKubeContext(release *ReleaseSpec) string { + if release.KubeContext != "" { + return release.KubeContext + } + if st.Environments[st.Env.Name].KubeContext != "" { + return st.Environments[st.Env.Name].KubeContext + } + if st.HelmDefaults.KubeContext != "" { + return st.HelmDefaults.KubeContext + } + return "" +} + func (st *HelmState) kubeConnectionFlags(release *ReleaseSpec) []string { flags := []string{} - if release.KubeContext != "" { - flags = append(flags, "--kube-context", release.KubeContext) - } else if st.Environments[st.Env.Name].KubeContext != "" { - flags = append(flags, "--kube-context", st.Environments[st.Env.Name].KubeContext) - } else if st.HelmDefaults.KubeContext != "" { - flags = append(flags, "--kube-context", st.HelmDefaults.KubeContext) + if kubeContext := st.getKubeContext(release); kubeContext != "" { + flags = append(flags, "--kube-context", kubeContext) } return flags } @@ -3270,6 +3307,8 @@ func (st *HelmState) flagsForTemplate(helm helmexec.Interface, release *ReleaseS flags = st.appendChartDownloadFlags(flags, release) flags = st.appendShowOnlyFlags(flags, showOnly) flags = st.appendSkipSchemaValidationFlags(flags, release, skipSchemaValidation) + // Issue #2309: Add kube-context flags for helm template when using jsonPatches with --dry-run=server + flags = st.appendConnectionFlags(flags, release) common, files, err := st.namespaceAndValuesFlags(helm, release, workerIndex) if err != nil { diff --git a/pkg/state/state_test.go b/pkg/state/state_test.go index 86d208fb..f9e8e642 100644 --- a/pkg/state/state_test.go +++ b/pkg/state/state_test.go @@ -784,6 +784,8 @@ func TestHelmState_flagsForTemplate(t *testing.T) { defaults HelmSpec release *ReleaseSpec templateOpts TemplateOpts + environments map[string]EnvironmentSpec + envName string want []string wantErr string }{ @@ -903,15 +905,114 @@ func TestHelmState_flagsForTemplate(t *testing.T) { "--namespace", "test-namespace", }, }, + // Issue #2309: kube-context tests + { + name: "kube-context-from-release", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + KubeContext: "release-context", + }, + want: []string{ + "--version", "0.1", + "--kube-context", "release-context", + "--namespace", "test-namespace", + }, + }, + { + name: "kube-context-from-helmdefaults", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + KubeContext: "default-context", + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + }, + want: []string{ + "--version", "0.1", + "--kube-context", "default-context", + "--namespace", "test-namespace", + }, + }, + { + name: "kube-context-release-overrides-helmdefaults", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + KubeContext: "default-context", + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + KubeContext: "release-context", + }, + want: []string{ + "--version", "0.1", + "--kube-context", "release-context", + "--namespace", "test-namespace", + }, + }, + { + name: "kube-context-from-environment", + defaults: HelmSpec{ + Verify: false, + CreateNamespace: &enable, + }, + version: semver.MustParse("3.10.0"), + release: &ReleaseSpec{ + Chart: "test/chart", + Version: "0.1", + Verify: &disable, + Name: "test-charts", + Namespace: "test-namespace", + }, + environments: map[string]EnvironmentSpec{ + "production": {KubeContext: "env-context"}, + }, + envName: "production", + want: []string{ + "--version", "0.1", + "--kube-context", "env-context", + "--namespace", "test-namespace", + }, + }, } for i := range tests { tt := tests[i] t.Run(tt.name, func(t *testing.T) { + envName := tt.envName + if envName == "" { + envName = "default" + } + environments := tt.environments + if environments == nil { + environments = make(map[string]EnvironmentSpec) + } state := &HelmState{ basePath: "./", ReleaseSetSpec: ReleaseSetSpec{ Releases: []ReleaseSpec{*tt.release}, HelmDefaults: tt.defaults, + Environments: environments, + Env: environment.Environment{Name: envName}, }, valsRuntime: valsRuntime, } @@ -4913,3 +5014,102 @@ func TestChartCache(t *testing.T) { t.Errorf("Expected path %s, got %s", path, cachedPath) } } + +// TestHelmState_getKubeContext tests the kube-context resolution logic +// Issue #2309: This helper is used to pass kube-context to chartify +func TestHelmState_getKubeContext(t *testing.T) { + tests := []struct { + name string + defaults HelmSpec + environments map[string]EnvironmentSpec + envName string + release *ReleaseSpec + want string + }{ + { + name: "returns empty when no context configured", + defaults: HelmSpec{}, + release: &ReleaseSpec{Name: "test"}, + want: "", + }, + { + name: "returns release kubeContext when set", + defaults: HelmSpec{ + KubeContext: "default-context", + }, + release: &ReleaseSpec{ + Name: "test", + KubeContext: "release-context", + }, + want: "release-context", + }, + { + name: "returns helmDefaults kubeContext when release not set", + defaults: HelmSpec{ + KubeContext: "default-context", + }, + release: &ReleaseSpec{ + Name: "test", + }, + want: "default-context", + }, + { + name: "environment overrides helmDefaults when release not set", + defaults: HelmSpec{ + KubeContext: "default-context", + }, + environments: map[string]EnvironmentSpec{ + "production": {KubeContext: "env-context"}, + }, + envName: "production", + release: &ReleaseSpec{ + Name: "test", + }, + want: "env-context", + }, + { + name: "release overrides environment", + defaults: HelmSpec{ + KubeContext: "default-context", + }, + environments: map[string]EnvironmentSpec{ + "production": {KubeContext: "env-context"}, + }, + envName: "production", + release: &ReleaseSpec{ + Name: "test", + KubeContext: "release-context", + }, + want: "release-context", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + envName := tt.envName + if envName == "" { + envName = "default" + } + environments := tt.environments + if environments == nil { + environments = make(map[string]EnvironmentSpec) + } + + state := &HelmState{ + basePath: "./", + ReleaseSetSpec: ReleaseSetSpec{ + HelmDefaults: tt.defaults, + Environments: environments, + Env: environment.Environment{ + Name: envName, + }, + }, + } + + got := state.getKubeContext(tt.release) + if got != tt.want { + t.Errorf("getKubeContext() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/test/integration/run.sh b/test/integration/run.sh index 5d57c08b..06e0b9f3 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -119,6 +119,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2291.sh . ${dir}/test-cases/oci-parallel-pull.sh . ${dir}/test-cases/issue-2297-local-chart-transformers.sh +. ${dir}/test-cases/issue-2309-kube-context-template.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/issue-2309-kube-context-template.sh b/test/integration/test-cases/issue-2309-kube-context-template.sh new file mode 100644 index 00000000..b475f761 --- /dev/null +++ b/test/integration/test-cases/issue-2309-kube-context-template.sh @@ -0,0 +1,32 @@ +# Issue #2309: Test that helm template receives --kube-context when using jsonPatches +# https://github.com/helmfile/helmfile/issues/2309 +# +# This test verifies that helmfile correctly passes --kube-context to helm template +# when helmDefaults.kubeContext is set and jsonPatches are used. + +issue_2309_case_input_dir="${cases_dir}/issue-2309-kube-context-template/input" +issue_2309_case_output_dir="${cases_dir}/issue-2309-kube-context-template/output" + +config_file="helmfile.yaml.gotmpl" +issue_2309_tmp=$(mktemp -d) +issue_2309_template_output=${issue_2309_tmp}/issue_2309.template.log + +test_start "helmfile template with kube-context and jsonPatches (issue #2309)" + +info "Running helmfile template with kubeContext and jsonPatches" +${helmfile} -f ${issue_2309_case_input_dir}/${config_file} template > ${issue_2309_template_output} || fail "\"helmfile template\" shouldn't fail" + +info "Checking template output" +cat ${issue_2309_template_output} + +# Verify the output contains the patched ConfigMap +if grep -q "patched: \"true\"" ${issue_2309_template_output}; then + info "Found patched value in output - jsonPatches applied successfully" +else + fail "jsonPatches were not applied - missing 'patched: true' in output" +fi + +# Compare with expected output +./dyff between -bs ${issue_2309_case_output_dir}/template ${issue_2309_template_output} || fail "\"helmfile template\" output should match expected" + +test_pass "helmfile template with kube-context and jsonPatches (issue #2309)" diff --git a/test/integration/test-cases/issue-2309-kube-context-template/input/helmfile.yaml.gotmpl b/test/integration/test-cases/issue-2309-kube-context-template/input/helmfile.yaml.gotmpl new file mode 100644 index 00000000..b34a3d0b --- /dev/null +++ b/test/integration/test-cases/issue-2309-kube-context-template/input/helmfile.yaml.gotmpl @@ -0,0 +1,30 @@ +# Issue #2309: helm template should receive --kube-context when using jsonPatches +# https://github.com/helmfile/helmfile/issues/2309 +helmDefaults: + kubeContext: test-context + +repositories: +- name: incubator + url: https://charts.helm.sh/incubator +--- +releases: +- name: test-release + chart: incubator/raw + namespace: default + values: + - resources: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: test-configmap + data: + key: value + jsonPatches: + - target: + version: v1 + kind: ConfigMap + name: test-configmap + patch: + - op: add + path: /data/patched + value: "true" diff --git a/test/integration/test-cases/issue-2309-kube-context-template/output/template b/test/integration/test-cases/issue-2309-kube-context-template/output/template new file mode 100644 index 00000000..1a03ca6e --- /dev/null +++ b/test/integration/test-cases/issue-2309-kube-context-template/output/template @@ -0,0 +1,15 @@ +--- +# Source: raw/templates/patched_resources.yaml +apiVersion: v1 +data: + key: value + patched: "true" +kind: ConfigMap +metadata: + labels: + app: raw + chart: raw-0.2.5 + heritage: Helm + release: test-release + name: test-configmap +