Skip to content
Open
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
2 changes: 2 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ jobs:

- name: helm diff upgrade --install helm-diff ./helm-diff
run: helm diff upgrade --install helm-diff ./helm-diff
env:
HELM_DEBUG: "true"

- name: helm upgrade -i helm-diff ./helm-diff
run: helm upgrade -i helm-diff ./helm-diff
Expand Down
66 changes: 28 additions & 38 deletions cmd/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,12 @@ 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.
// See https://github.com/databus23/helm-diff/issues/894
if d.dryRunMode != "client" {
flags = append(flags, "--dry-run=server")
}
} else {
flags = append(flags, "--validate")
}
Expand All @@ -353,42 +357,28 @@ 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 is already added above if d.dryRunMode != "client"
// Only add it here if d.dryRunMode == "client" or for Helm v3
if !(isHelmV4 && d.dryRunMode != "client") {
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
174 changes: 174 additions & 0 deletions cmd/helm_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,185 @@
package cmd

import (
"reflect"
"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 cfg.dryRunMode != "client" {
flags = append(flags, "--dry-run=server")
}
} else {
flags = append(flags, "--validate")
}
}

if cfg.supportsDryRunLookup {
if !(cfg.isHelmV4 && cfg.dryRunMode != "client") {
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 TestExtractManifestFromHelmUpgradeDryRunOutput(t *testing.T) {
type testdata struct {
description string
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