Skip to content

Remake dry-run=server optional #547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 27, 2024
Merged

Remake dry-run=server optional #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38df431
Remake dry-run=server optional
mumoshu Jan 27, 2024
baa7897
Remove redundant test case
mumoshu Jan 27, 2024
0736ae1
optimze little code
yxxhero Jan 27, 2024
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
111 changes: 34 additions & 77 deletions cmd/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import (
"fmt"
"log"
"os"
"slices"
"strconv"
"strings"

jsonpatch "github.com/evanphx/json-patch"
jsoniterator "github.com/json-iterator/go"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/cli"
"helm.sh/helm/v3/pkg/kube"
Expand All @@ -29,6 +29,14 @@ import (
"github.com/databus23/helm-diff/v3/manifest"
)

var (
validDryRunValues = []string{"server", "client", "true", "false"}
)

const (
dryRunNoOptDefVal = "client"
)

type diffCmd struct {
release string
chart string
Expand All @@ -38,8 +46,6 @@ type diffCmd struct {
devel bool
disableValidation bool
disableOpenAPIValidation bool
dryRunMode string
dryRunModeSpecified bool
enableDNS bool
namespace string // namespace to assume the release to be installed into. Defaults to the current kube config namespace.
valueFiles valueFiles
Expand All @@ -62,6 +68,14 @@ type diffCmd struct {
kubeVersion string
useUpgradeDryRun bool
diff.Options

// dryRunMode can take the following values:
// - "none": no dry run is performed
// - "client": dry run is performed without remote cluster access
// - "server": dry run is performed with remote cluster access
// - "true": same as "client"
// - "false": same as "none"
dryRunMode string
}

func (d *diffCmd) isAllowUnreleased() bool {
Expand All @@ -73,10 +87,9 @@ func (d *diffCmd) isAllowUnreleased() bool {

// clusterAccessAllowed returns true if the diff command is allowed to access the cluster at some degree.
//
// helm-diff basically have 3 modes of operation:
// 1. no cluster access at all when --dry-run, --dry-run=client, or --dry-run=true is specified.
// 2. basic cluster access when --dry-run is not specified.
// 3. extra cluster access when --dry-run=server is specified.
// helm-diff basically have 2 modes of operation:
// 1. without cluster access at all when --dry-run=true or --dry-run=client is specified.
// 2. with cluster access when --dry-run is unspecified, false, or server.
//
// clusterAccessAllowed returns true when the mode is either 2 or 3.
//
Expand All @@ -89,8 +102,7 @@ func (d *diffCmd) isAllowUnreleased() bool {
//
// See also https://github.com/helm/helm/pull/9426#discussion_r1181397259
func (d *diffCmd) clusterAccessAllowed() bool {
clientOnly := (d.dryRunModeSpecified && d.dryRunMode == "") || d.dryRunMode == "true" || d.dryRunMode == "client"
return !clientOnly
return d.dryRunMode == "none" || d.dryRunMode == "false" || d.dryRunMode == "server"
}

const globalUsage = `Show a diff explaining what a helm upgrade would change.
Expand All @@ -111,10 +123,9 @@ func newChartCommand() *cobra.Command {
unknownFlags := os.Getenv("HELM_DIFF_IGNORE_UNKNOWN_FLAGS") == "true"

cmd := &cobra.Command{
Use: "upgrade [flags] [RELEASE] [CHART]",
Short: "Show a diff explaining what a helm upgrade would change.",
Long: globalUsage,
DisableFlagParsing: true,
Use: "upgrade [flags] [RELEASE] [CHART]",
Short: "Show a diff explaining what a helm upgrade would change.",
Long: globalUsage,
Example: strings.Join([]string{
" helm diff upgrade my-release stable/postgresql --values values.yaml",
"",
Expand Down Expand Up @@ -153,71 +164,14 @@ func newChartCommand() *cobra.Command {
"# Read the flag usage below for more information on --context.",
"HELM_DIFF_OUTPUT_CONTEXT=5 helm diff upgrade my-release datadog/datadog",
}, "\n"),
Args: func(cmd *cobra.Command, args []string) error {
return checkArgsLength(len(args), "release name", "chart path")
},
RunE: func(cmd *cobra.Command, args []string) error {
// Note that we can't just move this block to PersistentPreRunE,
// because cmd.SetArgs(args) does not persist between PersistentPreRunE and RunE.
// The choice is between:
// 1. Doing this in RunE
// 2. Doing this in PersistentPreRunE, saving args somewhere, and calling cmd.SetArgs(args) again in RunE
// 2 is more complicated without much benefit, so we choose 1.
{
const (
dryRunUsage = "--dry-run, --dry-run=client, or --dry-run=true disables cluster access and show diff as if it was install. Implies --install, --reset-values, and --disable-validation." +
" --dry-run=server enables the cluster access with helm-get and the lookup template function."
)

cmdFlags := cmd.Flags()

// see: https://github.com/databus23/helm-diff/issues/537
cmdFlags.ParseErrorsWhitelist.UnknownFlags = unknownFlags

legacyDryRunFlagSet := pflag.NewFlagSet("upgrade", pflag.ContinueOnError)
legacyDryRun := legacyDryRunFlagSet.Bool("dry-run", false, dryRunUsage)
if err := legacyDryRunFlagSet.Parse(args); err == nil && *legacyDryRun {
diff.dryRunModeSpecified = true
args = legacyDryRunFlagSet.Args()
} else {
cmdFlags.StringVar(&diff.dryRunMode, "dry-run", "", dryRunUsage)
}

// Here we parse the flags ourselves so that we can support
// both --dry-run and --dry-run=ARG syntaxes.
//
// If you don't do this, then cobra will treat --dry-run as
// a "flag needs an argument: --dry-run" error.
//
// This works becase we have `DisableFlagParsing: true`` above.
// Never turn that off, or you'll get the error again.
if err := cmdFlags.Parse(args); err != nil {
return err
}

args = cmdFlags.Args()

if !diff.dryRunModeSpecified {
dryRunModeSpecified := cmd.Flags().Changed("dry-run")
diff.dryRunModeSpecified = dryRunModeSpecified

if dryRunModeSpecified && !(diff.dryRunMode == "client" || diff.dryRunMode == "server") {
return fmt.Errorf("flag %q must take an argument of %q or %q but got %q", "dry-run", "client", "server", diff.dryRunMode)
}
}

cmd.SetArgs(args)

// We have to do this here because we have to sort out the
// --dry-run flag ambiguity to determine the args to be checked.
//
// In other words, we can't just do:
//
// cmd.Args = func(cmd *cobra.Command, args []string) error {
// return checkArgsLength(len(args), "release name", "chart path")
// }
//
// Because it seems to take precedence over the PersistentPreRunE
if err := checkArgsLength(len(args), "release name", "chart path"); err != nil {
return err
}
if diff.dryRunMode == "" {
diff.dryRunMode = "none"
} else if !slices.Contains(validDryRunValues, diff.dryRunMode) {
return fmt.Errorf("flag %q must take a bool value or either %q or %q, but got %q", "dry-run", "client", "server", diff.dryRunMode)
}

// Suppress the command usage on error. See #77 for more info
Expand Down Expand Up @@ -293,6 +247,9 @@ func newChartCommand() *cobra.Command {
f.BoolVar(&diff.devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored.")
f.BoolVar(&diff.disableValidation, "disable-validation", false, "disables rendered templates validation against the Kubernetes cluster you are currently pointing to. This is the same validation performed on an install")
f.BoolVar(&diff.disableOpenAPIValidation, "disable-openapi-validation", false, "disables rendered templates validation against the Kubernetes OpenAPI Schema")
f.StringVar(&diff.dryRunMode, "dry-run", "", "--dry-run, --dry-run=client, or --dry-run=true disables cluster access and show diff as if it was install. Implies --install, --reset-values, and --disable-validation."+
" --dry-run=server enables the cluster access with helm-get and the lookup template function.")
f.Lookup("dry-run").NoOptDefVal = dryRunNoOptDefVal
f.BoolVar(&diff.enableDNS, "enable-dns", false, "enable DNS lookups when rendering templates")
f.StringVar(&diff.postRenderer, "post-renderer", "", "the path to an executable to be used for post rendering. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path")
f.StringArrayVar(&diff.postRendererArgs, "post-renderer-args", []string{}, "an argument to the post-renderer (can specify multiple)")
Expand Down
27 changes: 16 additions & 11 deletions cmd/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,44 @@ func TestIsRemoteAccessAllowed(t *testing.T) {
expected bool
}{
{
name: "no flags",
cmd: diffCmd{},
name: "no flags",
cmd: diffCmd{
dryRunMode: "none",
},
expected: true,
},
{
name: "legacy explicit dry-run flag",
name: "legacy explicit dry-run=true flag",
cmd: diffCmd{
dryRunModeSpecified: true,
dryRunMode: "true",
dryRunMode: "true",
},
expected: false,
},
{
name: "legacy explicit dry-run=false flag",
cmd: diffCmd{
dryRunMode: "false",
},
expected: true,
},
{
name: "legacy empty dry-run flag",
cmd: diffCmd{
dryRunModeSpecified: true,
dryRunMode: "",
dryRunMode: dryRunNoOptDefVal,
},
expected: false,
},
{
name: "server-side dry-run flag",
cmd: diffCmd{
dryRunModeSpecified: true,
dryRunMode: "server",
dryRunMode: "server",
},
expected: true,
},
{
name: "client-side dry-run flag",
cmd: diffCmd{
dryRunModeSpecified: true,
dryRunMode: "client",
dryRunMode: "client",
},
expected: false,
},
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/evanphx/json-patch v5.8.1+incompatible
github.com/json-iterator/go v1.1.12
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
github.com/pkg/errors v0.9.1
github.com/pkg/errors v0.9.1 // indirect
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.8.4
Expand Down