Merge pull request #127096 from neolit123/1.32-fix-unknown-phase-error

kubeadm: better error handling for unknown phases and commands

cmd/kubeadm/app/cmd/certs.go

@@ -104,9 +104,9 @@ func newCmdCertsUtility(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "certs",
 		Aliases: []string{"certificates"},
-		Short:   "Commands related to handling kubernetes certificates",
+		Short:   "Commands related to handling Kubernetes certificates",
-		Run:     cmdutil.SubCmdRun(),
 	}
+	cmdutil.RequireSubcommand(cmd)
 
 	cmd.AddCommand(newCmdCertsRenewal(out))
 	cmd.AddCommand(newCmdCertsExpiration(out, kubeadmconstants.KubernetesDir))
@@ -215,9 +215,8 @@ func newCmdCertsRenewal(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "renew",
 		Short: "Renew certificates for a Kubernetes cluster",
-		Long:  cmdutil.MacroCommandLongDescription,
-		Run:   cmdutil.SubCmdRun(),
 	}
+	cmdutil.RequireSubcommand(cmd)
 
 	cmd.AddCommand(getRenewSubCommands(out, kubeadmconstants.KubernetesDir)...)
 

cmd/kubeadm/app/cmd/config.go

@@ -62,13 +62,8 @@ func newCmdConfig(out io.Writer) *cobra.Command {
 			initialized your cluster using kubeadm v1.7.x or lower, you must use the 'kubeadm init phase upload-config' command to
 			create this ConfigMap. This is required so that 'kubeadm upgrade' can configure your upgraded cluster correctly.
 		`), metav1.NamespaceSystem, constants.KubeadmConfigConfigMap),
-		// Without this callback, if a user runs just the "upload"
-		// command without a subcommand, or with an invalid subcommand,
-		// cobra will print usage information, but still exit cleanly.
-		// We want to return an error code in these cases so that the
-		// user knows that their command was invalid.
-		Run: cmdutil.SubCmdRun(),
 	}
+	cmdutil.RequireSubcommand(cmd)
 
 	options.AddKubeConfigFlag(cmd.PersistentFlags(), &kubeConfigFile)
 
@@ -88,8 +83,8 @@ func newCmdConfigPrint(out io.Writer) *cobra.Command {
 		Long: dedent.Dedent(`
 			This command prints configurations for subcommands provided.
 			For details, see: https://pkg.go.dev/k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm#section-directories`),
-		Run: cmdutil.SubCmdRun(),
 	}
+	cmdutil.RequireSubcommand(cmd)
 	cmd.AddCommand(newCmdConfigPrintInitDefaults(out))
 	cmd.AddCommand(newCmdConfigPrintJoinDefaults(out))
 	cmd.AddCommand(newCmdConfigPrintResetDefaults(out))
@@ -360,8 +355,8 @@ func newCmdConfigImages(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "images",
 		Short: "Interact with container images used by kubeadm",
-		Run:   cmdutil.SubCmdRun(),
 	}
+	cmdutil.RequireSubcommand(cmd)
 	cmd.AddCommand(newCmdConfigImagesList(out, nil))
 	cmd.AddCommand(newCmdConfigImagesPull())
 	return cmd

cmd/kubeadm/app/cmd/phases/init/addons.go

@@ -56,7 +56,6 @@ func NewAddonPhase() workflow.Phase {
 	return workflow.Phase{
 		Name:  "addon",
 		Short: "Install required addons for passing conformance tests",
-		Long:  cmdutil.MacroCommandLongDescription,
 		Phases: []workflow.Phase{
 			{
 				Name:           "all",

cmd/kubeadm/app/cmd/phases/init/certs.go

@@ -57,7 +57,6 @@ func NewCertsPhase() workflow.Phase {
 		Short:  "Certificate generation",
 		Phases: newCertSubPhases(),
 		Run:    runCerts,
-		Long:   cmdutil.MacroCommandLongDescription,
 	}
 }
 

cmd/kubeadm/app/cmd/phases/init/controlplane.go

@@ -66,7 +66,6 @@ func NewControlPlanePhase() workflow.Phase {
 	phase := workflow.Phase{
 		Name:  "control-plane",
 		Short: "Generate all static Pod manifest files necessary to establish the control plane",
-		Long:  cmdutil.MacroCommandLongDescription,
 		Phases: []workflow.Phase{
 			{
 				Name:           "all",

cmd/kubeadm/app/cmd/phases/init/etcd.go

@@ -47,7 +47,6 @@ func NewEtcdPhase() workflow.Phase {
 	phase := workflow.Phase{
 		Name:  "etcd",
 		Short: "Generate static Pod manifest file for local etcd",
-		Long:  cmdutil.MacroCommandLongDescription,
 		Phases: []workflow.Phase{
 			newEtcdLocalSubPhase(),
 		},

cmd/kubeadm/app/cmd/phases/init/kubeconfig.go

@@ -74,7 +74,6 @@ func NewKubeConfigPhase() workflow.Phase {
 	return workflow.Phase{
 		Name:  "kubeconfig",
 		Short: "Generate all kubeconfig files necessary to establish the control plane and the admin kubeconfig file",
-		Long:  cmdutil.MacroCommandLongDescription,
 		Phases: []workflow.Phase{
 			{
 				Name:           "all",

cmd/kubeadm/app/cmd/phases/init/uploadconfig.go

@@ -65,7 +65,6 @@ func NewUploadConfigPhase() workflow.Phase {
 		Name:    "upload-config",
 		Aliases: []string{"uploadconfig"},
 		Short:   "Upload the kubeadm and kubelet configuration to a ConfigMap",
-		Long:    cmdutil.MacroCommandLongDescription,
 		Phases: []workflow.Phase{
 			{
 				Name:           "all",

cmd/kubeadm/app/cmd/phases/workflow/runner.go

@@ -23,6 +23,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+
+	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 )
 
 // phaseSeparator defines the separator to be used when concatenating nested
@@ -332,8 +334,9 @@ func (e *Runner) BindToCommand(cmd *cobra.Command) {
 	// adds the phases subcommand
 	phaseCommand := &cobra.Command{
 		Use:   "phase",
-		Short: fmt.Sprintf("Use this command to invoke single phase of the %s workflow", cmd.Name()),
+		Short: fmt.Sprintf("Use this command to invoke single phase of the %q workflow", cmd.Name()),
 	}
+	cmdutil.RequireSubcommand(phaseCommand)
 
 	cmd.AddCommand(phaseCommand)
 
@@ -363,7 +366,8 @@ func (e *Runner) BindToCommand(cmd *cobra.Command) {
 			RunE: func(cmd *cobra.Command, args []string) error {
 				// if the phase has subphases, print the help and exits
 				if len(p.Phases) > 0 {
-					return cmd.Help()
+					_ = cmd.Help()
+					return cmdutil.ErrorSubcommandRequired
 				}
 
 				// overrides the command triggering the Runner using the phaseCmd

cmd/kubeadm/app/cmd/token.go

@@ -77,14 +77,8 @@ func newCmdToken(out io.Writer, errW io.Writer) *cobra.Command {
 			You can read more about bootstrap tokens here:
 			  https://kubernetes.io/docs/admin/bootstrap-tokens/
 		`),
-
-		// Without this callback, if a user runs just the "token"
-		// command without a subcommand, or with an invalid subcommand,
-		// cobra will print usage information, but still exit cleanly.
-		// We want to return an error code in these cases so that the
-		// user knows that their command was invalid.
-		Run: cmdutil.SubCmdRun(),
 	}
+	cmdutil.RequireSubcommand(tokenCmd)
 
 	options.AddKubeConfigFlag(tokenCmd.PersistentFlags(), &kubeConfigFile)
 	tokenCmd.PersistentFlags().BoolVar(&dryRun,

cmd/kubeadm/app/cmd/upgrade/upgrade.go

@@ -56,8 +56,8 @@ func NewCmdUpgrade(out io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "upgrade",
 		Short: "Upgrade your cluster smoothly to a newer version with this command",
-		Run:   cmdutil.SubCmdRun(),
 	}
+	cmdutil.RequireSubcommand(cmd)
 
 	cmd.AddCommand(newCmdApply(flags))
 	cmd.AddCommand(newCmdPlan(flags))

cmd/kubeadm/app/cmd/util/cmdutil.go

@@ -34,27 +34,23 @@ import (
 
 	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/options"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient"
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
 )
 
-// SubCmdRun returns a function that handles a case where a subcommand must be specified
+// ErrorSubcommandRequired is an error returned when a parent command cannot be executed.
-// Without this callback, if a user runs just the command without a subcommand,
+// It starts with a new line so that it's separated from previous information like a Help() screen.
-// or with an invalid subcommand, cobra will print usage information, but still exit cleanly.
+var ErrorSubcommandRequired = errors.New("\nerror: subcommand is required")
-func SubCmdRun() func(c *cobra.Command, args []string) {
-	return func(c *cobra.Command, args []string) {
-		if len(args) > 0 {
-			kubeadmutil.CheckErr(usageErrorf(c, "invalid subcommand %q", strings.Join(args, " ")))
-		}
-		c.Help()
-		kubeadmutil.CheckErr(kubeadmutil.ErrExit)
-	}
-}
 
-func usageErrorf(c *cobra.Command, format string, args ...interface{}) error {
+// RequireSubcommand can be used to set an empty Run function and NoArgs on a Cobra command.
-	msg := fmt.Sprintf(format, args...)
+// This handles a case where a subcommand must be specified for a parent command 'c'.
-	return errors.Errorf("%s\nSee '%s -h' for help and examples", msg, c.CommandPath())
+// If no subcommand is specified the CLI exist with an error.
+func RequireSubcommand(c *cobra.Command) {
+	c.RunE = func(c *cobra.Command, args []string) error {
+		_ = c.Help()
+		return ErrorSubcommandRequired
+	}
+	c.Args = cobra.NoArgs
 }
 
 // ValidateExactArgNumber validates that the required top-level arguments are specified

cmd/kubeadm/app/cmd/util/documentation.go

@@ -25,11 +25,6 @@ var (
 	AlphaDisclaimer = `
 		Alpha Disclaimer: this command is currently alpha.
 	`
-
-	// MacroCommandLongDescription provide a standard description for "macro" commands
-	MacroCommandLongDescription = LongDesc(`
-		This command is not meant to be run on its own. See list of available subcommands.
-	`)
 )
 
 // LongDesc is designed to help with producing better long command line descriptions in code.

cmd/kubeadm/app/util/error.go

@@ -38,8 +38,6 @@ const (
 )
 
 var (
-	// ErrInvalidSubCommandMsg is an error message returned on invalid subcommands
-	ErrInvalidSubCommandMsg = "invalid subcommand"
 	// ErrExit is an error returned when kubeadm is about to exit
 	ErrExit = errors.New("exit")
 )
@@ -80,10 +78,10 @@ func checkErr(err error, handleErr func(string, int)) {
 	}
 
 	msg := fmt.Sprintf("%s\nTo see the stack trace of this error execute with --v=5 or higher", err.Error())
-	// check if the verbosity level in klog is high enough and print a stack trace.
+	// Check if the verbosity level in klog is high enough and print a stack trace.
 	f := flag.CommandLine.Lookup("v")
 	if f != nil {
-		// assume that the "v" flag contains a parseable Int32 as per klog's "Level" type alias,
+		// Assume that the "v" flag contains a parsable Int32 as per klog's "Level" type alias,
 		// thus no error from ParseInt is handled here.
 		if v, e := strconv.ParseInt(f.Value.String(), 10, 32); e == nil {
 			// https://git.k8s.io/community/contributors/devel/sig-instrumentation/logging.md
@@ -97,15 +95,12 @@ func checkErr(err error, handleErr func(string, int)) {
 	switch {
 	case err == ErrExit:
 		handleErr("", DefaultErrorExitCode)
-	case strings.Contains(err.Error(), ErrInvalidSubCommandMsg):
-		handleErr(err.Error(), DefaultErrorExitCode)
 	default:
 		switch err.(type) {
 		case preflightError:
 			handleErr(msg, PreFlightExitCode)
 		case errorsutil.Aggregate:
 			handleErr(msg, ValidationExitCode)
-
 		default:
 			handleErr(msg, DefaultErrorExitCode)
 		}