From 3ef595ba195f84af0f9ce2f3e8767dcd1a6c0679 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 07:41:26 +0000 Subject: [PATCH 2/4] Add --kube-context flag to chartify TemplateArgs for cluster operations When using jsonPatches (kustomize), the helm template command now receives the --kube-context flag when running cluster-requiring commands (diff, apply, sync, etc.). This ensures that helm uses the correct cluster context defined in helmDefaults.kubeContext, release.kubeContext, or environment.kubeContext. The fix follows the same priority order as the existing kubeConnectionFlags method: 1. release.KubeContext 2. environment.KubeContext 3. helmDefaults.KubeContext Tests added to verify the fix works correctly for different commands and context configurations. Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/chartify_kubecontext_test.go | 289 +++++++++++++++++++++++++ pkg/state/state.go | 32 ++- 2 files changed, 317 insertions(+), 4 deletions(-) create mode 100644 pkg/state/chartify_kubecontext_test.go diff --git a/pkg/state/chartify_kubecontext_test.go b/pkg/state/chartify_kubecontext_test.go new file mode 100644 index 00000000..e1489912 --- /dev/null +++ b/pkg/state/chartify_kubecontext_test.go @@ -0,0 +1,289 @@ +package state + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/helmfile/helmfile/pkg/environment" + "github.com/helmfile/helmfile/pkg/filesystem" +) + +// TestProcessChartificationKubeContext tests that --kube-context flag is properly +// added to chartifyOpts.TemplateArgs when using jsonPatches with cluster-requiring commands. +// This is a regression test for the issue where helm template does not receive +// --kube-context arg when using kustomize (jsonPatches). +func TestProcessChartificationKubeContext(t *testing.T) { + tests := []struct { + name string + helmfileCommand string + kubeContext string + envKubeContext string + helmDefaults string + expectContext bool + expectDryRun bool + }{ + { + name: "diff command with helmDefaults kubeContext", + helmfileCommand: "diff", + helmDefaults: "minikube", + expectContext: true, + expectDryRun: true, + }, + { + name: "apply command with release kubeContext", + helmfileCommand: "apply", + kubeContext: "prod-cluster", + expectContext: true, + expectDryRun: true, + }, + { + name: "sync command with env kubeContext", + helmfileCommand: "sync", + envKubeContext: "staging-cluster", + expectContext: true, + expectDryRun: true, + }, + { + name: "template command should not add cluster flags", + helmfileCommand: "template", + helmDefaults: "minikube", + expectContext: false, + expectDryRun: false, + }, + { + name: "build command should not add cluster flags", + helmfileCommand: "build", + helmDefaults: "minikube", + expectContext: false, + expectDryRun: false, + }, + { + name: "diff command without kubeContext", + helmfileCommand: "diff", + expectContext: false, + expectDryRun: true, + }, + { + name: "destroy command with kubeContext", + helmfileCommand: "destroy", + helmDefaults: "test-cluster", + expectContext: true, + expectDryRun: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a release with jsonPatches to trigger chartification + release := &ReleaseSpec{ + Name: "test-release", + Namespace: "default", + Chart: "./test-chart", + JSONPatches: []interface{}{ + map[string]interface{}{ + "target": map[string]interface{}{ + "group": "apps", + "version": "v1", + "kind": "Deployment", + "name": "test", + }, + "patch": []interface{}{ + map[string]interface{}{ + "op": "add", + "path": "/spec/template/spec/containers/0/args/-", + "value": "test", + }, + }, + }, + }, + } + + // Set kubeContext on release if provided + if tt.kubeContext != "" { + release.KubeContext = tt.kubeContext + } + + // Create HelmState + state := &HelmState{ + basePath: "/tmp/test", + fs: filesystem.FromFileSystem(filesystem.FileSystem{ + DirectoryExistsAt: func(path string) bool { + return strings.Contains(path, "test-chart") + }, + FileExistsAt: func(path string) bool { + return false + }, + DeleteFile: func(path string) error { + return nil + }, + Glob: func(pattern string) ([]string, error) { + return nil, nil + }, + }), + logger: logger, + valsRuntime: valsRuntime, + RenderedValues: map[string]any{}, + ReleaseSetSpec: ReleaseSetSpec{ + Env: environment.Environment{ + Name: "default", + }, + Environments: map[string]EnvironmentSpec{ + "default": { + KubeContext: tt.envKubeContext, + }, + }, + }, + } + + // Set helmDefaults kubeContext if provided + if tt.helmDefaults != "" { + state.ReleaseSetSpec.HelmDefaults.KubeContext = tt.helmDefaults + } + + // Prepare chartify (this generates the Chartify object with jsonPatches) + chartification, clean, err := state.PrepareChartify(nil, release, "./test-chart", 0) + require.NoError(t, err) + defer clean() + + // Ensure chartification is needed (jsonPatches should trigger it) + require.NotNil(t, chartification, "Chartification should be needed when jsonPatches are present") + + // Process chartification with the test command + opts := ChartPrepareOptions{} + _, _, err = state.processChartification(chartification, release, "./test-chart", opts, false, tt.helmfileCommand) + + // We expect an error because we don't have an actual chart, but we can still + // check that TemplateArgs was set correctly + // The error will come from chartify.Chartify, not from our logic + // So let's check the chartification.Opts.TemplateArgs directly + + if tt.expectContext { + // Determine which kubeContext should be used + expectedContext := tt.kubeContext + if expectedContext == "" { + expectedContext = tt.envKubeContext + } + if expectedContext == "" { + expectedContext = tt.helmDefaults + } + + assert.Contains(t, chartification.Opts.TemplateArgs, "--kube-context", + "TemplateArgs should contain --kube-context flag") + assert.Contains(t, chartification.Opts.TemplateArgs, expectedContext, + "TemplateArgs should contain the expected kube context: %s", expectedContext) + } else if tt.helmDefaults != "" || tt.kubeContext != "" || tt.envKubeContext != "" { + // If a context is configured but not expected (offline commands) + assert.NotContains(t, chartification.Opts.TemplateArgs, "--kube-context", + "TemplateArgs should not contain --kube-context flag for offline commands") + } + + if tt.expectDryRun { + assert.Contains(t, chartification.Opts.TemplateArgs, "--dry-run=server", + "TemplateArgs should contain --dry-run=server flag") + } else { + assert.NotContains(t, chartification.Opts.TemplateArgs, "--dry-run", + "TemplateArgs should not contain --dry-run flag for offline commands") + } + }) + } +} + +// TestProcessChartificationKubeContextPriority tests the priority order +// for kube context selection: release.KubeContext > env.KubeContext > helmDefaults.KubeContext +func TestProcessChartificationKubeContextPriority(t *testing.T) { + tests := []struct { + name string + releaseContext string + envContext string + helmDefaultsContext string + expectedContext string + }{ + { + name: "release context takes priority", + releaseContext: "release-ctx", + envContext: "env-ctx", + helmDefaultsContext: "defaults-ctx", + expectedContext: "release-ctx", + }, + { + name: "env context when release not set", + envContext: "env-ctx", + helmDefaultsContext: "defaults-ctx", + expectedContext: "env-ctx", + }, + { + name: "helmDefaults context when others not set", + helmDefaultsContext: "defaults-ctx", + expectedContext: "defaults-ctx", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + release := &ReleaseSpec{ + Name: "test-release", + Namespace: "default", + Chart: "./test-chart", + JSONPatches: []interface{}{ + map[string]interface{}{ + "target": map[string]interface{}{ + "kind": "Deployment", + }, + }, + }, + } + + if tt.releaseContext != "" { + release.KubeContext = tt.releaseContext + } + + state := &HelmState{ + basePath: "/tmp/test", + fs: filesystem.FromFileSystem(filesystem.FileSystem{ + DirectoryExistsAt: func(path string) bool { + return strings.Contains(path, "test-chart") + }, + FileExistsAt: func(path string) bool { + return false + }, + DeleteFile: func(path string) error { + return nil + }, + Glob: func(pattern string) ([]string, error) { + return nil, nil + }, + }), + logger: logger, + valsRuntime: valsRuntime, + RenderedValues: map[string]any{}, + ReleaseSetSpec: ReleaseSetSpec{ + Env: environment.Environment{ + Name: "default", + }, + Environments: map[string]EnvironmentSpec{ + "default": { + KubeContext: tt.envContext, + }, + }, + HelmDefaults: HelmSpec{ + KubeContext: tt.helmDefaultsContext, + }, + }, + } + + chartification, clean, err := state.PrepareChartify(nil, release, "./test-chart", 0) + require.NoError(t, err) + defer clean() + require.NotNil(t, chartification) + + opts := ChartPrepareOptions{} + _, _, _ = state.processChartification(chartification, release, "./test-chart", opts, false, "diff") + + assert.Contains(t, chartification.Opts.TemplateArgs, tt.expectedContext, + "TemplateArgs should contain the expected context: %s", tt.expectedContext) + }) + } +} diff --git a/pkg/state/state.go b/pkg/state/state.go index 2882617a..dbf2a182 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1421,10 +1421,34 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re // for all cluster-requiring operations (diff, apply, sync, etc.) but not for offline // commands (template, lint, build, etc.) if requiresCluster { - if chartifyOpts.TemplateArgs == "" { - chartifyOpts.TemplateArgs = "--dry-run=server" - } else if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { - chartifyOpts.TemplateArgs += " --dry-run=server" + // Add --kube-context when a kube context is configured + // This ensures chartify's helm template command uses the correct cluster + var kubeContext string + if release.KubeContext != "" { + kubeContext = release.KubeContext + } else if st.Environments[st.Env.Name].KubeContext != "" { + kubeContext = st.Environments[st.Env.Name].KubeContext + } else if st.HelmDefaults.KubeContext != "" { + kubeContext = st.HelmDefaults.KubeContext + } + + if kubeContext != "" { + if chartifyOpts.TemplateArgs == "" { + chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s --dry-run=server", kubeContext) + } else { + if !strings.Contains(chartifyOpts.TemplateArgs, "--kube-context") { + chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s %s", kubeContext, chartifyOpts.TemplateArgs) + } + if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { + chartifyOpts.TemplateArgs += " --dry-run=server" + } + } + } else { + if chartifyOpts.TemplateArgs == "" { + chartifyOpts.TemplateArgs = "--dry-run=server" + } else if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { + chartifyOpts.TemplateArgs += " --dry-run=server" + } } } From e11ef412f5f436dc87c3b3ca01c6194b7d7078dd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 07:55:36 +0000 Subject: [PATCH 3/4] Add shell escaping for kubeContext in TemplateArgs Addressed code review feedback to properly quote the kubeContext value using shellescape.Quote() to prevent potential command injection if the context name contains spaces or special characters. Also added clearer comments explaining the string matching logic for existing flags. Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/state/state.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index dbf2a182..f3cb0887 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -20,6 +20,7 @@ import ( "text/template" "time" + shellescape "al.essio.dev/pkg/shellescape" "dario.cat/mergo" "github.com/Masterminds/semver/v3" "github.com/gofrs/flock" @@ -1433,12 +1434,17 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re } if kubeContext != "" { + // Build the template args with proper quoting for the kubeContext value + // Use shellescape to safely quote the context name in case it contains special characters + quotedContext := shellescape.Quote(kubeContext) if chartifyOpts.TemplateArgs == "" { - chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s --dry-run=server", kubeContext) + chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s --dry-run=server", quotedContext) } else { + // Only add --kube-context if not already present if !strings.Contains(chartifyOpts.TemplateArgs, "--kube-context") { - chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s %s", kubeContext, chartifyOpts.TemplateArgs) + chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s %s", quotedContext, chartifyOpts.TemplateArgs) } + // Add --dry-run if not already present if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { chartifyOpts.TemplateArgs += " --dry-run=server" } From e5ba1743142c5b0b0a0aeb7ed0a4246041860991 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 08:00:25 +0000 Subject: [PATCH 4/4] Improve flag detection logic with word boundary checks Enhanced the flag detection logic to check for word boundaries when detecting existing --kube-context and --dry-run flags. This prevents false positives if a flag contains these strings as substrings. Now checks for: - Flag at start with space after: "--kube-context " - Flag at start with equals: "--kube-context=" - Flag in middle with space before equals: " --kube-context=" Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- go.mod | 2 +- pkg/state/state.go | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index ebc590da..2de281cf 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/helmfile/helmfile go 1.25.4 require ( + al.essio.dev/pkg/shellescape v1.6.0 dario.cat/mergo v1.0.2 github.com/Masterminds/semver/v3 v3.4.0 github.com/Masterminds/sprig/v3 v3.3.0 @@ -110,7 +111,6 @@ require ( ) require ( - al.essio.dev/pkg/shellescape v1.6.0 // indirect cel.dev/expr v0.24.0 // indirect cloud.google.com/go/auth v0.17.0 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect diff --git a/pkg/state/state.go b/pkg/state/state.go index f3cb0887..38b92b22 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1441,18 +1441,26 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s --dry-run=server", quotedContext) } else { // Only add --kube-context if not already present - if !strings.Contains(chartifyOpts.TemplateArgs, "--kube-context") { + // Check for the flag at word boundaries to avoid false matches + if !strings.Contains(chartifyOpts.TemplateArgs, "--kube-context ") && + !strings.HasPrefix(chartifyOpts.TemplateArgs, "--kube-context=") && + !strings.Contains(chartifyOpts.TemplateArgs, " --kube-context=") { chartifyOpts.TemplateArgs = fmt.Sprintf("--kube-context %s %s", quotedContext, chartifyOpts.TemplateArgs) } // Add --dry-run if not already present - if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { + // Check for the flag at word boundaries to avoid false matches + if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run ") && + !strings.HasPrefix(chartifyOpts.TemplateArgs, "--dry-run=") && + !strings.Contains(chartifyOpts.TemplateArgs, " --dry-run=") { chartifyOpts.TemplateArgs += " --dry-run=server" } } } else { if chartifyOpts.TemplateArgs == "" { chartifyOpts.TemplateArgs = "--dry-run=server" - } else if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { + } else if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run ") && + !strings.HasPrefix(chartifyOpts.TemplateArgs, "--dry-run=") && + !strings.Contains(chartifyOpts.TemplateArgs, " --dry-run=") { chartifyOpts.TemplateArgs += " --dry-run=server" } }