From ca4856bf6e9ef5b43e38567c03a66fd398ea1953 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Sat, 21 Dec 2024 00:16:06 +0100 Subject: [PATCH] Reject unknown sub-commands Cobra has the property of not reporting any unrecognized sub-commands and simply displaying the help page for any command that doesn't have a run function set. Add a unit test that probes all sub-commands to properly reject unknown sub-commands. Whenever a sub-command is added that accepts arguments and doesn't declare them as ValidArgs, it must be added to the ignore list at the beginning of that test. Signed-off-by: Tom Wieczorek --- cmd/airgap/airgap.go | 2 + cmd/airgap/listimages.go | 3 +- cmd/airgap/listimages_test.go | 12 ++-- cmd/api/api.go | 3 +- cmd/backup/backup_unix.go | 5 +- cmd/backup/backup_windows.go | 1 + cmd/config/config.go | 19 ++++++ cmd/config/config_test.go | 64 +++++++++++++++++++ cmd/config/create.go | 3 +- cmd/config/edit.go | 10 +-- cmd/config/status.go | 14 ++--- cmd/config/validate.go | 3 +- cmd/controller/controller.go | 1 + cmd/etcd/etcd.go | 2 + cmd/etcd/leave.go | 2 +- cmd/etcd/list.go | 2 +- cmd/etcd/list_test.go | 32 ---------- cmd/install/controller.go | 3 +- cmd/install/install.go | 2 + cmd/install/worker.go | 3 +- cmd/kubeconfig/admin.go | 1 + cmd/kubeconfig/kubeconfig.go | 2 + cmd/reset/reset.go | 8 ++- cmd/root.go | 8 +-- cmd/root_test.go | 84 +++++++++++++++++++++++++ cmd/start/start.go | 3 +- cmd/status/status.go | 6 +- cmd/stop/stop.go | 3 +- cmd/sysinfo/sysinfo.go | 3 +- cmd/token/create.go | 5 +- cmd/token/invalidate.go | 7 +-- cmd/token/list.go | 5 +- cmd/token/preshared.go | 5 +- cmd/token/token.go | 2 + cmd/validate/validate.go | 7 ++- cmd/version/version.go | 4 +- cmd/worker/worker.go | 1 + pkg/autopilot/controller/root_worker.go | 2 +- pkg/backup/manager_unix.go | 2 +- 39 files changed, 248 insertions(+), 96 deletions(-) create mode 100644 cmd/config/config_test.go delete mode 100644 cmd/etcd/list_test.go diff --git a/cmd/airgap/airgap.go b/cmd/airgap/airgap.go index 4c07343e9e7f..9d15ee6df5d0 100644 --- a/cmd/airgap/airgap.go +++ b/cmd/airgap/airgap.go @@ -26,6 +26,8 @@ func NewAirgapCmd() *cobra.Command { cmd := &cobra.Command{ Use: "airgap", Short: "Manage airgap setup", + Args: cobra.NoArgs, + Run: func(*cobra.Command, []string) { /* Enforce arg validation. */ }, } cmd.AddCommand(NewAirgapListImagesCmd()) diff --git a/cmd/airgap/listimages.go b/cmd/airgap/listimages.go index 842ad5ce1705..071b34b5f02e 100644 --- a/cmd/airgap/listimages.go +++ b/cmd/airgap/listimages.go @@ -32,7 +32,8 @@ func NewAirgapListImagesCmd() *cobra.Command { Use: "list-images", Short: "List image names and version needed for air-gap install", Example: `k0s airgap list-images`, - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/airgap/listimages_test.go b/cmd/airgap/listimages_test.go index be20ffc85a87..dfa749b0a1a9 100644 --- a/cmd/airgap/listimages_test.go +++ b/cmd/airgap/listimages_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package airgap +package airgap_test import ( "errors" @@ -26,6 +26,7 @@ import ( "testing" "testing/iotest" + "github.com/k0sproject/k0s/cmd" internalio "github.com/k0sproject/k0s/internal/io" "github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1" @@ -46,15 +47,12 @@ func TestAirgapListImages(t *testing.T) { t.Run("HonorsIOErrors", func(t *testing.T) { var writes uint - underTest := NewAirgapListImagesCmd() - underTest.SetIn(iotest.ErrReader(errors.New("unexpected read from standard input"))) + underTest, _, stderr := newAirgapListImagesCmdWithConfig(t, "") underTest.SilenceUsage = true // Cobra writes usage to stdout on errors 🤔 underTest.SetOut(internalio.WriterFunc(func(p []byte) (int, error) { writes++ return 0, assert.AnError })) - var stderr strings.Builder - underTest.SetErr(&stderr) assert.Same(t, assert.AnError, underTest.Execute()) assert.Equal(t, uint(1), writes, "Expected a single write to stdout") @@ -127,8 +125,8 @@ func newAirgapListImagesCmdWithConfig(t *testing.T, config string, args ...strin require.NoError(t, os.WriteFile(configFile, []byte(config), 0644)) out, err = new(strings.Builder), new(strings.Builder) - cmd := NewAirgapListImagesCmd() - cmd.SetArgs(append([]string{"--config=" + configFile}, args...)) + cmd := cmd.NewRootCmd() + cmd.SetArgs(append([]string{"airgap", "--config=" + configFile, "list-images"}, args...)) cmd.SetIn(iotest.ErrReader(errors.New("unexpected read from standard input"))) cmd.SetOut(out) cmd.SetErr(err) diff --git a/cmd/api/api.go b/cmd/api/api.go index d4e079e1cf8e..5961743f2adf 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -53,12 +53,13 @@ func NewAPICmd() *cobra.Command { cmd := &cobra.Command{ Use: "api", Short: "Run the controller API", + Args: cobra.NoArgs, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { logrus.SetOutput(cmd.OutOrStdout()) k0slog.SetInfoLevel() return config.CallParentPersistentPreRun(cmd, args) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/backup/backup_unix.go b/cmd/backup/backup_unix.go index 1db3fef39df5..dd80d8443c6c 100644 --- a/cmd/backup/backup_unix.go +++ b/cmd/backup/backup_unix.go @@ -42,11 +42,12 @@ func NewBackupCmd() *cobra.Command { cmd := &cobra.Command{ Use: "backup", Short: "Back-Up k0s configuration. Must be run as root (or with sudo)", + Args: cobra.NoArgs, PreRun: func(cmd *cobra.Command, args []string) { // ensure logs don't mess up output logrus.SetOutput(cmd.ErrOrStderr()) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err @@ -69,7 +70,7 @@ func NewBackupCmd() *cobra.Command { func (c *command) backup(savePath string, out io.Writer) error { if os.Geteuid() != 0 { - logrus.Fatal("this command must be run as root!") + return errors.New("this command must be run as root!") } if savePath != "-" && !dir.IsDirectory(savePath) { diff --git a/cmd/backup/backup_windows.go b/cmd/backup/backup_windows.go index 5c0293d8a62a..bd2a43661168 100644 --- a/cmd/backup/backup_windows.go +++ b/cmd/backup/backup_windows.go @@ -28,6 +28,7 @@ func NewBackupCmd() *cobra.Command { return &cobra.Command{ Use: "backup", Short: "Back-Up k0s configuration. Not supported on Windows OS", + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return errors.New("unsupported Operating System for this command") }, diff --git a/cmd/config/config.go b/cmd/config/config.go index 7b4a0224596d..d45ab38ba0e2 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -24,6 +24,8 @@ func NewConfigCmd() *cobra.Command { cmd := &cobra.Command{ Use: "config", Short: "Configuration related sub-commands", + Args: cobra.NoArgs, + Run: func(*cobra.Command, []string) { /* Enforce arg validation. */ }, } cmd.AddCommand(NewCreateCmd()) cmd.AddCommand(NewEditCmd()) @@ -31,3 +33,20 @@ func NewConfigCmd() *cobra.Command { cmd.AddCommand(NewValidateCmd()) return cmd } + +func reExecKubectl(cmd *cobra.Command, kubectlArgs ...string) error { + args := []string{"kubectl"} + if dataDir, err := cmd.Flags().GetString("data-dir"); err != nil { + return err + } else if dataDir != "" { + args = append(args, "--data-dir", dataDir) + } + + root := cmd.Root() + root.SetArgs(append(args, kubectlArgs...)) + + silenceErrors := root.SilenceErrors + defer func() { root.SilenceErrors = silenceErrors }() + root.SilenceErrors = true // So that errors aren't printed twice. + return root.Execute() +} diff --git a/cmd/config/config_test.go b/cmd/config/config_test.go new file mode 100644 index 000000000000..b8528a3ce743 --- /dev/null +++ b/cmd/config/config_test.go @@ -0,0 +1,64 @@ +/* +Copyright 2024 k0s authors + +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 config_test + +import ( + "errors" + "fmt" + "slices" + "strings" + "testing" + "testing/iotest" + + "github.com/k0sproject/k0s/cmd" + "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" +) + +func TestConfigCmd_RejectsUnknownCommands(t *testing.T) { + for _, cmds := range [][]string{ + {"config"}, + {"config", "create"}, + {"config", "edit"}, + {"config", "status"}, + {"config", "validate"}, + } { + subCommand := strings.Join(cmds, " ") + t.Run(subCommand, func(t *testing.T) { + var stdout, stderr strings.Builder + underTest := cmd.NewRootCmd() + + // Reset any "required" annotations on flags + if cmd, _, err := underTest.Find(cmds); assert.NoError(t, err) { + flags := cmd.Flags() + flags.VisitAll(func(flag *pflag.Flag) { + flag.Annotations = nil + }) + } + + underTest.SetArgs(slices.Concat(cmds, []string{"bogus"})) + underTest.SetIn(iotest.ErrReader(errors.New("unexpected read from standard input"))) + underTest.SetOut(&stdout) + underTest.SetErr(&stderr) + + msg := fmt.Sprintf(`unknown command "bogus" for "k0s %s"`, subCommand) + assert.ErrorContains(t, underTest.Execute(), msg) + assert.Equal(t, "Error: "+msg+"\n", stderr.String()) + assert.Empty(t, stdout.String()) + }) + } +} diff --git a/cmd/config/create.go b/cmd/config/create.go index 3d4e7030e36b..7f72e2d5747b 100644 --- a/cmd/config/create.go +++ b/cmd/config/create.go @@ -32,7 +32,8 @@ func NewCreateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "create", Short: "Output the default k0s configuration yaml to stdout", - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { config := v1beta1.DefaultClusterConfig() if !includeImages { config.Spec.Images = nil diff --git a/cmd/config/edit.go b/cmd/config/edit.go index df25652533e7..f03e61f8a13a 100644 --- a/cmd/config/edit.go +++ b/cmd/config/edit.go @@ -17,8 +17,6 @@ limitations under the License. package config import ( - "os" - "github.com/k0sproject/k0s/pkg/config" "github.com/spf13/cobra" @@ -28,13 +26,9 @@ func NewEditCmd() *cobra.Command { cmd := &cobra.Command{ Use: "edit", Short: "Launch the editor configured in your shell to edit k0s configuration", + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - opts, err := config.GetCmdOpts(cmd) - if err != nil { - return err - } - os.Args = []string{os.Args[0], "kubectl", "--data-dir", opts.K0sVars.DataDir, "-n", "kube-system", "edit", "clusterconfig", "k0s"} - return cmd.Execute() + return reExecKubectl(cmd, "-n", "kube-system", "edit", "clusterconfig", "k0s") }, } cmd.PersistentFlags().AddFlagSet(config.GetKubeCtlFlagSet()) diff --git a/cmd/config/status.go b/cmd/config/status.go index 7a188b8cf9d4..0bd9e69d6dc2 100644 --- a/cmd/config/status.go +++ b/cmd/config/status.go @@ -17,8 +17,6 @@ limitations under the License. package config import ( - "os" - "github.com/k0sproject/k0s/pkg/config" "github.com/spf13/cobra" @@ -30,16 +28,14 @@ func NewStatusCmd() *cobra.Command { cmd := &cobra.Command{ Use: "status", Short: "Display dynamic configuration reconciliation status", + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - opts, err := config.GetCmdOpts(cmd) - if err != nil { - return err - } - os.Args = []string{os.Args[0], "kubectl", "--data-dir", opts.K0sVars.DataDir, "-n", "kube-system", "get", "event", "--field-selector", "involvedObject.name=k0s"} + args := []string{"-n", "kube-system", "get", "event", "--field-selector", "involvedObject.name=k0s"} if outputFormat != "" { - os.Args = append(os.Args, "-o", outputFormat) + args = append(args, "-o", outputFormat) } - return cmd.Execute() + + return reExecKubectl(cmd, args...) }, } cmd.PersistentFlags().AddFlagSet(config.GetKubeCtlFlagSet()) diff --git a/cmd/config/validate.go b/cmd/config/validate.go index c0d9a4debfde..15cbdcc1e6dd 100644 --- a/cmd/config/validate.go +++ b/cmd/config/validate.go @@ -34,7 +34,8 @@ func NewValidateCmd() *cobra.Command { Short: "Validate k0s configuration", Long: `Example: k0s config validate --config path_to_config.yaml`, - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { var reader io.Reader // config.CfgFile is the global value holder for --config flag, set by cobra/pflag diff --git a/cmd/controller/controller.go b/cmd/controller/controller.go index 15226f20a9bf..d8373a46e004 100644 --- a/cmd/controller/controller.go +++ b/cmd/controller/controller.go @@ -80,6 +80,7 @@ func NewControllerCmd() *cobra.Command { or CLI flag: $ k0s controller --token-file [path_to_file] Note: Token can be passed either as a CLI argument or as a flag`, + Args: cobra.MaximumNArgs(1), PersistentPreRunE: func(cmd *cobra.Command, args []string) error { logrus.SetOutput(cmd.OutOrStdout()) k0slog.SetInfoLevel() diff --git a/cmd/etcd/etcd.go b/cmd/etcd/etcd.go index 7b14ae291216..f96a7564a979 100644 --- a/cmd/etcd/etcd.go +++ b/cmd/etcd/etcd.go @@ -30,6 +30,7 @@ func NewEtcdCmd() *cobra.Command { cmd := &cobra.Command{ Use: "etcd", Short: "Manage etcd cluster", + Args: cobra.NoArgs, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if err := config.CallParentPersistentPreRun(cmd, args); err != nil { return err @@ -51,6 +52,7 @@ func NewEtcdCmd() *cobra.Command { } return nil }, + Run: func(*cobra.Command, []string) { /* Enforce arg validation. */ }, } cmd.AddCommand(etcdLeaveCmd()) cmd.AddCommand(etcdListCmd()) diff --git a/cmd/etcd/leave.go b/cmd/etcd/leave.go index 2985ee2adf8c..19020b9382da 100644 --- a/cmd/etcd/leave.go +++ b/cmd/etcd/leave.go @@ -39,7 +39,7 @@ func etcdLeaveCmd() *cobra.Command { Use: "leave", Short: "Leave the etcd cluster, or remove a specific peer", Args: cobra.NoArgs, // accept peer address via flag, not via arg - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/etcd/list.go b/cmd/etcd/list.go index 6de0d8f8c355..61e7bb324f67 100644 --- a/cmd/etcd/list.go +++ b/cmd/etcd/list.go @@ -36,7 +36,7 @@ func etcdListCmd() *cobra.Command { // ensure logs don't mess up the output logrus.SetOutput(cmd.ErrOrStderr()) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/etcd/list_test.go b/cmd/etcd/list_test.go deleted file mode 100644 index f8d7d95a595d..000000000000 --- a/cmd/etcd/list_test.go +++ /dev/null @@ -1,32 +0,0 @@ -/* -Copyright 2024 k0s authors - -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 etcd - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestEtcdListCmd(t *testing.T) { - t.Run("rejects_args", func(t *testing.T) { - leaveCmd := etcdListCmd() - leaveCmd.SetArgs([]string{"bogus"}) - err := leaveCmd.Execute() - assert.ErrorContains(t, err, `unknown command "bogus" for "member-list"`) - }) -} diff --git a/cmd/install/controller.go b/cmd/install/controller.go index f3245ab02aaa..36e62c01b252 100644 --- a/cmd/install/controller.go +++ b/cmd/install/controller.go @@ -36,7 +36,8 @@ With the controller subcommand you can setup a single node cluster by running: k0s install controller --single `, - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/install/install.go b/cmd/install/install.go index b9310f2a82e6..11bd266faa30 100644 --- a/cmd/install/install.go +++ b/cmd/install/install.go @@ -40,6 +40,8 @@ func NewInstallCmd() *cobra.Command { cmd := &cobra.Command{ Use: "install", Short: "Install k0s on a brand-new system. Must be run as root (or with sudo)", + Args: cobra.NoArgs, + Run: func(*cobra.Command, []string) { /* Enforce arg validation. */ }, } cmd.AddCommand(installControllerCmd(&installFlags)) diff --git a/cmd/install/worker.go b/cmd/install/worker.go index f965ef6cad21..55c79cb7dc8a 100644 --- a/cmd/install/worker.go +++ b/cmd/install/worker.go @@ -30,7 +30,8 @@ func installWorkerCmd(installFlags *installFlags) *cobra.Command { All default values of worker command will be passed to the service stub unless overridden. Windows flags like "--api-server", "--cidr-range" and "--cluster-dns" will be ignored since install command doesn't yet support Windows services`, - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/kubeconfig/admin.go b/cmd/kubeconfig/admin.go index c94dfdc3ac4c..e96f5fcb921f 100644 --- a/cmd/kubeconfig/admin.go +++ b/cmd/kubeconfig/admin.go @@ -38,6 +38,7 @@ func kubeConfigAdminCmd() *cobra.Command { Example: ` $ k0s kubeconfig admin > ~/.kube/config $ export KUBECONFIG=~/.kube/config $ kubectl get nodes`, + Args: cobra.NoArgs, PreRun: func(cmd *cobra.Command, args []string) { // ensure logs don't mess up the output logrus.SetOutput(cmd.ErrOrStderr()) diff --git a/cmd/kubeconfig/kubeconfig.go b/cmd/kubeconfig/kubeconfig.go index 5781d7e9e3d9..888bdd9eb3c9 100644 --- a/cmd/kubeconfig/kubeconfig.go +++ b/cmd/kubeconfig/kubeconfig.go @@ -26,6 +26,8 @@ func NewKubeConfigCmd() *cobra.Command { cmd := &cobra.Command{ Use: "kubeconfig [command]", Short: "Create a kubeconfig file for a specified user", + Args: cobra.NoArgs, + Run: func(*cobra.Command, []string) { /* Enforce arg validation. */ }, } cmd.AddCommand(kubeconfigCreateCmd()) cmd.AddCommand(kubeConfigAdminCmd()) diff --git a/cmd/reset/reset.go b/cmd/reset/reset.go index 13253e803ec7..908c1540f7c5 100644 --- a/cmd/reset/reset.go +++ b/cmd/reset/reset.go @@ -17,6 +17,7 @@ limitations under the License. package reset import ( + "errors" "fmt" "os" @@ -34,7 +35,8 @@ func NewResetCmd() *cobra.Command { cmd := &cobra.Command{ Use: "reset", Short: "Uninstall k0s. Must be run as root (or with sudo)", - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err @@ -51,12 +53,12 @@ func NewResetCmd() *cobra.Command { func (c *command) reset() error { if os.Geteuid() != 0 { - logrus.Fatal("this command must be run as root!") + return errors.New("this command must be run as root!") } k0sStatus, _ := status.GetStatusInfo(c.K0sVars.StatusSocketPath) if k0sStatus != nil && k0sStatus.Pid != 0 { - logrus.Fatal("k0s seems to be running! please stop k0s before reset.") + return errors.New("k0s seems to be running! please stop k0s before reset.") } nodeCfg, err := c.K0sVars.NodeConfig() diff --git a/cmd/root.go b/cmd/root.go index 25e207d9abb0..a4b8f164c4c0 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -123,10 +123,10 @@ func NewRootCmd() *cobra.Command { func newDocsCmd() *cobra.Command { return &cobra.Command{ - Use: "docs ", + Use: "docs {markdown|man}", Short: "Generate k0s command documentation", ValidArgs: []string{"markdown", "man"}, - Args: cobra.ExactValidArgs(1), + Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs), RunE: func(cmd *cobra.Command, args []string) error { switch args[0] { case "markdown": @@ -149,7 +149,7 @@ func newDefaultConfigCmd() *cobra.Command { func newCompletionCmd() *cobra.Command { return &cobra.Command{ - Use: "completion ", + Use: "completion {bash|zsh|fish|powershell}", Short: "Generate completion script", Long: `To load completions: @@ -181,7 +181,7 @@ $ k0s completion fish > ~/.config/fish/completions/k0s.fish `, DisableFlagsInUseLine: true, ValidArgs: []string{"bash", "zsh", "fish", "powershell"}, - Args: cobra.ExactValidArgs(1), + Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs), RunE: func(cmd *cobra.Command, args []string) error { out := cmd.OutOrStdout() switch args[0] { diff --git a/cmd/root_test.go b/cmd/root_test.go index 14e3dd2ae66f..263c45c80ca8 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -18,13 +18,19 @@ package cmd_test import ( "bytes" + "errors" + "fmt" "io" "slices" + "strings" "testing" + "testing/iotest" "github.com/k0sproject/k0s/cmd" + "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestRootCmd_Flags ensures that no unwanted global flags have been registered @@ -66,3 +72,81 @@ func TestRootCmd_Flags(t *testing.T) { assert.Equal(t, expectedVisibleFlags, visibleFlags, "visible flags changed unexpectedly") assert.Equal(t, expectedHiddenFlags, hiddenFlags, "hidden flags changed unexpectedly") } + +func TestUnknownSubCommandsAreRejected(t *testing.T) { + commandsWithArguments := []string{ + "controller", + "kubeconfig create", + "restore", + "token invalidate", + "worker", + } + t.Cleanup(func() { + if !t.Failed() { + assert.Empty(t, commandsWithArguments, "Some sub-commands are listed unnecessarily") + } + }) + + shouldBeTested := func(t *testing.T, underTest *cobra.Command, subCommand string) { + if underTest.ValidArgs != nil { + t.Skipf("has ValidArgs: %v", underTest.ValidArgs) + } + if underTest.Deprecated != "" { + t.Skipf("is deprecated") + } + + if idx := slices.Index(commandsWithArguments, subCommand); idx >= 0 { + commandsWithArguments = slices.Delete(commandsWithArguments, idx, idx+1) + t.Skip("accepts arguments") + } + t.Cleanup(func() { + if t.Failed() { + t.Logf("If this sub-command accepts arguments, include %q in the above list", subCommand) + } + }) + } + + var testCommand func(underTest *cobra.Command, args []string) func(t *testing.T) + testCommand = func(underTest *cobra.Command, args []string) func(t *testing.T) { + return func(t *testing.T) { + for _, cmd := range underTest.Commands() { + name, _, _ := strings.Cut(cmd.Use, " ") + require.NotEmpty(t, name) + t.Run(name, testCommand(cmd, slices.Concat(args, []string{name}))) + } + + subCommand := strings.Join(args, " ") + shouldBeTested(t, underTest, subCommand) + + var stdout, stderr strings.Builder + // Reset any "required" annotations on flags + underTest.Flags().VisitAll(func(flag *pflag.Flag) { + flag.Annotations = nil + }) + + root := cmd.NewRootCmd() + + root.SetArgs(slices.Concat(args, []string{"bogus"})) + root.SetIn(iotest.ErrReader(errors.New("unexpected read from standard input"))) + root.SetOut(&stdout) + root.SetErr(&stderr) + + msg := fmt.Sprintf(`unknown command "bogus" for "k0s %s"`, subCommand) + assert.ErrorContains(t, root.Execute(), msg) + assert.Equal(t, "Error: "+msg+"\n", stderr.String()) + assert.Empty(t, stdout.String()) + } + } + + underTest := cmd.NewRootCmd() + for _, cmd := range underTest.Commands() { + name, _, _ := strings.Cut(cmd.Use, " ") + require.NotEmpty(t, name) + + switch name { + case "ctr", "kubectl": // Don't test embedded sub-commands + default: + t.Run(name, testCommand(cmd, []string{name})) + } + } +} diff --git a/cmd/start/start.go b/cmd/start/start.go index b07c5aca1bb9..c0e511e04bb8 100644 --- a/cmd/start/start.go +++ b/cmd/start/start.go @@ -30,7 +30,8 @@ func NewStartCmd() *cobra.Command { return &cobra.Command{ Use: "start", Short: "Start the k0s service configured on this host. Must be run as root (or with sudo)", - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { if os.Geteuid() != 0 { return errors.New("this command must be run as root") } diff --git a/cmd/status/status.go b/cmd/status/status.go index 25c9aac9141c..f797b6ebd033 100644 --- a/cmd/status/status.go +++ b/cmd/status/status.go @@ -35,7 +35,8 @@ func NewStatusCmd() *cobra.Command { Use: "status", Short: "Get k0s instance status information", Example: `The command will return information about system init, PID, k0s role, kubeconfig and similar.`, - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err @@ -66,7 +67,8 @@ func NewStatusSubCmdComponents() *cobra.Command { Use: "components", Short: "Get k0s instance component status information", Example: `The command will return information about k0s components.`, - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/stop/stop.go b/cmd/stop/stop.go index 57c51ff09a24..69429b9d26c0 100644 --- a/cmd/stop/stop.go +++ b/cmd/stop/stop.go @@ -30,7 +30,8 @@ func NewStopCmd() *cobra.Command { return &cobra.Command{ Use: "stop", Short: "Stop the k0s service configured on this host. Must be run as root (or with sudo)", - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { if os.Geteuid() != 0 { return errors.New("this command must be run as root") } diff --git a/cmd/sysinfo/sysinfo.go b/cmd/sysinfo/sysinfo.go index 70671d9e972e..3ac0e433e38e 100644 --- a/cmd/sysinfo/sysinfo.go +++ b/cmd/sysinfo/sysinfo.go @@ -38,7 +38,8 @@ func NewSysinfoCmd() *cobra.Command { Use: "sysinfo", Short: "Display system information", Long: `Runs k0s's pre-flight checks and issues the results to stdout.`, - RunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { sysinfoSpec.AddDebugProbes = true probes := sysinfoSpec.NewSysinfoProbes() out := cmd.OutOrStdout() diff --git a/cmd/token/create.go b/cmd/token/create.go index 3f5a8fb75e09..05e24e45ae2c 100644 --- a/cmd/token/create.go +++ b/cmd/token/create.go @@ -44,10 +44,11 @@ func tokenCreateCmd() *cobra.Command { Example: `k0s token create --role worker --expiry 100h //sets expiration time to 100 hours k0s token create --role worker --expiry 10m //sets expiration time to 10 minutes `, - PreRunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + PreRunE: func(cmd *cobra.Command, _ []string) error { return checkTokenRole(createTokenRole) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/token/invalidate.go b/cmd/token/invalidate.go index e7a301b22934..8fe6ff68829a 100644 --- a/cmd/token/invalidate.go +++ b/cmd/token/invalidate.go @@ -17,7 +17,6 @@ limitations under the License. package token import ( - "errors" "fmt" "github.com/k0sproject/k0s/pkg/config" @@ -28,17 +27,15 @@ import ( func tokenInvalidateCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "invalidate", + Use: "invalidate join-token...", Short: "Invalidates existing join token", Example: "k0s token invalidate xyz123", + Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err } - if len(args) < 1 { - return errors.New("invalidate requires at least one token ID to invalidate") - } manager, err := token.NewManager(opts.K0sVars.AdminKubeConfigPath) if err != nil { return err diff --git a/cmd/token/list.go b/cmd/token/list.go index d6e4c1448fad..493d8d6318f7 100644 --- a/cmd/token/list.go +++ b/cmd/token/list.go @@ -33,10 +33,11 @@ func tokenListCmd() *cobra.Command { Use: "list", Short: "List join tokens", Example: `k0s token list --role worker // list worker tokens`, - PreRunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + PreRunE: func(cmd *cobra.Command, _ []string) error { return checkTokenRole(listTokenRole) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { opts, err := config.GetCmdOpts(cmd) if err != nil { return err diff --git a/cmd/token/preshared.go b/cmd/token/preshared.go index de1e2329abb6..8bb62d778b70 100644 --- a/cmd/token/preshared.go +++ b/cmd/token/preshared.go @@ -50,7 +50,8 @@ func preSharedCmd() *cobra.Command { Use: "pre-shared", Short: "Generates token and secret and stores them as a files", Example: `k0s token pre-shared --role worker --cert //ca.crt --url https://:/`, - PreRunE: func(cmd *cobra.Command, args []string) error { + Args: cobra.NoArgs, + PreRunE: func(cmd *cobra.Command, _ []string) error { err := checkTokenRole(preSharedRole) if err != nil { return err @@ -64,7 +65,7 @@ func preSharedCmd() *cobra.Command { return nil }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { t, err := createSecret(preSharedRole, validity, outDir) if err != nil { return err diff --git a/cmd/token/token.go b/cmd/token/token.go index ba213f7cc191..dee7086113d7 100644 --- a/cmd/token/token.go +++ b/cmd/token/token.go @@ -28,6 +28,8 @@ func NewTokenCmd() *cobra.Command { cmd := &cobra.Command{ Use: "token", Short: "Manage join tokens", + Args: cobra.NoArgs, + Run: func(*cobra.Command, []string) { /* Enforce arg validation. */ }, } cmd.AddCommand(tokenCreateCmd()) diff --git a/cmd/validate/validate.go b/cmd/validate/validate.go index a550a66e2f23..0d2e266aef80 100644 --- a/cmd/validate/validate.go +++ b/cmd/validate/validate.go @@ -25,9 +25,10 @@ import ( // TODO deprecated, remove when appropriate func NewValidateCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "validate", - Short: "Validation related sub-commands", - Hidden: true, + Use: "validate", + Short: "Validation related sub-commands", + Hidden: true, + Deprecated: "use 'k0s config validate' instead", } cmd.AddCommand(newConfigCmd()) return cmd diff --git a/cmd/version/version.go b/cmd/version/version.go index f12041dc22e6..99d618c4ae5f 100644 --- a/cmd/version/version.go +++ b/cmd/version/version.go @@ -35,8 +35,8 @@ func NewVersionCmd() *cobra.Command { cmd := &cobra.Command{ Use: "version", Short: "Print the k0s version", - - Run: func(cmd *cobra.Command, args []string) { + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, _ []string) { info := versionInfo{ Version: build.Version, Runc: build.RuncVersion, diff --git a/cmd/worker/worker.go b/cmd/worker/worker.go index 6a22aa8b2981..13c03c0c36ad 100644 --- a/cmd/worker/worker.go +++ b/cmd/worker/worker.go @@ -58,6 +58,7 @@ func NewWorkerCmd() *cobra.Command { or CLI flag: $ k0s worker --token-file [path_to_file] Note: Token can be passed either as a CLI argument or as a flag`, + Args: cobra.MaximumNArgs(1), PersistentPreRunE: func(cmd *cobra.Command, args []string) error { logrus.SetOutput(cmd.OutOrStdout()) k0slog.SetInfoLevel() diff --git a/pkg/autopilot/controller/root_worker.go b/pkg/autopilot/controller/root_worker.go index abc15fd1020b..8bb43c670700 100644 --- a/pkg/autopilot/controller/root_worker.go +++ b/pkg/autopilot/controller/root_worker.go @@ -82,7 +82,7 @@ func (w *rootWorker) Run(ctx context.Context) error { logger.WithError(err).Debugf("Failed to start controller manager in attempt #%d, retrying after backoff", attempt+1) }), ); err != nil { - logger.WithError(err).Fatal("unable to start controller manager") + return fmt.Errorf("unable to start controller manager: %w", err) } // In some cases, we need to wait on the worker side until controller deploys all autopilot CRDs diff --git a/pkg/backup/manager_unix.go b/pkg/backup/manager_unix.go index e18604c76b53..3bf8dd26797c 100644 --- a/pkg/backup/manager_unix.go +++ b/pkg/backup/manager_unix.go @@ -136,7 +136,7 @@ func (bm Manager) save(backupFileName string, assets []string) error { // Create the archive and write the output to the "out" Writer err = createArchive(out, assets, bm.dataDir) if err != nil { - logrus.Fatalf("error creating archive: %v", err) + return fmt.Errorf("error creating archive: %w", err) } destinationFile := filepath.Join(bm.tmpDir, backupFileName)