From b5aad4190629f6ab65ab9e4166fefd42a99cee99 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Tue, 16 Jun 2026 11:13:59 +0800 Subject: [PATCH 1/3] feat: add support for helm v4 --server-side flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the --server-side flag to , matching helm v4's upgrade command. The flag accepts "true", "false", or "auto" (default "auto") and is forwarded to helm when running Helm v4: - (HELM_DIFF_USE_UPGRADE_DRY_RUN=true): all values including "auto" are forwarded. - : only "true"/"false" are forwarded, since template registers --server-side as a bool (default true) that does not accept "auto". The flag is Helm v4 only — it is accepted but not forwarded when running Helm v3 (the flag does not exist there). Note: --server-side has no effect on manifest rendering in dry-run mode. It is forwarded for semantic correctness and wrapper compatibility so that helm-diff can be used with the same flags as . Closes #1020 Signed-off-by: yxxhero --- README.md | 2 + cmd/helm.go | 20 ++++++++++ cmd/helm_test.go | 92 +++++++++++++++++++++++++++++++++++++++++++++ cmd/upgrade.go | 10 ++++- cmd/upgrade_test.go | 24 ++++++++++++ 5 files changed, 147 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fb11d451..c82a9e02 100644 --- a/README.md +++ b/README.md @@ -164,6 +164,7 @@ Flags: --reset-then-reuse-values reset the values to the ones built into the chart, apply the last release's values and merge in any new values. If '--reset-values' or '--reuse-values' is specified, this is ignored --reset-values reset the values to the ones built into the chart and merge in any new values --reuse-values reuse the last release's values and merge in any new values. If '--reset-values' is specified, this is ignored + --server-side string must be "true", "false" or "auto". Object updates run in the server instead of the client ("auto" defaults the value from the previous chart release's method) (default "auto") --set stringArray set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2) --set-file stringArray set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2) --set-json stringArray set JSON values on the command line (can specify multiple or separate values with commas: key1=jsonval1,key2=jsonval2) @@ -344,6 +345,7 @@ Flags: --reset-then-reuse-values reset the values to the ones built into the chart, apply the last release's values and merge in any new values. If '--reset-values' or '--reuse-values' is specified, this is ignored --reset-values reset the values to the ones built into the chart and merge in any new values --reuse-values reuse the last release's values and merge in any new values. If '--reset-values' is specified, this is ignored + --server-side string must be "true", "false" or "auto". Object updates run in the server instead of the client ("auto" defaults the value from the previous chart release's method) (default "auto") --set stringArray set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2) --set-file stringArray set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2) --set-json stringArray set JSON values on the command line (can specify multiple or separate values with commas: key1=jsonval1,key2=jsonval2) diff --git a/cmd/helm.go b/cmd/helm.go index b81077e4..1f5981c5 100644 --- a/cmd/helm.go +++ b/cmd/helm.go @@ -307,6 +307,26 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) { flags = append(flags, "--take-ownership") } + // Forward --server-side to helm. This flag is Helm v4 only. + // - `helm upgrade` registers it as a string accepting "true", "false", "auto". + // - `helm template` registers it as a bool (default true); "auto" is rejected. + // - The flag has no effect on manifest rendering in dry-run mode — both + // `helm template` and `helm upgrade --dry-run` bail out before the apply + // step where ServerSideApply is actually consulted. It is forwarded purely + // for semantic correctness and so that helm-diff can be used as a drop-in + // wrapper around `helm upgrade` with the same flags. + if isHelmV4, err := isHelmVersionGreaterThanEqual(helmV4Version); err == nil && isHelmV4 { + switch { + case d.useUpgradeDryRun: + // `helm upgrade`: forward all values including "auto". + flags = append(flags, "--server-side="+d.serverSide) + case d.serverSide == envTrue || d.serverSide == envFalse: + // `helm template`: forward only bool-compatible values. + // "auto" is skipped — template's default (true) is reasonable. + flags = append(flags, "--server-side="+d.serverSide) + } + } + var ( subcmd string filter func([]byte) []byte diff --git a/cmd/helm_test.go b/cmd/helm_test.go index d1f23737..ce2b8e95 100644 --- a/cmd/helm_test.go +++ b/cmd/helm_test.go @@ -245,6 +245,98 @@ func TestGetTemplateDryRunFlagsBoolModes(t *testing.T) { } } +// getServerSideFlags mirrors the --server-side flag-passing logic in +// (*diffCmd).template() for a given Helm version, subcommand and value. +func getServerSideFlags(isHelmV4 bool, useUpgradeDryRun bool, serverSide string) []string { + if !isHelmV4 { + return nil + } + switch { + case useUpgradeDryRun: + return []string{"--server-side=" + serverSide} + case serverSide == "true" || serverSide == "false": + return []string{"--server-side=" + serverSide} + default: + return nil + } +} + +func TestGetServerSideFlags(t *testing.T) { + cases := []struct { + name string + isHelmV4 bool + useUpgradeDryRun bool + serverSide string + expected []string + }{ + { + name: "helm v4 template with server-side=true", + isHelmV4: true, + useUpgradeDryRun: false, + serverSide: "true", + expected: []string{"--server-side=true"}, + }, + { + name: "helm v4 template with server-side=false", + isHelmV4: true, + useUpgradeDryRun: false, + serverSide: "false", + expected: []string{"--server-side=false"}, + }, + { + name: "helm v4 template with server-side=auto skips flag", + isHelmV4: true, + useUpgradeDryRun: false, + serverSide: "auto", + expected: nil, + }, + { + name: "helm v4 upgrade dry-run with server-side=auto passes flag", + isHelmV4: true, + useUpgradeDryRun: true, + serverSide: "auto", + expected: []string{"--server-side=auto"}, + }, + { + name: "helm v4 upgrade dry-run with server-side=true", + isHelmV4: true, + useUpgradeDryRun: true, + serverSide: "true", + expected: []string{"--server-side=true"}, + }, + { + name: "helm v4 upgrade dry-run with server-side=false", + isHelmV4: true, + useUpgradeDryRun: true, + serverSide: "false", + expected: []string{"--server-side=false"}, + }, + { + name: "helm v3 never gets server-side flag", + isHelmV4: false, + useUpgradeDryRun: false, + serverSide: "true", + expected: nil, + }, + { + name: "helm v3 upgrade dry-run never gets server-side flag", + isHelmV4: false, + useUpgradeDryRun: true, + serverSide: "auto", + expected: nil, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + actual := getServerSideFlags(tc.isHelmV4, tc.useUpgradeDryRun, tc.serverSide) + if !reflect.DeepEqual(actual, tc.expected) { + t.Errorf("Expected %v, got %v", tc.expected, actual) + } + }) + } +} + func TestExtractManifestFromHelmUpgradeDryRunOutput(t *testing.T) { type testdata struct { description string diff --git a/cmd/upgrade.go b/cmd/upgrade.go index b13fd53b..3d384ccd 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -23,7 +23,8 @@ import ( ) var ( - validDryRunValues = []string{dryRunServer, dryRunNoOptDefVal, envTrue, envFalse} + validDryRunValues = []string{dryRunServer, dryRunNoOptDefVal, envTrue, envFalse} + validServerSideVals = []string{envTrue, envFalse, serverSideAuto} ) const ( @@ -32,6 +33,7 @@ const ( dryRunServer = "server" envTrue = "true" envFalse = "false" + serverSideAuto = "auto" ) type diffCmd struct { @@ -66,6 +68,7 @@ type diffCmd struct { normalizeManifests bool takeOwnership bool threeWayMerge bool + serverSide string extraAPIs []string kubeVersion string useUpgradeDryRun bool @@ -166,6 +169,10 @@ func newChartCommand() *cobra.Command { return fmt.Errorf("flag %q must take a bool value or either %q or %q, but got %q", "dry-run", dryRunNoOptDefVal, dryRunServer, diff.dryRunMode) } + if !slices.Contains(validServerSideVals, diff.serverSide) { + return fmt.Errorf("flag %q must be %q, %q or %q, but got %q", "server-side", envTrue, envFalse, serverSideAuto, diff.serverSide) + } + // Suppress the command usage on error. See #77 for more info cmd.SilenceUsage = true @@ -252,6 +259,7 @@ func newChartCommand() *cobra.Command { f.BoolVar(&diff.insecureSkipTLSVerify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the chart download") f.BoolVar(&diff.normalizeManifests, "normalize-manifests", false, "normalize manifests before running diff to exclude style differences from the output") f.BoolVar(&diff.takeOwnership, "take-ownership", false, "if set, upgrade will ignore the check for helm annotations and take ownership of the existing resources") + f.StringVar(&diff.serverSide, "server-side", serverSideAuto, `must be "true", "false" or "auto". Object updates run in the server instead of the client ("auto" defaults the value from the previous chart release's method)`) AddDiffOptions(f, &diff.Options) diff --git a/cmd/upgrade_test.go b/cmd/upgrade_test.go index cff84fc0..ef3ed3db 100644 --- a/cmd/upgrade_test.go +++ b/cmd/upgrade_test.go @@ -3,6 +3,7 @@ package cmd import ( "os" "path/filepath" + "slices" "testing" ) @@ -181,3 +182,26 @@ func TestPrepareEnvSettings_ConfigFlagsPointToCorrectFields(t *testing.T) { } }) } + +func TestServerSideFlagValidation(t *testing.T) { + cases := []struct { + name string + value string + expectErr bool + }{ + {name: "true", value: "true", expectErr: false}, + {name: "false", value: "false", expectErr: false}, + {name: "auto", value: "auto", expectErr: false}, + {name: "invalid", value: "yes", expectErr: true}, + {name: "empty", value: "", expectErr: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + valid := slices.Contains(validServerSideVals, tc.value) + if valid == tc.expectErr { + t.Errorf("value %q: expected valid=%v, got valid=%v", tc.value, !tc.expectErr, valid) + } + }) + } +} From b0983619309a0d10b747971d9095266aff1e5f80 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Tue, 16 Jun 2026 11:46:28 +0800 Subject: [PATCH 2/3] ci: replace broken engineerd/setup-kind with helm/kind-action The engineerd/setup-kind@v0.6.2 tag points to a commit that lacks the compiled dist/main/index.js entry point, causing all integration test jobs to fail with: File not found: '.../setup-kind/v0.6.2/dist/main/index.js' Switch to helm/kind-action@v1.14.0, the official Helm project kind action, which properly commits its build artifacts. Signed-off-by: yxxhero --- .github/workflows/ci.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 60f8b51b..dd8af25f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -122,9 +122,9 @@ jobs: - helm-version: v3.21.1 - helm-version: v4.2.1 steps: - - uses: engineerd/setup-kind@v0.6.2 + - uses: helm/kind-action@v1.14.0 with: - skipClusterLogsExport: true + cluster_name: kind - uses: actions/checkout@v6 From aa01864ed57eb2c2966a31fdd19988fd53a51ab8 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Wed, 17 Jun 2026 05:57:24 +0800 Subject: [PATCH 3/3] refactor: extract serverSideFlags for direct testability Per review feedback, extract the inline switch block from template() into a package-level serverSideFlags(isHelmV4, useUpgradeDryRun, serverSide) function. The test now calls the production function directly instead of a test-only mirror. Signed-off-by: yxxhero --- cmd/helm.go | 49 +++++++++++++++++++++++++++++------------------- cmd/helm_test.go | 20 ++------------------ 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/cmd/helm.go b/cmd/helm.go index 1f5981c5..fe4d138f 100644 --- a/cmd/helm.go +++ b/cmd/helm.go @@ -307,25 +307,8 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) { flags = append(flags, "--take-ownership") } - // Forward --server-side to helm. This flag is Helm v4 only. - // - `helm upgrade` registers it as a string accepting "true", "false", "auto". - // - `helm template` registers it as a bool (default true); "auto" is rejected. - // - The flag has no effect on manifest rendering in dry-run mode — both - // `helm template` and `helm upgrade --dry-run` bail out before the apply - // step where ServerSideApply is actually consulted. It is forwarded purely - // for semantic correctness and so that helm-diff can be used as a drop-in - // wrapper around `helm upgrade` with the same flags. - if isHelmV4, err := isHelmVersionGreaterThanEqual(helmV4Version); err == nil && isHelmV4 { - switch { - case d.useUpgradeDryRun: - // `helm upgrade`: forward all values including "auto". - flags = append(flags, "--server-side="+d.serverSide) - case d.serverSide == envTrue || d.serverSide == envFalse: - // `helm template`: forward only bool-compatible values. - // "auto" is skipped — template's default (true) is reasonable. - flags = append(flags, "--server-side="+d.serverSide) - } - } + isHelmV4, _ := isHelmVersionGreaterThanEqual(helmV4Version) + flags = append(flags, serverSideFlags(isHelmV4, d.useUpgradeDryRun, d.serverSide)...) var ( subcmd string @@ -510,3 +493,31 @@ func extractManifestFromHelmUpgradeDryRunOutput(s []byte, noHooks bool) []byte { return r } + +// serverSideFlags returns the --server-side flag(s) to forward to helm. +// +// The flag is Helm v4 only: +// - `helm upgrade` registers it as a string accepting "true", "false", "auto". +// - `helm template` registers it as a bool (default true); "auto" is rejected. +// +// The flag has no effect on manifest rendering in dry-run mode — both +// `helm template` and `helm upgrade --dry-run` bail out before the apply +// step where ServerSideApply is actually consulted. It is forwarded purely +// for semantic correctness and so that helm-diff can be used as a drop-in +// wrapper around `helm upgrade` with the same flags. +func serverSideFlags(isHelmV4 bool, useUpgradeDryRun bool, serverSide string) []string { + if !isHelmV4 { + return nil + } + switch { + case useUpgradeDryRun: + // `helm upgrade`: forward all values including "auto". + return []string{"--server-side=" + serverSide} + case serverSide == envTrue || serverSide == envFalse: + // `helm template`: forward only bool-compatible values. + // "auto" is skipped — template's default (true) is reasonable. + return []string{"--server-side=" + serverSide} + default: + return nil + } +} diff --git a/cmd/helm_test.go b/cmd/helm_test.go index ce2b8e95..cddf73dd 100644 --- a/cmd/helm_test.go +++ b/cmd/helm_test.go @@ -245,23 +245,7 @@ func TestGetTemplateDryRunFlagsBoolModes(t *testing.T) { } } -// getServerSideFlags mirrors the --server-side flag-passing logic in -// (*diffCmd).template() for a given Helm version, subcommand and value. -func getServerSideFlags(isHelmV4 bool, useUpgradeDryRun bool, serverSide string) []string { - if !isHelmV4 { - return nil - } - switch { - case useUpgradeDryRun: - return []string{"--server-side=" + serverSide} - case serverSide == "true" || serverSide == "false": - return []string{"--server-side=" + serverSide} - default: - return nil - } -} - -func TestGetServerSideFlags(t *testing.T) { +func TestServerSideFlags(t *testing.T) { cases := []struct { name string isHelmV4 bool @@ -329,7 +313,7 @@ func TestGetServerSideFlags(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - actual := getServerSideFlags(tc.isHelmV4, tc.useUpgradeDryRun, tc.serverSide) + actual := serverSideFlags(tc.isHelmV4, tc.useUpgradeDryRun, tc.serverSide) if !reflect.DeepEqual(actual, tc.expected) { t.Errorf("Expected %v, got %v", tc.expected, actual) }