From de5a090ca1b6b8e928d7ba8ab1fc90a07a05c142 Mon Sep 17 00:00:00 2001 From: Rishabh Venu Date: Sat, 10 Oct 2026 05:27:16 -0400 Subject: [PATCH 1/3] fix: reuse only user-supplied values when no values flag is given (#464) Without any --set/--values flag, helm-diff fetched the values of the release with `helm get values --all` and passed them to the render command as --values. That output contains the whole values.yaml of the chart the release was installed from, and a --values file takes precedence over the values.yaml of the new chart. Changes to the chart's own values.yaml were therefore missing from the diff. `helm upgrade` only does that with --reuse-values. By default it reuses just the user-supplied values of the release. Fetch the values without --all in that case, and pass no --values at all when the release has no user-supplied values (`helm get values` prints "null" then). The diff for --reuse-values and --reset-then-reuse-values does not change. --- cmd/helm.go | 17 ++++++++++++++--- cmd/main_test.go | 7 ++++++- cmd/upgrade_test.go | 39 +++++++++++++++++++++++++++++++++++---- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/cmd/helm.go b/cmd/helm.go index 4281aa51..e71d1f0a 100644 --- a/cmd/helm.go +++ b/cmd/helm.go @@ -214,7 +214,7 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) { if d.insecureSkipTLSVerify { flags = append(flags, "--insecure-skip-tls-verify") } - // Helm automatically enable --reuse-values when there's no --set, --set-string, --set-json, --set-values, --set-file present. + // Helm automatically reuses the user-supplied values of the last release when there's no --set, --set-string, --set-json, --set-values, --set-file present. // Let's simulate that in helm-diff. // See https://medium.com/@kcatstack/understand-helm-upgrade-flags-reset-values-reuse-values-6e58ac8f127e shouldDefaultReusingValues := len(d.values) == 0 && len(d.stringValues) == 0 && len(d.stringLiteralValues) == 0 && len(d.jsonValues) == 0 && len(d.valueFiles) == 0 && len(d.fileValues) == 0 @@ -240,12 +240,23 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) { } err = d.writeExistingValues(tmpfile, false) } else { - err = d.writeExistingValues(tmpfile, true) + // Only --reuse-values also reuses the values of the chart that the release was installed from. + // By default, Helm reuses just the user-supplied values, so that changes to the values.yaml + // of the chart take effect. See https://github.com/databus23/helm-diff/issues/464 + err = d.writeExistingValues(tmpfile, d.reuseValues) } if err != nil { return nil, err } - flags = append(flags, "--values", tmpfile.Name()) + existingValues, err := os.ReadFile(tmpfile.Name()) + if err != nil { + return nil, err + } + // `helm get values` prints "null" for a release without user-supplied values. + // There is nothing to reuse then. + if v := bytes.TrimSpace(existingValues); len(v) > 0 && !bytes.Equal(v, []byte("null")) { + flags = append(flags, "--values", tmpfile.Name()) + } } for _, value := range d.values { flags = append(flags, "--set", value) diff --git a/cmd/main_test.go b/cmd/main_test.go index 3b4fc58e..5d25b27b 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -32,13 +32,18 @@ func printFakeHelmVersion() { } // printFakeHelmOutput prints the output for a fake helm invocation. -// A `helm version` call prints helm version build info; any other +// A `helm version` call prints helm version build info; a `helm get values` +// call prints HELM_DIFF_FAKE_VALUES_OUTPUT if that is set; any other // invocation prints HELM_DIFF_FAKE_OUTPUT. func printFakeHelmOutput() { if len(os.Args) > 1 && os.Args[1] == "version" { printFakeHelmVersion() return } + if v, ok := os.LookupEnv("HELM_DIFF_FAKE_VALUES_OUTPUT"); ok && len(os.Args) > 2 && os.Args[1] == "get" && os.Args[2] == "values" { + fmt.Print(v) + return + } fmt.Print(os.Getenv("HELM_DIFF_FAKE_OUTPUT")) } diff --git a/cmd/upgrade_test.go b/cmd/upgrade_test.go index e7a79151..d59de7db 100644 --- a/cmd/upgrade_test.go +++ b/cmd/upgrade_test.go @@ -401,35 +401,60 @@ data: fakeHelmMode string args []string useUpgradeDryRun bool + // existingValues is what `helm get values` prints for an existing release. + // Empty means that it prints the manifest, which stands in for some values. + existingValues string // wantGetValues is the expected `helm get values` call. // Empty means that the values must not be fetched at all. wantGetValues string + // wantValues is whether the fetched values must be passed on with --values. + wantValues bool }{ { name: "existing release with --reuse-values", fakeHelmMode: "capture_args", args: []string{"--reuse-values"}, wantGetValues: "get values my-release --output yaml --all --namespace prod-apps\n", + wantValues: true, }, { name: "existing release with --reset-then-reuse-values", fakeHelmMode: "capture_args", args: []string{"--reset-then-reuse-values"}, wantGetValues: "get values my-release --output yaml --namespace prod-apps\n", + wantValues: true, }, { name: "existing release with --install --reuse-values", fakeHelmMode: "capture_args", args: []string{"--install", "--reuse-values"}, wantGetValues: "get values my-release --output yaml --all --namespace prod-apps\n", + wantValues: true, }, { - // Helm enables --reuse-values by default when there is no --set, --set-string, - // --set-literal, --set-json, --values or --set-file flag, so the values of an - // existing release are fetched even without any of the reuse flags. + // Helm reuses the user-supplied values of an existing release when there is no + // --set, --set-string, --set-literal, --set-json, --values or --set-file flag. + // Unlike with --reuse-values, the values of the chart that the release was installed + // from are not reused, so the values are fetched without --all. + // See https://github.com/databus23/helm-diff/issues/464 name: "existing release without value flags", fakeHelmMode: "capture_args", - wantGetValues: "get values my-release --output yaml --all --namespace prod-apps\n", + wantGetValues: "get values my-release --output yaml --namespace prod-apps\n", + wantValues: true, + }, + { + name: "existing release without value flags and HELM_DIFF_USE_UPGRADE_DRY_RUN", + fakeHelmMode: "capture_args", + useUpgradeDryRun: true, + wantGetValues: "get values my-release --output yaml --namespace prod-apps\n", + wantValues: true, + }, + { + // `helm get values` prints "null" for a release without user-supplied values. + name: "existing release without value flags and without user-supplied values", + fakeHelmMode: "capture_args", + existingValues: "null\n", + wantGetValues: "get values my-release --output yaml --namespace prod-apps\n", }, { name: "unreleased with --install", @@ -468,6 +493,9 @@ data: t.Run(tc.name, func(t *testing.T) { argsFile := t.TempDir() + "/args" setupFakeHelm(t, tc.fakeHelmMode, manifestYAML, argsFile, "") + if tc.existingValues != "" { + t.Setenv("HELM_DIFF_FAKE_VALUES_OUTPUT", tc.existingValues) + } renderSubcmd := "template" if tc.useUpgradeDryRun { t.Setenv("HELM_DIFF_USE_UPGRADE_DRY_RUN", "true") @@ -499,6 +527,9 @@ data: } else if !strings.Contains(argsContent, tc.wantGetValues) { t.Errorf("expected 'helm %s', got:\n%s", strings.TrimSpace(tc.wantGetValues), argsContent) } + if strings.Contains(argsContent, " --values ") != tc.wantValues { + t.Errorf("expected the existing values to be passed with --values: %t, got:\n%s", tc.wantValues, argsContent) + } }) } } From df7c60ed9192c8f478b492f85b6855f66c6db8a8 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 18 Oct 2026 08:28:31 +0800 Subject: [PATCH 2/3] test: cover the full value-flag matrix and add issue #464 reproduction Complete the review of #1087 ("reuse only user-supplied values when no values flag is given"): - Fill the remaining gaps of the flag matrix in TestUpgradeCommand_Execution_ReuseValues: an existing release with --set or --reset-values must not fetch the release values at all (Helm reuses them only when no new values are given), --reuse-values with --set must keep fetching with --all, and --reset-then-reuse-values on a release without user-supplied values must skip --values like the default path. The last case fails on master; the others pin behavior the fix must not regress. - Pin the new HELM_DIFF_FAKE_VALUES_OUTPUT branch of the fake helm, including the set-but-empty case that os.LookupEnv makes possible. - Add scripts/issues/464.sh, the end-to-end reproduction of #464 for the CI integration job (scripts/issues/run.sh picks it up automatically): a chart default change must be visible in the default diff, a user-supplied value must win over a changed default, --reuse-values keeps the old chart values, and --set replaces the release values. The unit tests cannot assert rendered values because the fake helm echoes a fixed manifest. Signed-off-by: yxxhero --- cmd/main_test.go | 22 ++++++++ cmd/upgrade_test.go | 32 +++++++++++ scripts/issues/464.sh | 128 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+) create mode 100755 scripts/issues/464.sh diff --git a/cmd/main_test.go b/cmd/main_test.go index 5d25b27b..56e9d454 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -134,4 +134,26 @@ func TestFakeHelmVersionOutput(t *testing.T) { require.NoError(t, err) require.Equal(t, "manifest-output", string(out)) }) + + t.Run("get values prints HELM_DIFF_FAKE_VALUES_OUTPUT when set", func(t *testing.T) { + t.Setenv("HELM_DIFF_FAKE_HELM", "1") + t.Setenv("HELM_DIFF_FAKE_HELM_MODE", "default") + t.Setenv("HELM_DIFF_FAKE_VALUES_OUTPUT", "user: value\n") + t.Setenv("HELM_DIFF_FAKE_OUTPUT", "manifest-output") + + out, err := exec.Command(exe, "get", "values", "my-release").CombinedOutput() + require.NoError(t, err) + require.Equal(t, "user: value\n", string(out)) + }) + + t.Run("get values prints empty output when HELM_DIFF_FAKE_VALUES_OUTPUT is set but empty", func(t *testing.T) { + t.Setenv("HELM_DIFF_FAKE_HELM", "1") + t.Setenv("HELM_DIFF_FAKE_HELM_MODE", "default") + t.Setenv("HELM_DIFF_FAKE_VALUES_OUTPUT", "") + t.Setenv("HELM_DIFF_FAKE_OUTPUT", "manifest-output") + + out, err := exec.Command(exe, "get", "values", "my-release").CombinedOutput() + require.NoError(t, err) + require.Empty(t, out) + }) } diff --git a/cmd/upgrade_test.go b/cmd/upgrade_test.go index d59de7db..d948c4f8 100644 --- a/cmd/upgrade_test.go +++ b/cmd/upgrade_test.go @@ -456,6 +456,38 @@ data: existingValues: "null\n", wantGetValues: "get values my-release --output yaml --namespace prod-apps\n", }, + { + // Helm reuses the values of an existing release only when no new values are + // given, so with --set there is nothing to fetch and the release values are dropped. + name: "existing release with --set", + fakeHelmMode: "capture_args", + args: []string{"--set", "key=val"}, + }, + { + // --reset-values resets to the values built into the chart, so the values + // of the release are not fetched at all. + name: "existing release with --reset-values", + fakeHelmMode: "capture_args", + args: []string{"--reset-values"}, + }, + { + // An explicit --reuse-values also reuses the values of the chart that the + // release was installed from, even when --set is given on top. + name: "existing release with --reuse-values and --set", + fakeHelmMode: "capture_args", + args: []string{"--reuse-values", "--set", "key=val"}, + wantGetValues: "get values my-release --output yaml --all --namespace prod-apps\n", + wantValues: true, + }, + { + // `helm get values` prints "null" for a release without user-supplied values, + // so --reset-then-reuse-values has nothing to pass on with --values either. + name: "existing release with --reset-then-reuse-values and without user-supplied values", + fakeHelmMode: "capture_args", + args: []string{"--reset-then-reuse-values"}, + existingValues: "null\n", + wantGetValues: "get values my-release --output yaml --namespace prod-apps\n", + }, { name: "unreleased with --install", fakeHelmMode: "unreleased", diff --git a/scripts/issues/464.sh b/scripts/issues/464.sh new file mode 100755 index 00000000..3dcab456 --- /dev/null +++ b/scripts/issues/464.sh @@ -0,0 +1,128 @@ +#!/usr/bin/env bash +# Reproduction test for https://github.com/databus23/helm-diff/issues/464 +# +# Bug: "Diff is incorrect when values in values.yaml change and using local Chart". +# +# When no --set/--values flag is given, helm-diff fetched the values of the +# release with `helm get values --all`, i.e. the fully coalesced values +# including the values.yaml of the chart the release was installed from, +# and passed them to the render command as --values. A --values file takes +# precedence over the new chart's values.yaml, so every change to a chart +# default was masked and the diff came out empty. +# +# Helm's own default only reuses the user-supplied values of the release +# (current.Config), never the coalesced ones, so chart default changes do +# apply. `helm upgrade --reuse-values` is the flag that keeps the old chart +# values. +# +# Scenarios (each prints its full diff output to the CI log): +# A. chart default change, release without user-supplied values: +# the change is visible in the diff +# B. same release with --reuse-values: the old chart values are kept, +# so the change is not visible (unchanged --reuse-values semantics) +# C. chart default change, release with a user-supplied value: +# the user value wins, no diff +# D. same release with --set: the release values are not reused, +# the --set value replaces the user value in the diff + +set -euo pipefail + +ISSUE=464 +# shellcheck source=lib.sh +source "$(dirname "$0")/lib.sh" + +# expect +expect() { + local scenario="$1" desc="$2" pattern="$3" want="$4" got + local out="$WORK/${scenario// /_}.out" + strip_ansi < "$out" > "$WORK/stripped.out" + if grep -qE "$pattern" "$WORK/stripped.out"; then + got=present + else + got=absent + fi + if [ "$got" = "$want" ]; then + echo "OK: [$scenario] $desc is $want" + else + echo "FAIL: [$scenario] expected $desc to be $want (#${ISSUE})" + FAIL=1 + fi +} + +# new_chart writes a chart with a single ConfigMap whose +# only value comes from the chart's values.yaml. +new_chart() { + mkdir -p "$1/templates" + cat > "$1/Chart.yaml" <<'YAML' +apiVersion: v2 +name: issue-test +version: 0.1.0 +YAML + printf 'key: %s\n' "$2" > "$1/values.yaml" + cat > "$1/templates/cm.yaml" <<'YAML' +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm +data: + key: {{ .Values.key | quote }} +YAML +} + +kubectl create namespace "$NS" 2>/dev/null || true + +new_chart "$WORK/chart" old +# rel has no user-supplied values, rel2 has one that shadows the default. +helm upgrade -i rel "$WORK/chart" -n "$NS" >/dev/null +helm upgrade -i rel2 "$WORK/chart" -n "$NS" --set key=user >/dev/null + +# check guards a setup assumption and fails the +# test on mismatch. +check() { + local desc="$1" got="$2" want="$3" + if [ "$got" = "$want" ]; then + echo "OK: $desc" + else + echo "FAIL: $desc, got: $got, want: $want (#${ISSUE})" + # shellcheck disable=SC2034 # FAIL is read by finish() in lib.sh + FAIL=1 + fi +} + +# `helm get values` must print exactly what the scenarios below rely on. +check "rel has no user-supplied values" "$(helm get values rel -n "$NS" --output yaml)" "null" +check "rel2 has key=user as its only user-supplied value" "$(helm get values rel2 -n "$NS" --output yaml)" "key: user" + +# The only change from here on is the chart's default in values.yaml. +printf 'key: new\n' > "$WORK/chart/values.yaml" + +############################################################################### +# Variant A: the chart default change must be visible in the default diff +############################################################################### +run_diff "A default" noassert rel "$WORK/chart" -n "$NS" +expect "A default" "the new chart default (key: \"new\")" '^\+[[:space:]]*key: "new"' present +expect "A default" "the old chart default (key: \"old\")" '^-[[:space:]]*key: "old"' present + +############################################################################### +# Variant B: --reuse-values keeps the values of the chart the release was +# installed from, so the change stays invisible (as in plain Helm) +############################################################################### +run_diff "B reuse-values" noassert rel "$WORK/chart" -n "$NS" --reuse-values +expect "B reuse-values" "no key diff (--reuse-values keeps the old chart values)" '^[+-][[:space:]]*key:' absent + +############################################################################### +# Variant C: a user-supplied value shadows the default, so the default change +# does not affect the diff +############################################################################### +run_diff "C user-value" noassert rel2 "$WORK/chart" -n "$NS" +expect "C user-value" "no key diff (the user-supplied value wins)" '^[+-][[:space:]]*key:' absent + +############################################################################### +# Variant D: with --set, Helm does not reuse the values of the release, +# so the --set value replaces the user value +############################################################################### +run_diff "D set-override" noassert rel2 "$WORK/chart" -n "$NS" --set key=override +expect "D set-override" "the old user value (key: \"user\")" '^-[[:space:]]*key: "user"' present +expect "D set-override" "the new value (key: \"override\")" '^\+[[:space:]]*key: "override"' present + +finish From 5f8cda456fd05d39e66e390f8982d7f4c878f9ad Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 18 Oct 2026 08:51:30 +0800 Subject: [PATCH 3/3] fix: keep the issue 464 releases from owning the same ConfigMap The reproduction installs two releases of the same chart into one namespace, which made both render the same ConfigMap cm. Helm 3.18+ and v4 refuse to import a resource another release owns, so the rel2 install aborted with an ownership validation error before any scenario ran. Name the ConfigMap after the release so each release owns its own object; the scenario assertions only match data lines and do not care. Signed-off-by: yxxhero --- scripts/issues/464.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/issues/464.sh b/scripts/issues/464.sh index 3dcab456..fd27429c 100755 --- a/scripts/issues/464.sh +++ b/scripts/issues/464.sh @@ -50,7 +50,9 @@ expect() { } # new_chart writes a chart with a single ConfigMap whose -# only value comes from the chart's values.yaml. +# only value comes from the chart's values.yaml. The ConfigMap is named +# after the release so the two releases below can coexist in one namespace: +# helm (3.18+ and v4) refuses to import a resource another release owns. new_chart() { mkdir -p "$1/templates" cat > "$1/Chart.yaml" <<'YAML' @@ -63,7 +65,7 @@ YAML apiVersion: v1 kind: ConfigMap metadata: - name: cm + name: cm-{{ .Release.Name }} data: key: {{ .Values.key | quote }} YAML