Merge pull request #1937 from Adirio/enhance-root-cmd

✨ Build the command and use it to report user errors

Author: k8s-ci-robot

pkg/cli/cli.go

@@ -18,7 +18,6 @@ package cli
 
 import (
 	"fmt"
-	"log"
 	"os"
 	"strings"
 
@@ -95,51 +94,33 @@ type cli struct { //nolint:maligned
 	// A filtered set of plugins that should be used by command constructors.
 	resolvedPlugins []plugin.Plugin
 
-	// Whether some generic help should be printed, i.e. if the binary
-	// was invoked outside of a project with incorrect flags or -h|--help.
-	doHelp bool
-
 	// Root command.
 	cmd *cobra.Command
 }
 
 // New creates a new cli instance.
+// Developer errors (e.g. not registering any plugins, extra commands with conflicting names) return an error
+// while user errors (e.g. errors while parsing flags, unresolvable plugins) create a command which return the error.
 func New(opts ...Option) (CLI, error) {
 	// Create the CLI.
 	c, err := newCLI(opts...)
 	if err != nil {
 		return nil, err
 	}
 
-	// Get project version and plugin keys.
-	if err := c.getInfo(); err != nil {
-		return nil, err
-	}
-
-	// Resolve plugins for project version and plugin keys.
-	if err := c.resolve(); err != nil {
-		return nil, err
+	// Build the cmd tree.
+	if err := c.buildCmd(); err != nil {
+		c.cmd.RunE = errCmdFunc(err)
+		return c, nil
 	}
 
-	// Build the root command.
-	c.cmd = c.buildRootCmd()
-
 	// Add extra commands injected by options.
-	for _, cmd := range c.extraCommands {
-		for _, subCmd := range c.cmd.Commands() {
-			if cmd.Name() == subCmd.Name() {
-				return nil, fmt.Errorf("command %q already exists", cmd.Name())
-			}
-		}
-		c.cmd.AddCommand(cmd)
+	if err := c.addExtraCommands(); err != nil {
+		return nil, err
 	}
 
 	// Write deprecation notices after all commands have been constructed.
-	for _, p := range c.resolvedPlugins {
-		if d, isDeprecated := p.(plugin.Deprecated); isDeprecated {
-			fmt.Printf(noticeColor, fmt.Sprintf(deprecationFmt, d.DeprecationWarning()))
-		}
-	}
+	c.printDeprecationWarnings()
 
 	return c, nil
 }
@@ -166,37 +147,47 @@ func newCLI(opts ...Option) (*cli, error) {
 }
 
 // getInfoFromFlags obtains the project version and plugin keys from flags.
-func (c *cli) getInfoFromFlags() (string, []string) {
+func (c *cli) getInfoFromFlags() (string, []string, error) {
 	// Partially parse the command line arguments
-	fs := pflag.NewFlagSet("base", pflag.ExitOnError)
+	fs := pflag.NewFlagSet("base", pflag.ContinueOnError)
+
+	// Load the base command global flags
+	fs.AddFlagSet(c.cmd.PersistentFlags())
 
 	// Omit unknown flags to avoid parsing errors
 	fs.ParseErrorsWhitelist = pflag.ParseErrorsWhitelist{UnknownFlags: true}
 
-	// Define the flags needed for plugin resolution
-	var projectVersion, plugins string
-	var help bool
-	fs.StringVar(&projectVersion, projectVersionFlag, "", "project version")
-	fs.StringVar(&plugins, pluginsFlag, "", "plugins to run")
-	fs.BoolVarP(&help, "help", "h", false, "help flag")
+	// FlagSet special cases --help and -h, so we need to create a dummy flag with these 2 values to prevent the default
+	// behavior (printing the usage of this FlagSet) as we want to print the usage message of the underlying command.
+	fs.BoolP("help", "h", false, fmt.Sprintf("help for %s", c.commandName))
 
 	// Parse the arguments
-	err := fs.Parse(os.Args[1:])
+	if err := fs.Parse(os.Args[1:]); err != nil {
+		return "", []string{}, err
+	}
 
-	// User needs *generic* help if args are incorrect or --help is set and
-	// --project-version is not set. Plugin-specific help is given if a
-	// plugin.Context is updated, which does not require this field.
-	c.doHelp = err != nil || help && !fs.Lookup(projectVersionFlag).Changed
+	// Define the flags needed for plugin resolution
+	var (
+		projectVersion string
+		plugins        []string
+		err            error
+	)
+	// GetXxxxx methods will not yield errors because we know for certain these flags exist and types match.
+	projectVersion, err = fs.GetString(projectVersionFlag)
+	if err != nil {
+		return "", []string{}, err
+	}
+	plugins, err = fs.GetStringSlice(pluginsFlag)
+	if err != nil {
+		return "", []string{}, err
+	}
 
-	// Split the comma-separated plugins
-	var pluginSet []string
-	if plugins != "" {
-		for _, p := range strings.Split(plugins, ",") {
-			pluginSet = append(pluginSet, strings.TrimSpace(p))
-		}
+	// Remove leading and trailing spaces
+	for i, key := range plugins {
+		plugins[i] = strings.TrimSpace(key)
 	}
 
-	return projectVersion, pluginSet
+	return projectVersion, plugins, nil
 }
 
 // getInfoFromConfigFile obtains the project version and plugin keys from the project config file.
@@ -293,14 +284,16 @@ func (c cli) resolveFlagsAndConfigFileConflicts(
 // getInfo obtains the project version and plugin keys resolving conflicts among flags and the project config file.
 func (c *cli) getInfo() error {
 	// Get project version and plugin info from flags
-	flagProjectVersion, flagPlugins := c.getInfoFromFlags()
+	flagProjectVersion, flagPlugins, err := c.getInfoFromFlags()
+	if err != nil {
+		return err
+	}
 	// Get project version and plugin info from project configuration file
 	cfgProjectVersion, cfgPlugins, _ := getInfoFromConfigFile()
 	// We discard the error because not being able to read a project configuration file
 	// is not fatal for some commands. The ones that require it need to check its existence.
 
 	// Resolve project version and plugin keys
-	var err error
 	c.projectVersion, c.pluginKeys, err = c.resolveFlagsAndConfigFileConflicts(
 		flagProjectVersion, cfgProjectVersion, flagPlugins, cfgPlugins,
 	)
@@ -399,15 +392,13 @@ func (c *cli) resolve() error {
 	return nil
 }
 
-// buildRootCmd returns a root command with a subcommand tree reflecting the
+// addSubcommands returns a root command with a subcommand tree reflecting the
 // current project's state.
-func (c cli) buildRootCmd() *cobra.Command {
-	rootCmd := c.defaultCommand()
-
+func (c *cli) addSubcommands() {
 	// kubebuilder completion
 	// Only add completion if requested
 	if c.completionCommand {
-		rootCmd.AddCommand(c.newCompletionCmd())
+		c.cmd.AddCommand(c.newCompletionCmd())
 	}
 
 	// kubebuilder create
@@ -416,76 +407,61 @@ func (c cli) buildRootCmd() *cobra.Command {
 	createCmd.AddCommand(c.newCreateAPICmd())
 	createCmd.AddCommand(c.newCreateWebhookCmd())
 	if createCmd.HasSubCommands() {
-		rootCmd.AddCommand(createCmd)
+		c.cmd.AddCommand(createCmd)
 	}
 
 	// kubebuilder edit
-	rootCmd.AddCommand(c.newEditCmd())
+	c.cmd.AddCommand(c.newEditCmd())
 
 	// kubebuilder init
-	rootCmd.AddCommand(c.newInitCmd())
+	c.cmd.AddCommand(c.newInitCmd())
 
 	// kubebuilder version
 	// Only add version if a version string was provided
 	if c.version != "" {
-		rootCmd.AddCommand(c.newVersionCmd())
+		c.cmd.AddCommand(c.newVersionCmd())
 	}
-
-	return rootCmd
 }
 
-// defaultCommand returns the root command without its subcommands.
-func (c cli) defaultCommand() *cobra.Command {
-	return &cobra.Command{
-		Use:   c.commandName,
-		Short: "Development kit for building Kubernetes extensions and tools.",
-		Long: fmt.Sprintf(`Development kit for building Kubernetes extensions and tools.
-
-Provides libraries and tools to create new projects, APIs and controllers.
-Includes tools for packaging artifacts into an installer container.
-
-Typical project lifecycle:
-
-- initialize a project:
-
-  %[1]s init --domain example.com --license apache2 --owner "The Kubernetes authors"
+// buildCmd creates the underlying cobra command and stores it internally.
+func (c *cli) buildCmd() error {
+	c.cmd = c.newRootCmd()
 
-- create one or more a new resource APIs and add your code to them:
-
-  %[1]s create api --group <group> --version <version> --kind <Kind>
-
-Create resource will prompt the user for if it should scaffold the Resource and / or Controller. To only
-scaffold a Controller for an existing Resource, select "n" for Resource. To only define
-the schema for a Resource without writing a Controller, select "n" for Controller.
-
-After the scaffold is written, api will run make on the project.
-`,
-			c.commandName),
-		Example: fmt.Sprintf(`
-  # Initialize your project
-  %[1]s init --domain example.com --license apache2 --owner "The Kubernetes authors"
-
-  # Create a frigates API with Group: ship, Version: v1beta1 and Kind: Frigate
-  %[1]s create api --group ship --version v1beta1 --kind Frigate
+	// Get project version and plugin keys.
+	if err := c.getInfo(); err != nil {
+		return err
+	}
 
-  # Edit the API Scheme
-  nano api/v1beta1/frigate_types.go
+	// Resolve plugins for project version and plugin keys.
+	if err := c.resolve(); err != nil {
+		return err
+	}
 
-  # Edit the Controller
-  nano controllers/frigate_controller.go
+	// Add the subcommands
+	c.addSubcommands()
 
-  # Install CRDs into the Kubernetes cluster using kubectl apply
-  make install
+	return nil
+}
 
-  # Regenerate code and run against the Kubernetes cluster configured by ~/.kube/config
-  make run
-`,
-			c.commandName),
-		Run: func(cmd *cobra.Command, args []string) {
-			if err := cmd.Help(); err != nil {
-				log.Fatal(err)
+// addExtraCommands adds the additional commands.
+func (c *cli) addExtraCommands() error {
+	for _, cmd := range c.extraCommands {
+		for _, subCmd := range c.cmd.Commands() {
+			if cmd.Name() == subCmd.Name() {
+				return fmt.Errorf("command %q already exists", cmd.Name())
 			}
-		},
+		}
+		c.cmd.AddCommand(cmd)
+	}
+	return nil
+}
+
+// printDeprecationWarnings prints the deprecation warnings of the resolved plugins.
+func (c cli) printDeprecationWarnings() {
+	for _, p := range c.resolvedPlugins {
+		if d, isDeprecated := p.(plugin.Deprecated); isDeprecated {
+			fmt.Printf(noticeColor, fmt.Sprintf(deprecationFmt, d.DeprecationWarning()))
+		}
 	}
 }