From cc6f328f64088511ff036336aad3cd2f29a5698d Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Fri, 31 Jan 2020 17:43:58 +0100 Subject: [PATCH] Raising error if arguments are provided to version and install commands Aims to conform to the same behavior with other commands: `install` and `version` ones are not expecting to receive arguments, so raising the error `errorWantedNoArgs`. In addition, provided some little test coverage and added the required flags for the `install` command since `--git-email` was missing but marked as mandatory. Pretty sure the change is breaking for users using the installation method via the CLI instead of the Helm Chart: they could have automation scripts that are pushing arguments and these could break. Closes #2808 --- cmd/fluxctl/install_cmd.go | 5 ++- cmd/fluxctl/install_cmd_test.go | 63 +++++++++++++++++++++++++++++++++ cmd/fluxctl/version_cmd.go | 3 ++ cmd/fluxctl/version_cmd_test.go | 59 ++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 cmd/fluxctl/install_cmd_test.go create mode 100644 cmd/fluxctl/version_cmd_test.go diff --git a/cmd/fluxctl/install_cmd.go b/cmd/fluxctl/install_cmd.go index 3040adabd..d3fa77384 100644 --- a/cmd/fluxctl/install_cmd.go +++ b/cmd/fluxctl/install_cmd.go @@ -25,7 +25,7 @@ func (opts *installOpts) Command() *cobra.Command { Use: "install", Short: "Print and tweak Kubernetes manifests needed to install Flux in a Cluster", Example: `# Install Flux and make it use Git repository git@github.com:/flux-get-started -fluxctl install --git-url 'git@github.com:/flux-get-started' | kubectl -f -`, +fluxctl install --git-url 'git@github.com:/flux-get-started' --git-email= | kubectl -f -`, RunE: opts.RunE, } cmd.Flags().StringVar(&opts.GitURL, "git-url", "", @@ -60,6 +60,9 @@ fluxctl install --git-url 'git@github.com:/flux-get-started' | ku } func (opts *installOpts) RunE(cmd *cobra.Command, args []string) error { + if len(args) != 0 { + return errorWantedNoArgs + } if opts.GitURL == "" { return fmt.Errorf("please supply a valid --git-url argument") } diff --git a/cmd/fluxctl/install_cmd_test.go b/cmd/fluxctl/install_cmd_test.go new file mode 100644 index 000000000..2ea4f29c4 --- /dev/null +++ b/cmd/fluxctl/install_cmd_test.go @@ -0,0 +1,63 @@ +package main + +import ( + "bytes" + "fmt" + "testing" +) + +func TestInstallCommand_ExtraArgumentFailure(t *testing.T) { + for k, v := range [][]string{ + {"foo"}, + {"foo", "bar"}, + {"foo", "bar", "bizz"}, + {"foo", "bar", "bizz", "buzz"}, + } { + t.Run(fmt.Sprintf("%d", k), func(t *testing.T) { + cmd := newInstall().Command() + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.SetArgs(v) + _ = cmd.Flags().Set("git-url", "git@github.com:testcase/flux-get-started") + _ = cmd.Flags().Set("git-email", "testcase@weave.works") + if err := cmd.Execute(); err == nil { + t.Fatalf("expecting error, got nil") + } + }) + } +} + +func TestInstallCommand_MissingRequiredFlag(t *testing.T) { + for k, v := range map[string]string{ + "git-url": "git@github.com:testcase/flux-get-started", + "git-email": "testcase@weave.works", + } { + t.Run(fmt.Sprintf("only --%s", k), func(t *testing.T) { + cmd := newInstall().Command() + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.SetArgs([]string{}) + _ = cmd.Flags().Set(k, v) + if err := cmd.Execute(); err == nil { + t.Fatalf("expecting error, got nil") + } + }) + } +} + +func TestInstallCommand_Success(t *testing.T) { + f := make(map[string]string) + f["git-url"] = "git@github.com:testcase/flux-get-started" + f["git-email"] = "testcase@weave.works" + + cmd := newInstall().Command() + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.SetArgs([]string{}) + for k, v := range f { + _ = cmd.Flags().Set(k, v) + } + if err := cmd.Execute(); err != nil { + t.Fatalf("expecting nil, got error (%s)", err.Error()) + } +} diff --git a/cmd/fluxctl/version_cmd.go b/cmd/fluxctl/version_cmd.go index 4af5b03d0..f945eb8ff 100644 --- a/cmd/fluxctl/version_cmd.go +++ b/cmd/fluxctl/version_cmd.go @@ -13,6 +13,9 @@ func newVersionCommand() *cobra.Command { Use: "version", Short: "Output the version of fluxctl", RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 0 { + return errorWantedNoArgs + } if version == "" { version = "unversioned" } diff --git a/cmd/fluxctl/version_cmd_test.go b/cmd/fluxctl/version_cmd_test.go new file mode 100644 index 000000000..c85083a3d --- /dev/null +++ b/cmd/fluxctl/version_cmd_test.go @@ -0,0 +1,59 @@ +package main + +import ( + "bytes" + "fmt" + "strings" + "testing" +) + +func TestVersionCommand_InputFailure(t *testing.T) { + for k, v := range [][]string{ + {"foo"}, + {"foo", "bar"}, + {"foo", "bar", "bizz"}, + {"foo", "bar", "bizz", "buzz"}, + } { + t.Run(fmt.Sprintf("%d", k), func(t *testing.T) { + buf := new(bytes.Buffer) + cmd := newVersionCommand() + cmd.SetOut(buf) + cmd.SetArgs(v) + if err := cmd.Execute(); err == nil { + t.Fatalf("Expecting error: command is not expecting extra arguments") + } + }) + } +} + +func TestVersionCommand_Success(t *testing.T) { + buf := new(bytes.Buffer) + cmd := newVersionCommand() + cmd.SetOut(buf) + cmd.SetArgs([]string{}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Expecting nil, got error (%s)", err.Error()) + } +} + +func TestVersionCommand_SuccessCheckVersion(t *testing.T) { + for _, e := range []string{ + "v1.0.0", + "v2.0.0", + } { + t.Run(e, func(t *testing.T) { + buf := new(bytes.Buffer) + cmd := newVersionCommand() + version = e + cmd.SetOut(buf) + cmd.SetArgs([]string{}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Expecting nil, got error (%s)", err.Error()) + } + if g := strings.TrimRight(buf.String(), "\n"); e != g { + println(e == g) + t.Fatalf("Expected %s, got %s", e, g) + } + }) + } +}