From 4cc34986081b55687b565c4041171c152c9179d6 Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Wed, 16 May 2018 09:56:45 -0700 Subject: [PATCH] ref(cmd): refactor argument validation --- cmd/helm/create.go | 6 +- cmd/helm/delete.go | 8 +- cmd/helm/dependency.go | 5 +- cmd/helm/dependency_build.go | 4 +- cmd/helm/dependency_update.go | 4 +- cmd/helm/docs.go | 3 + cmd/helm/fetch.go | 8 +- cmd/helm/get.go | 10 +- cmd/helm/get_hooks.go | 7 +- cmd/helm/get_manifest.go | 7 +- cmd/helm/get_values.go | 7 +- cmd/helm/helm.go | 14 --- cmd/helm/history.go | 7 +- cmd/helm/home.go | 3 + cmd/helm/init.go | 5 +- cmd/helm/inspect.go | 17 +--- cmd/helm/install.go | 6 +- cmd/helm/lint.go | 2 +- cmd/helm/list.go | 4 +- cmd/helm/package.go | 2 +- cmd/helm/plugin_install.go | 5 +- cmd/helm/release_testing.go | 6 +- cmd/helm/repo.go | 5 +- cmd/helm/repo_add.go | 8 +- cmd/helm/repo_index.go | 9 +- cmd/helm/repo_list.go | 4 +- cmd/helm/repo_remove.go | 7 +- cmd/helm/repo_update.go | 2 + cmd/helm/require/args.go | 88 ++++++++++++++++++ cmd/helm/require/args_test.go | 91 +++++++++++++++++++ cmd/helm/rollback.go | 8 +- cmd/helm/root.go | 2 + cmd/helm/status.go | 7 +- cmd/helm/template.go | 2 +- cmd/helm/testdata/output/delete-no-args.txt | 4 +- .../testdata/output/get-hooks-no-args.txt | 4 +- .../testdata/output/get-manifest-no-args.txt | 4 +- cmd/helm/testdata/output/get-no-args.txt | 4 +- cmd/helm/testdata/output/get-values-args.txt | 4 +- cmd/helm/testdata/output/install-no-args.txt | 4 +- cmd/helm/testdata/output/rollback-no-args.txt | 4 +- .../upgrade-with-missing-dependencies.txt | 4 +- cmd/helm/upgrade.go | 6 +- cmd/helm/verify.go | 8 +- cmd/helm/verify_test.go | 2 +- cmd/helm/version.go | 2 + 46 files changed, 295 insertions(+), 128 deletions(-) create mode 100644 cmd/helm/require/args.go create mode 100644 cmd/helm/require/args_test.go diff --git a/cmd/helm/create.go b/cmd/helm/create.go index 6a099454a..6e63fd305 100644 --- a/cmd/helm/create.go +++ b/cmd/helm/create.go @@ -21,9 +21,9 @@ import ( "io" "path/filepath" - "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/hapi/chart" ) @@ -60,10 +60,8 @@ func newCreateCmd(out io.Writer) *cobra.Command { Use: "create NAME", Short: "create a new chart with the given name", Long: createDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errors.New("the name of the new chart is required") - } o.name = args[0] return o.run(out) }, diff --git a/cmd/helm/delete.go b/cmd/helm/delete.go index 49911257a..eec08ccc1 100644 --- a/cmd/helm/delete.go +++ b/cmd/helm/delete.go @@ -20,9 +20,9 @@ import ( "fmt" "io" - "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm" ) @@ -50,15 +50,13 @@ func newDeleteCmd(c helm.Interface, out io.Writer) *cobra.Command { o := &deleteOptions{client: c} cmd := &cobra.Command{ - Use: "delete [flags] RELEASE_NAME [...]", + Use: "delete RELEASE_NAME [...]", Aliases: []string{"del"}, SuggestFor: []string{"remove", "rm"}, Short: "given a release name, delete the release from Kubernetes", Long: deleteDesc, + Args: require.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errors.New("command 'delete' requires a release name") - } o.client = ensureHelmClient(o.client, false) for i := 0; i < len(args); i++ { diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index 37e408c10..e31c26000 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -25,6 +25,7 @@ import ( "github.com/gosuri/uitable" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/chartutil" ) @@ -93,6 +94,7 @@ func newDependencyCmd(out io.Writer) *cobra.Command { Aliases: []string{"dep", "dependencies"}, Short: "manage a chart's dependencies", Long: dependencyDesc, + Args: require.NoArgs, } cmd.AddCommand(newDependencyListCmd(out)) @@ -112,10 +114,11 @@ func newDependencyListCmd(out io.Writer) *cobra.Command { } cmd := &cobra.Command{ - Use: "list [flags] CHART", + Use: "list CHART", Aliases: []string{"ls"}, Short: "list the dependencies for the given chart", Long: dependencyListDesc, + Args: require.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { o.chartpath = filepath.Clean(args[0]) diff --git a/cmd/helm/dependency_build.go b/cmd/helm/dependency_build.go index 7a9942087..06b534ab1 100644 --- a/cmd/helm/dependency_build.go +++ b/cmd/helm/dependency_build.go @@ -20,6 +20,7 @@ import ( "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/downloader" "k8s.io/helm/pkg/getter" ) @@ -48,9 +49,10 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command { } cmd := &cobra.Command{ - Use: "build [flags] CHART", + Use: "build CHART", Short: "rebuild the charts/ directory based on the requirements.lock file", Long: dependencyBuildDesc, + Args: require.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { o.chartpath = args[0] diff --git a/cmd/helm/dependency_update.go b/cmd/helm/dependency_update.go index 47c116e00..3a8f7c0f2 100644 --- a/cmd/helm/dependency_update.go +++ b/cmd/helm/dependency_update.go @@ -21,6 +21,7 @@ import ( "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/downloader" "k8s.io/helm/pkg/getter" "k8s.io/helm/pkg/helm/helmpath" @@ -60,10 +61,11 @@ func newDependencyUpdateCmd(out io.Writer) *cobra.Command { } cmd := &cobra.Command{ - Use: "update [flags] CHART", + Use: "update CHART", Aliases: []string{"up"}, Short: "update charts/ based on the contents of requirements.yaml", Long: dependencyUpDesc, + Args: require.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { o.chartpath = filepath.Clean(args[0]) diff --git a/cmd/helm/docs.go b/cmd/helm/docs.go index 4139bbc32..1292e5807 100644 --- a/cmd/helm/docs.go +++ b/cmd/helm/docs.go @@ -22,6 +22,8 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/cobra/doc" + + "k8s.io/helm/cmd/helm/require" ) const docsDesc = ` @@ -51,6 +53,7 @@ func newDocsCmd(out io.Writer) *cobra.Command { Short: "Generate documentation as markdown or man pages", Long: docsDesc, Hidden: true, + Args: require.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { o.topCmd = cmd.Root() return o.run(out) diff --git a/cmd/helm/fetch.go b/cmd/helm/fetch.go index 284851085..e06b1402e 100644 --- a/cmd/helm/fetch.go +++ b/cmd/helm/fetch.go @@ -26,6 +26,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/downloader" "k8s.io/helm/pkg/getter" @@ -70,14 +71,11 @@ func newFetchCmd(out io.Writer) *cobra.Command { o := &fetchOptions{} cmd := &cobra.Command{ - Use: "fetch [flags] [chart URL | repo/chartname] [...]", + Use: "fetch [chart URL | repo/chartname] [...]", Short: "download a chart from a repository and (optionally) unpack it in local directory", Long: fetchDesc, + Args: require.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errors.Errorf("need at least one argument, url or repo/name of the chart") - } - if o.version == "" && o.devel { debug("setting version to >0.0.0-0") o.version = ">0.0.0-0" diff --git a/cmd/helm/get.go b/cmd/helm/get.go index 4c666d34c..ff9e9a3ca 100644 --- a/cmd/helm/get.go +++ b/cmd/helm/get.go @@ -19,9 +19,9 @@ package main import ( "io" - "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm" ) @@ -38,8 +38,6 @@ By default, this prints a human readable collection of information about the chart, the supplied values, and the generated manifest file. ` -var errReleaseRequired = errors.New("release name is required") - type getOptions struct { version int // --revision @@ -52,13 +50,11 @@ func newGetCmd(client helm.Interface, out io.Writer) *cobra.Command { o := &getOptions{client: client} cmd := &cobra.Command{ - Use: "get [flags] RELEASE_NAME", + Use: "get RELEASE_NAME", Short: "download a named release", Long: getHelp, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errReleaseRequired - } o.release = args[0] o.client = ensureHelmClient(o.client, false) return o.run(out) diff --git a/cmd/helm/get_hooks.go b/cmd/helm/get_hooks.go index 4716d7a62..19afa33cc 100644 --- a/cmd/helm/get_hooks.go +++ b/cmd/helm/get_hooks.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm" ) @@ -41,13 +42,11 @@ func newGetHooksCmd(client helm.Interface, out io.Writer) *cobra.Command { o := &getHooksOptions{client: client} cmd := &cobra.Command{ - Use: "hooks [flags] RELEASE_NAME", + Use: "hooks RELEASE_NAME", Short: "download all hooks for a named release", Long: getHooksHelp, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errReleaseRequired - } o.release = args[0] o.client = ensureHelmClient(o.client, false) return o.run(out) diff --git a/cmd/helm/get_manifest.go b/cmd/helm/get_manifest.go index 217d3d9cc..136b8b581 100644 --- a/cmd/helm/get_manifest.go +++ b/cmd/helm/get_manifest.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm" ) @@ -45,13 +46,11 @@ func newGetManifestCmd(client helm.Interface, out io.Writer) *cobra.Command { o := &getManifestOptions{client: client} cmd := &cobra.Command{ - Use: "manifest [flags] RELEASE_NAME", + Use: "manifest RELEASE_NAME", Short: "download the manifest for a named release", Long: getManifestHelp, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errReleaseRequired - } o.release = args[0] o.client = ensureHelmClient(o.client, false) return o.run(out) diff --git a/cmd/helm/get_values.go b/cmd/helm/get_values.go index a1e26a43d..cf1e37343 100644 --- a/cmd/helm/get_values.go +++ b/cmd/helm/get_values.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/helm" ) @@ -43,13 +44,11 @@ func newGetValuesCmd(client helm.Interface, out io.Writer) *cobra.Command { o := &getValuesOptions{client: client} cmd := &cobra.Command{ - Use: "values [flags] RELEASE_NAME", + Use: "values RELEASE_NAME", Short: "download the values file for a named release", Long: getValuesHelp, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errReleaseRequired - } o.release = args[0] o.client = ensureHelmClient(o.client, false) return o.run(out) diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 1e58be85f..390aa328a 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -20,10 +20,8 @@ import ( "fmt" "log" "os" - "strings" "sync" - "github.com/pkg/errors" // Import to initialize client auth plugins. _ "k8s.io/client-go/plugin/pkg/client/auth" "k8s.io/client-go/tools/clientcmd" @@ -59,18 +57,6 @@ func main() { } } -func checkArgsLength(argsReceived int, requiredArgs ...string) error { - expectedNum := len(requiredArgs) - if argsReceived != expectedNum { - arg := "arguments" - if expectedNum == 1 { - arg = "argument" - } - return errors.Errorf("this command needs %v %s: %s", expectedNum, arg, strings.Join(requiredArgs, ", ")) - } - return nil -} - // ensureHelmClient returns a new helm client impl. if h is not nil. func ensureHelmClient(h helm.Interface, allNamespaces bool) helm.Interface { if h != nil { diff --git a/cmd/helm/history.go b/cmd/helm/history.go index 4e0ae16e8..1139d9648 100644 --- a/cmd/helm/history.go +++ b/cmd/helm/history.go @@ -26,6 +26,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/hapi/chart" "k8s.io/helm/pkg/hapi/release" "k8s.io/helm/pkg/helm" @@ -71,14 +72,12 @@ func newHistoryCmd(c helm.Interface, out io.Writer) *cobra.Command { o := &historyOptions{client: c} cmd := &cobra.Command{ - Use: "history [flags] RELEASE_NAME", + Use: "history RELEASE_NAME", Long: historyHelp, Short: "fetch release history", Aliases: []string{"hist"}, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errReleaseRequired - } o.client = ensureHelmClient(o.client, false) o.release = args[0] return o.run(out) diff --git a/cmd/helm/home.go b/cmd/helm/home.go index 4ab649d74..87eded594 100644 --- a/cmd/helm/home.go +++ b/cmd/helm/home.go @@ -21,6 +21,8 @@ import ( "io" "github.com/spf13/cobra" + + "k8s.io/helm/cmd/helm/require" ) var longHomeHelp = ` @@ -33,6 +35,7 @@ func newHomeCmd(out io.Writer) *cobra.Command { Use: "home", Short: "displays the location of HELM_HOME", Long: longHomeHelp, + Args: require.NoArgs, Run: func(cmd *cobra.Command, args []string) { h := settings.Home fmt.Fprintln(out, h) diff --git a/cmd/helm/init.go b/cmd/helm/init.go index d70da0eea..14fa8d6d5 100644 --- a/cmd/helm/init.go +++ b/cmd/helm/init.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/getter" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/repo" @@ -52,10 +53,8 @@ func newInitCmd(out io.Writer) *cobra.Command { Use: "init", Short: "initialize Helm client", Long: initDesc, + Args: require.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - if len(args) != 0 { - return errors.New("this command does not accept arguments") - } o.home = settings.Home return o.run(out) }, diff --git a/cmd/helm/inspect.go b/cmd/helm/inspect.go index 266e6d0c5..787433f1e 100644 --- a/cmd/helm/inspect.go +++ b/cmd/helm/inspect.go @@ -24,6 +24,7 @@ import ( "github.com/ghodss/yaml" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/hapi/chart" ) @@ -80,10 +81,8 @@ func newInspectCmd(out io.Writer) *cobra.Command { Use: "inspect [CHART]", Short: "inspect a chart", Long: inspectDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "chart name"); err != nil { - return err - } cp, err := locateChartPath(o.repoURL, o.username, o.password, args[0], o.version, o.verify, o.keyring, o.certFile, o.keyFile, o.caFile) if err != nil { @@ -98,11 +97,9 @@ func newInspectCmd(out io.Writer) *cobra.Command { Use: "values [CHART]", Short: "shows inspect values", Long: inspectValuesDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { o.output = valuesOnly - if err := checkArgsLength(len(args), "chart name"); err != nil { - return err - } cp, err := locateChartPath(o.repoURL, o.username, o.password, args[0], o.version, o.verify, o.keyring, o.certFile, o.keyFile, o.caFile) if err != nil { @@ -117,11 +114,9 @@ func newInspectCmd(out io.Writer) *cobra.Command { Use: "chart [CHART]", Short: "shows inspect chart", Long: inspectChartDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { o.output = chartOnly - if err := checkArgsLength(len(args), "chart name"); err != nil { - return err - } cp, err := locateChartPath(o.repoURL, o.username, o.password, args[0], o.version, o.verify, o.keyring, o.certFile, o.keyFile, o.caFile) if err != nil { @@ -136,11 +131,9 @@ func newInspectCmd(out io.Writer) *cobra.Command { Use: "readme [CHART]", Short: "shows inspect readme", Long: readmeChartDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { o.output = readmeOnly - if err := checkArgsLength(len(args), "chart name"); err != nil { - return err - } cp, err := locateChartPath(o.repoURL, o.username, o.password, args[0], o.version, o.verify, o.keyring, o.certFile, o.keyFile, o.caFile) if err != nil { diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 032285b24..cbf5ff8e5 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -32,6 +32,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/downloader" "k8s.io/helm/pkg/getter" @@ -155,11 +156,8 @@ func newInstallCmd(c helm.Interface, out io.Writer) *cobra.Command { Use: "install [CHART]", Short: "install a chart archive", Long: installDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "chart name"); err != nil { - return err - } - debug("Original chart version: %q", o.version) if o.version == "" && o.devel { debug("setting version to >0.0.0-0") diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index f2ba0ef9f..3dd8d23b8 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -55,7 +55,7 @@ func newLintCmd(out io.Writer) *cobra.Command { o := &lintOptions{paths: []string{"."}} cmd := &cobra.Command{ - Use: "lint [flags] PATH", + Use: "lint PATH", Short: "examines a chart for possible issues", Long: longLintHelp, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/helm/list.go b/cmd/helm/list.go index 16a7a9d9f..2a020d0a3 100644 --- a/cmd/helm/list.go +++ b/cmd/helm/list.go @@ -24,6 +24,7 @@ import ( "github.com/gosuri/uitable" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/hapi" "k8s.io/helm/pkg/hapi/release" "k8s.io/helm/pkg/helm" @@ -83,10 +84,11 @@ func newListCmd(client helm.Interface, out io.Writer) *cobra.Command { o := &listOptions{client: client} cmd := &cobra.Command{ - Use: "list [flags] [FILTER]", + Use: "list [FILTER]", Short: "list releases", Long: listHelp, Aliases: []string{"ls"}, + Args: require.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { o.filter = strings.Join(args, " ") diff --git a/cmd/helm/package.go b/cmd/helm/package.go index 1181afab4..4ce446368 100644 --- a/cmd/helm/package.go +++ b/cmd/helm/package.go @@ -71,7 +71,7 @@ func newPackageCmd(out io.Writer) *cobra.Command { o := &packageOptions{} cmd := &cobra.Command{ - Use: "package [flags] [CHART_PATH] [...]", + Use: "package [CHART_PATH] [...]", Short: "package a chart directory into a chart archive", Long: packageDesc, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/helm/plugin_install.go b/cmd/helm/plugin_install.go index 614a9e8a9..74937fcce 100644 --- a/cmd/helm/plugin_install.go +++ b/cmd/helm/plugin_install.go @@ -19,6 +19,7 @@ import ( "fmt" "io" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/plugin" "k8s.io/helm/pkg/plugin/installer" @@ -45,6 +46,7 @@ func newPluginInstallCmd(out io.Writer) *cobra.Command { Use: "install [options] ...", Short: "install one or more Helm plugins", Long: pluginInstallDesc, + Args: require.ExactArgs(1), PreRunE: func(cmd *cobra.Command, args []string) error { return o.complete(args) }, @@ -57,9 +59,6 @@ func newPluginInstallCmd(out io.Writer) *cobra.Command { } func (o *pluginInstallOptions) complete(args []string) error { - if err := checkArgsLength(len(args), "plugin"); err != nil { - return err - } o.source = args[0] o.home = settings.Home return nil diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go index 51dfffe63..bd265f123 100644 --- a/cmd/helm/release_testing.go +++ b/cmd/helm/release_testing.go @@ -23,6 +23,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/hapi/release" "k8s.io/helm/pkg/helm" ) @@ -48,11 +49,8 @@ func newReleaseTestCmd(c helm.Interface, out io.Writer) *cobra.Command { Use: "test [RELEASE]", Short: "test a release", Long: releaseTestDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "release name"); err != nil { - return err - } - o.name = args[0] o.client = ensureHelmClient(o.client, false) return o.run(out) diff --git a/cmd/helm/repo.go b/cmd/helm/repo.go index 8acc762e2..a8166cc44 100644 --- a/cmd/helm/repo.go +++ b/cmd/helm/repo.go @@ -20,6 +20,8 @@ import ( "io" "github.com/spf13/cobra" + + "k8s.io/helm/cmd/helm/require" ) var repoHelm = ` @@ -32,9 +34,10 @@ Example usage: func newRepoCmd(out io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "repo [FLAGS] add|remove|list|index|update [ARGS]", + Use: "repo add|remove|list|index|update [ARGS]", Short: "add, list, remove, update, and index chart repositories", Long: repoHelm, + Args: require.NoArgs, } cmd.AddCommand(newRepoAddCmd(out)) diff --git a/cmd/helm/repo_add.go b/cmd/helm/repo_add.go index dad3b8df3..62427a69f 100644 --- a/cmd/helm/repo_add.go +++ b/cmd/helm/repo_add.go @@ -23,6 +23,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/getter" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/repo" @@ -45,13 +46,10 @@ func newRepoAddCmd(out io.Writer) *cobra.Command { o := &repoAddOptions{} cmd := &cobra.Command{ - Use: "add [flags] [NAME] [URL]", + Use: "add [NAME] [URL]", Short: "add a chart repository", + Args: require.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "name for the chart repository", "the url of the chart repository"); err != nil { - return err - } - o.name = args[0] o.url = args[1] o.home = settings.Home diff --git a/cmd/helm/repo_index.go b/cmd/helm/repo_index.go index 499d3ea22..f578a80c7 100644 --- a/cmd/helm/repo_index.go +++ b/cmd/helm/repo_index.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/repo" ) @@ -48,16 +49,12 @@ func newRepoIndexCmd(out io.Writer) *cobra.Command { o := &repoIndexOptions{} cmd := &cobra.Command{ - Use: "index [flags] [DIR]", + Use: "index [DIR]", Short: "generate an index file given a directory containing packaged charts", Long: repoIndexDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "path to a directory"); err != nil { - return err - } - o.dir = args[0] - return o.run(out) }, } diff --git a/cmd/helm/repo_list.go b/cmd/helm/repo_list.go index 4275c2fcf..4d5dc546a 100644 --- a/cmd/helm/repo_list.go +++ b/cmd/helm/repo_list.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/repo" ) @@ -36,8 +37,9 @@ func newRepoListCmd(out io.Writer) *cobra.Command { o := &repoListOptions{} cmd := &cobra.Command{ - Use: "list [flags]", + Use: "list", Short: "list chart repositories", + Args: require.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { o.home = settings.Home return o.run(out) diff --git a/cmd/helm/repo_remove.go b/cmd/helm/repo_remove.go index a27bee307..91b9b1fa9 100644 --- a/cmd/helm/repo_remove.go +++ b/cmd/helm/repo_remove.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/repo" ) @@ -37,13 +38,11 @@ func newRepoRemoveCmd(out io.Writer) *cobra.Command { o := &repoRemoveOptions{} cmd := &cobra.Command{ - Use: "remove [flags] [NAME]", + Use: "remove [NAME]", Aliases: []string{"rm"}, Short: "remove a chart repository", + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "name of chart repository"); err != nil { - return err - } o.name = args[0] o.home = settings.Home diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index d6b89f9bd..15d3505c9 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/getter" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/repo" @@ -52,6 +53,7 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command { Aliases: []string{"up"}, Short: "update information of available charts locally from chart repositories", Long: updateDesc, + Args: require.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { o.home = settings.Home return o.run(out) diff --git a/cmd/helm/require/args.go b/cmd/helm/require/args.go new file mode 100644 index 000000000..3c71d4b7b --- /dev/null +++ b/cmd/helm/require/args.go @@ -0,0 +1,88 @@ +/* +Copyright 2018 The Kubernetes Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package require + +import ( + "github.com/pkg/errors" + "github.com/spf13/cobra" +) + +// NoArgs returns an error if any args are included. +func NoArgs(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + return errors.Errorf( + "%q accepts no arguments\n\nUsage: %s", + cmd.CommandPath(), + cmd.UseLine(), + ) + } + return nil +} + +// ExactArgs returns an error if there are not exactly n args. +func ExactArgs(n int) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if len(args) != n { + return errors.Errorf( + "%q requires %d %s\n\nUsage: %s", + cmd.CommandPath(), + n, + pluralize("argument", n), + cmd.UseLine(), + ) + } + return nil + } +} + +// MaximumNArgs returns an error if there are more than N args. +func MaximumNArgs(n int) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if len(args) > n { + return errors.Errorf( + "%q accepts at most %d %s\n\nUsage: %s", + cmd.CommandPath(), + n, + pluralize("argument", n), + cmd.UseLine(), + ) + } + return nil + } +} + +// MinimumNArgs returns an error if there is not at least N args. +func MinimumNArgs(n int) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if len(args) < n { + return errors.Errorf( + "%q requires at least %d %s\n\nUsage: %s", + cmd.CommandPath(), + n, + pluralize("argument", n), + cmd.UseLine(), + ) + } + return nil + } +} + +func pluralize(word string, n int) string { + if n == 1 { + return word + } + return word + "s" +} diff --git a/cmd/helm/require/args_test.go b/cmd/helm/require/args_test.go new file mode 100644 index 000000000..4098ed314 --- /dev/null +++ b/cmd/helm/require/args_test.go @@ -0,0 +1,91 @@ +/* +Copyright 2018 The Kubernetes Authors All rights reserved. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package require + +import ( + "fmt" + "io/ioutil" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func TestArgs(t *testing.T) { + runTestCases(t, []testCase{{ + validateFunc: NoArgs, + }, { + args: []string{"one"}, + validateFunc: NoArgs, + wantError: `"root" accepts no arguments`, + }, { + args: []string{"one"}, + validateFunc: ExactArgs(1), + }, { + validateFunc: ExactArgs(1), + wantError: `"root" requires 1 argument`, + }, { + validateFunc: ExactArgs(2), + wantError: `"root" requires 2 arguments`, + }, { + args: []string{"one"}, + validateFunc: MaximumNArgs(1), + }, { + args: []string{"one", "two"}, + validateFunc: MaximumNArgs(1), + wantError: `"root" accepts at most 1 argument`, + }, { + validateFunc: MinimumNArgs(1), + wantError: `"root" requires at least 1 argument`, + }, { + args: []string{"one", "two"}, + validateFunc: MinimumNArgs(1), + }}) +} + +type testCase struct { + args []string + validateFunc cobra.PositionalArgs + wantError string +} + +func runTestCases(t *testing.T, testCases []testCase) { + for i, tc := range testCases { + t.Run(fmt.Sprint(i), func(t *testing.T) { + cmd := &cobra.Command{ + Use: "root", + Run: func(*cobra.Command, []string) {}, + Args: tc.validateFunc, + } + cmd.SetArgs(tc.args) + cmd.SetOutput(ioutil.Discard) + + err := cmd.Execute() + if tc.wantError == "" { + if err != nil { + t.Fatalf("unexpected error, got '%v'", err) + } + return + } + if !strings.Contains(err.Error(), tc.wantError) { + t.Fatalf("unexpected error \n\nWANT:\n%q\n\nGOT:\n%q\n", tc.wantError, err) + } + if !strings.Contains(err.Error(), "Usage:") { + t.Fatalf("unexpected error: want Usage string\n\nGOT:\n%q\n", err) + } + }) + } +} diff --git a/cmd/helm/rollback.go b/cmd/helm/rollback.go index 5dd002909..29e025cc7 100644 --- a/cmd/helm/rollback.go +++ b/cmd/helm/rollback.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm" ) @@ -51,14 +52,11 @@ func newRollbackCmd(c helm.Interface, out io.Writer) *cobra.Command { o := &rollbackOptions{client: c} cmd := &cobra.Command{ - Use: "rollback [flags] [RELEASE] [REVISION]", + Use: "rollback [RELEASE] [REVISION]", Short: "roll back a release to a previous revision", Long: rollbackDesc, + Args: require.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "release name", "revision number"); err != nil { - return err - } - o.name = args[0] v64, err := strconv.ParseInt(args[1], 10, 32) diff --git a/cmd/helm/root.go b/cmd/helm/root.go index 4ee894588..15737a6e9 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -21,6 +21,7 @@ import ( "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/helm" ) @@ -51,6 +52,7 @@ func newRootCmd(c helm.Interface, out io.Writer, args []string) *cobra.Command { Short: "The Helm package manager for Kubernetes.", Long: globalUsage, SilenceUsage: true, + Args: require.NoArgs, } flags := cmd.PersistentFlags() diff --git a/cmd/helm/status.go b/cmd/helm/status.go index f16b66bcf..6894f0bbd 100644 --- a/cmd/helm/status.go +++ b/cmd/helm/status.go @@ -29,6 +29,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/hapi" "k8s.io/helm/pkg/hapi/release" "k8s.io/helm/pkg/helm" @@ -56,13 +57,11 @@ func newStatusCmd(client helm.Interface, out io.Writer) *cobra.Command { o := &statusOptions{client: client} cmd := &cobra.Command{ - Use: "status [flags] RELEASE_NAME", + Use: "status RELEASE_NAME", Short: "displays the status of the named release", Long: statusHelp, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errReleaseRequired - } o.release = args[0] o.client = ensureHelmClient(o.client, false) return o.run(out) diff --git a/cmd/helm/template.go b/cmd/helm/template.go index 2c8fb1284..f59c4161c 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -77,7 +77,7 @@ func newTemplateCmd(out io.Writer) *cobra.Command { o := &templateOptions{} cmd := &cobra.Command{ - Use: "template [flags] CHART", + Use: "template CHART", Short: fmt.Sprintf("locally render templates"), Long: templateDesc, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/helm/testdata/output/delete-no-args.txt b/cmd/helm/testdata/output/delete-no-args.txt index 8fa7e2d3f..f06a4cb4e 100644 --- a/cmd/helm/testdata/output/delete-no-args.txt +++ b/cmd/helm/testdata/output/delete-no-args.txt @@ -1 +1,3 @@ -Error: command 'delete' requires a release name +Error: "helm delete" requires at least 1 argument + +Usage: helm delete RELEASE_NAME [...] [flags] diff --git a/cmd/helm/testdata/output/get-hooks-no-args.txt b/cmd/helm/testdata/output/get-hooks-no-args.txt index 3d0b2a17a..2911fdb88 100644 --- a/cmd/helm/testdata/output/get-hooks-no-args.txt +++ b/cmd/helm/testdata/output/get-hooks-no-args.txt @@ -1 +1,3 @@ -Error: release name is required +Error: "helm get hooks" requires 1 argument + +Usage: helm get hooks RELEASE_NAME [flags] diff --git a/cmd/helm/testdata/output/get-manifest-no-args.txt b/cmd/helm/testdata/output/get-manifest-no-args.txt index 3d0b2a17a..df7aa5b04 100644 --- a/cmd/helm/testdata/output/get-manifest-no-args.txt +++ b/cmd/helm/testdata/output/get-manifest-no-args.txt @@ -1 +1,3 @@ -Error: release name is required +Error: "helm get manifest" requires 1 argument + +Usage: helm get manifest RELEASE_NAME [flags] diff --git a/cmd/helm/testdata/output/get-no-args.txt b/cmd/helm/testdata/output/get-no-args.txt index 3d0b2a17a..b911b38c5 100644 --- a/cmd/helm/testdata/output/get-no-args.txt +++ b/cmd/helm/testdata/output/get-no-args.txt @@ -1 +1,3 @@ -Error: release name is required +Error: "helm get" requires 1 argument + +Usage: helm get RELEASE_NAME [flags] diff --git a/cmd/helm/testdata/output/get-values-args.txt b/cmd/helm/testdata/output/get-values-args.txt index 3d0b2a17a..c8a65e7f3 100644 --- a/cmd/helm/testdata/output/get-values-args.txt +++ b/cmd/helm/testdata/output/get-values-args.txt @@ -1 +1,3 @@ -Error: release name is required +Error: "helm get values" requires 1 argument + +Usage: helm get values RELEASE_NAME [flags] diff --git a/cmd/helm/testdata/output/install-no-args.txt b/cmd/helm/testdata/output/install-no-args.txt index 7a4172b64..faafcb5c2 100644 --- a/cmd/helm/testdata/output/install-no-args.txt +++ b/cmd/helm/testdata/output/install-no-args.txt @@ -1 +1,3 @@ -Error: this command needs 1 argument: chart name +Error: "helm install" requires 1 argument + +Usage: helm install [CHART] [flags] diff --git a/cmd/helm/testdata/output/rollback-no-args.txt b/cmd/helm/testdata/output/rollback-no-args.txt index bee077279..3fde9d219 100644 --- a/cmd/helm/testdata/output/rollback-no-args.txt +++ b/cmd/helm/testdata/output/rollback-no-args.txt @@ -1 +1,3 @@ -Error: this command needs 2 arguments: release name, revision number +Error: "helm rollback" requires 2 arguments + +Usage: helm rollback [RELEASE] [REVISION] [flags] diff --git a/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt b/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt index f5fea53ed..e2186cd90 100644 --- a/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt +++ b/cmd/helm/testdata/output/upgrade-with-missing-dependencies.txt @@ -1 +1,3 @@ -Error: this command needs 2 arguments: release name, chart path +Error: "helm upgrade" requires 2 arguments + +Usage: helm upgrade [RELEASE] [CHART] [flags] diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 38feb91f3..cdbbc1f4a 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/chartutil" "k8s.io/helm/pkg/helm" "k8s.io/helm/pkg/storage/driver" @@ -90,11 +91,8 @@ func newUpgradeCmd(client helm.Interface, out io.Writer) *cobra.Command { Use: "upgrade [RELEASE] [CHART]", Short: "upgrade a release", Long: upgradeDesc, + Args: require.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - if err := checkArgsLength(len(args), "release name", "chart path"); err != nil { - return err - } - if o.version == "" && o.devel { debug("setting version to >0.0.0-0") o.version = ">0.0.0-0" diff --git a/cmd/helm/verify.go b/cmd/helm/verify.go index 4c96e5434..5d0ee221a 100644 --- a/cmd/helm/verify.go +++ b/cmd/helm/verify.go @@ -18,9 +18,9 @@ package main import ( "io" - "github.com/pkg/errors" "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/downloader" ) @@ -44,13 +44,11 @@ func newVerifyCmd(out io.Writer) *cobra.Command { o := &verifyOptions{} cmd := &cobra.Command{ - Use: "verify [flags] PATH", + Use: "verify PATH", Short: "verify that a chart at the given path has been signed and is valid", Long: verifyDesc, + Args: require.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return errors.New("a path to a package file is required") - } o.chartfile = args[0] return o.run(out) }, diff --git a/cmd/helm/verify_test.go b/cmd/helm/verify_test.go index 7ae98742c..ccc2ff475 100644 --- a/cmd/helm/verify_test.go +++ b/cmd/helm/verify_test.go @@ -41,7 +41,7 @@ func TestVerifyCmd(t *testing.T) { { name: "verify requires a chart", cmd: "verify", - expect: "a path to a package file is required", + expect: "\"helm verify\" requires 1 argument\n\nUsage: helm verify PATH [flags]", wantError: true, }, { diff --git a/cmd/helm/version.go b/cmd/helm/version.go index 7641523b9..857539fe1 100644 --- a/cmd/helm/version.go +++ b/cmd/helm/version.go @@ -23,6 +23,7 @@ import ( "github.com/spf13/cobra" + "k8s.io/helm/cmd/helm/require" "k8s.io/helm/pkg/version" ) @@ -52,6 +53,7 @@ func newVersionCmd(out io.Writer) *cobra.Command { Use: "version", Short: "print the client version information", Long: versionDesc, + Args: require.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return o.run(out) },