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 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..fe4d138f 100644 --- a/cmd/helm.go +++ b/cmd/helm.go @@ -307,6 +307,9 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) { flags = append(flags, "--take-ownership") } + isHelmV4, _ := isHelmVersionGreaterThanEqual(helmV4Version) + flags = append(flags, serverSideFlags(isHelmV4, d.useUpgradeDryRun, d.serverSide)...) + var ( subcmd string filter func([]byte) []byte @@ -490,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 d1f23737..cddf73dd 100644 --- a/cmd/helm_test.go +++ b/cmd/helm_test.go @@ -245,6 +245,82 @@ func TestGetTemplateDryRunFlagsBoolModes(t *testing.T) { } } +func TestServerSideFlags(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 := serverSideFlags(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) + } + }) + } +}