Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 37 additions & 38 deletions cmd/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
"os/exec"
"regexp"
"slices"
"strconv"
"strings"

Expand Down Expand Up @@ -331,8 +332,14 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) {
if !d.disableValidation && d.clusterAccessAllowed() {
isHelmV4, err := isHelmVersionGreaterThanEqual(helmV4Version)
if err == nil && isHelmV4 {
// Flag --validate has been deprecated, use '--dry-run=server' instead in Helm v4+
flags = append(flags, "--dry-run=server")
// For Helm v4, we use --dry-run=server by default to get correct .Capabilities.APIVersions.
// This is only applied if the user hasn't explicitly set --dry-run=client, --dry-run=true, or --dry-run=false.
// Note: dryRunMode="true" behaves like "client" (no cluster access).
// Note: dryRunMode="false" behaves like "none" (no dry-run flag at all).
// See https://github.com/databus23/helm-diff/issues/894
if !slices.Contains([]string{"client", "true", "false"}, d.dryRunMode) {
flags = append(flags, "--dry-run=server")
}
} else {
flags = append(flags, "--validate")
}
Expand All @@ -353,42 +360,34 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) {
// To keep the full compatibility with older helm-diff versions,
// we pass --dry-run to `helm template` only if Helm is greater than v3.13.0.
if useDryRunService, err := isHelmVersionAtLeast(minHelmVersionWithDryRunLookupSupport); err == nil && useDryRunService {
// However, which dry-run mode to use is still not clear.
//
// For compatibility with the old and new helm-diff options,
// old and new helm, we assume that the user wants to use the older `helm template --dry-run=client` mode
// if helm-diff has been invoked with any of the following flags:
//
// * no dry-run flags (to be consistent with helm-template)
// * --dry-run
// * --dry-run=""
// * --dry-run=client
//
// and the newer `helm template --dry-run=server` mode when invoked with:
//
// * --dry-run=server
//
// Any other values should result in errors.
//
// See the fllowing link for more details:
// - https://github.com/databus23/helm-diff/pull/458
// - https://github.com/helm/helm/pull/9426#issuecomment-1501005666
if d.dryRunMode == "server" {
// This is for security reasons!
//
// We give helm-template the additional cluster access for the helm `lookup` function
// only if the user has explicitly requested it by --dry-run=server,
//
// In other words, although helm-diff-upgrade implies limited cluster access by default,
// helm-diff-upgrade without a --dry-run flag does NOT imply
// full cluster-access via helm-template --dry-run=server!
flags = append(flags, "--dry-run=server")
} else {
// Since helm-diff 3.9.0 and helm 3.13.0, we pass --dry-run=client to `helm template` by default.
// This doesn't make any difference for helm-diff itself,
// because helm-template w/o flags is equivalent to helm-template --dry-run=client.
// See https://github.com/helm/helm/pull/9426#discussion_r1181397259
flags = append(flags, "--dry-run=client")
isHelmV4, _ := isHelmVersionGreaterThanEqual(helmV4Version)

// For Helm v4, --dry-run=server may already have been added above when
// clusterAccessAllowed() is true and d.dryRunMode is not "client", "true", or "false".
// In that case (Helm v4 and d.dryRunMode not "client"/"true"/"false"), we skip adding any
// additional dry-run flag here. In all other cases (Helm v3 or d.dryRunMode is "client"/"true"),
// we add the appropriate dry-run mode below.
// Note: dryRunMode="false" means no dry-run flag at all.
if d.dryRunMode == "false" {
// "false" means no dry-run, skip adding any dry-run flag
} else if !(isHelmV4 && !slices.Contains([]string{"client", "true"}, d.dryRunMode)) {
if d.dryRunMode == "server" {
// This is for security reasons!
//
// We give helm-template the additional cluster access for the helm `lookup` function
// only if the user has explicitly requested it by --dry-run=server,
//
// In other words, although helm-diff-upgrade implies limited cluster access by default,
// helm-diff-upgrade without a --dry-run flag does NOT imply
// full cluster-access via helm-template --dry-run=server!
flags = append(flags, "--dry-run=server")
} else {
// Since helm-diff 3.9.0 and helm 3.13.0, we pass --dry-run=client to `helm template` by default.
// This doesn't make any difference for helm-diff itself,
// because helm-template w/o flags is equivalent to helm-template --dry-run=client.
// See https://github.com/helm/helm/pull/9426#discussion_r1181397259
flags = append(flags, "--dry-run=client")
}
}
}

Expand Down
239 changes: 239 additions & 0 deletions cmd/helm_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,250 @@
package cmd

import (
"reflect"
"slices"
"testing"

"github.com/google/go-cmp/cmp"
)

type dryRunFlagsConfig struct {
isHelmV4 bool
supportsDryRunLookup bool
clusterAccessAllowed bool
disableValidation bool
dryRunMode string
}

func getTemplateDryRunFlags(cfg dryRunFlagsConfig) []string {
var flags []string

if !cfg.disableValidation && cfg.clusterAccessAllowed {
if cfg.isHelmV4 {
if !slices.Contains([]string{"client", "true", "false"}, cfg.dryRunMode) {
flags = append(flags, "--dry-run=server")
}
} else {
flags = append(flags, "--validate")
}
}

if cfg.supportsDryRunLookup {
if cfg.dryRunMode == "false" {
// "false" means no dry-run, skip adding any dry-run flag
} else if !(cfg.isHelmV4 && !slices.Contains([]string{"client", "true"}, cfg.dryRunMode)) {
if cfg.dryRunMode == "server" {
flags = append(flags, "--dry-run=server")
} else {
flags = append(flags, "--dry-run=client")
}
}
}

return flags
}

func TestGetTemplateDryRunFlags(t *testing.T) {
cases := []struct {
name string
config dryRunFlagsConfig
expected []string
}{
{
name: "Helm v4 with no explicit dry-run flag uses server mode",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "none",
},
expected: []string{"--dry-run=server"},
},
{
name: "Helm v4 with dry-run=client uses client mode",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: false,
disableValidation: false,
dryRunMode: "client",
},
expected: []string{"--dry-run=client"},
},
{
name: "Helm v4 with dry-run=server uses server mode",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "server",
},
expected: []string{"--dry-run=server"},
},
{
name: "Helm v4 with validation disabled and dry-run=none skips dry-run flags",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: true,
dryRunMode: "none",
},
expected: nil,
},
{
name: "Helm v4 with validation disabled and dry-run=client uses client mode",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: true,
dryRunMode: "client",
},
expected: []string{"--dry-run=client"},
},
{
name: "Helm v3 with no explicit dry-run flag uses validate and client",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "none",
},
expected: []string{"--validate", "--dry-run=client"},
},
{
name: "Helm v3 with dry-run=server uses server mode",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "server",
},
expected: []string{"--validate", "--dry-run=server"},
},
{
name: "Helm v3 with dry-run=client uses client mode",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: true,
clusterAccessAllowed: false,
disableValidation: false,
dryRunMode: "client",
},
expected: []string{"--dry-run=client"},
},
{
name: "Helm v3 without dry-run lookup support uses only validate",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: false,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "none",
},
expected: []string{"--validate"},
},
{
name: "Helm v4 without dry-run lookup support uses server mode",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: false,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "none",
},
expected: []string{"--dry-run=server"},
},
{
name: "Helm v4 with empty dry-run mode uses server mode",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "",
},
expected: []string{"--dry-run=server"},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
actual := getTemplateDryRunFlags(tc.config)
if !reflect.DeepEqual(actual, tc.expected) {
t.Errorf("Expected %v, got %v", tc.expected, actual)
}
})
}
}

func TestGetTemplateDryRunFlagsBoolModes(t *testing.T) {
cases := []struct {
name string
config dryRunFlagsConfig
expected []string
}{
{
name: "Helm v3 dryRunMode=true behaves like client",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "true",
},
expected: []string{"--validate", "--dry-run=client"},
},
{
name: "Helm v3 dryRunMode=false behaves like none",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "false",
},
expected: []string{"--validate"},
},
{
name: "Helm v4 dryRunMode=true behaves like client",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: false,
disableValidation: false,
dryRunMode: "true",
},
expected: []string{"--dry-run=client"},
},
{
name: "Helm v4 dryRunMode=false behaves like none",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "false",
},
expected: nil,
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
actual := getTemplateDryRunFlags(tc.config)
if !reflect.DeepEqual(actual, tc.expected) {
t.Errorf("Expected %v, got %v", tc.expected, actual)
}
})
}
}

Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test suite is missing coverage for dryRunMode="true" and dryRunMode="false" cases. According to cmd/upgrade.go, these are valid values that users can pass (line 25: validDryRunValues includes "true" and "false"). The behavior is documented as: "true" should behave like "client" and "false" should behave like "none". Test cases should be added to verify that these values are handled correctly for both Helm v3 and Helm v4, especially since the current implementation has a bug where dryRunMode="true" results in no dry-run flags being added for Helm v4.

Suggested change
func TestGetTemplateDryRunFlagsBoolModes(t *testing.T) {
cases := []struct {
name string
config dryRunFlagsConfig
expected []string
}{
{
name: "Helm v3 dryRunMode=true behaves like client",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "true",
},
expected: []string{"--validate", "--dry-run=client"},
},
{
name: "Helm v3 dryRunMode=false behaves like none",
config: dryRunFlagsConfig{
isHelmV4: false,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "false",
},
expected: []string{"--validate"},
},
{
name: "Helm v4 dryRunMode=true behaves like client",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "true",
},
expected: []string{"--dry-run=client"},
},
{
name: "Helm v4 dryRunMode=false behaves like none",
config: dryRunFlagsConfig{
isHelmV4: true,
supportsDryRunLookup: true,
clusterAccessAllowed: true,
disableValidation: false,
dryRunMode: "false",
},
expected: []string{},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
actual := getTemplateDryRunFlags(tc.config)
if !reflect.DeepEqual(actual, tc.expected) {
t.Errorf("Expected %v, got %v", tc.expected, actual)
}
})
}
}

Copilot uses AI. Check for mistakes.
func TestExtractManifestFromHelmUpgradeDryRunOutput(t *testing.T) {
type testdata struct {
description string
Expand Down
9 changes: 5 additions & 4 deletions cmd/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var (

const (
dryRunNoOptDefVal = "client"
envTrue = "true"
)

type diffCmd struct {
Expand Down Expand Up @@ -116,7 +117,7 @@ func newChartCommand() *cobra.Command {
diff := diffCmd{
namespace: os.Getenv("HELM_NAMESPACE"),
}
unknownFlags := os.Getenv("HELM_DIFF_IGNORE_UNKNOWN_FLAGS") == "true"
unknownFlags := os.Getenv("HELM_DIFF_IGNORE_UNKNOWN_FLAGS") == envTrue

cmd := &cobra.Command{
Use: "upgrade [flags] [RELEASE] [CHART]",
Expand Down Expand Up @@ -166,10 +167,10 @@ func newChartCommand() *cobra.Command {
cmd.SilenceUsage = true

// See https://github.com/databus23/helm-diff/issues/253
diff.useUpgradeDryRun = os.Getenv("HELM_DIFF_USE_UPGRADE_DRY_RUN") == "true"
diff.useUpgradeDryRun = os.Getenv("HELM_DIFF_USE_UPGRADE_DRY_RUN") == envTrue

if !diff.threeWayMerge && !cmd.Flags().Changed("three-way-merge") {
enabled := os.Getenv("HELM_DIFF_THREE_WAY_MERGE") == "true"
enabled := os.Getenv("HELM_DIFF_THREE_WAY_MERGE") == envTrue
diff.threeWayMerge = enabled

if enabled {
Expand All @@ -178,7 +179,7 @@ func newChartCommand() *cobra.Command {
}

if !diff.normalizeManifests && !cmd.Flags().Changed("normalize-manifests") {
enabled := os.Getenv("HELM_DIFF_NORMALIZE_MANIFESTS") == "true"
enabled := os.Getenv("HELM_DIFF_NORMALIZE_MANIFESTS") == envTrue
diff.normalizeManifests = enabled

if enabled {
Expand Down
4 changes: 3 additions & 1 deletion cmd/upgrade_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cmd

import "testing"
import (
"testing"
)

func TestIsRemoteAccessAllowed(t *testing.T) {
cases := []struct {
Expand Down
Loading