From 0f2c7d53905e0af6342943f20974727625928d9d Mon Sep 17 00:00:00 2001 From: Adrian Orive Date: Tue, 12 Jan 2021 13:57:06 +0100 Subject: [PATCH] Build the command and use it to report user errors so that the command help message is printed Signed-off-by: Adrian Orive --- pkg/cli/cli.go | 160 +++++++++++++++++--------------------------- pkg/cli/cli_test.go | 46 +++++++++---- pkg/cli/init.go | 4 -- pkg/cli/root.go | 116 ++++++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+), 114 deletions(-) create mode 100644 pkg/cli/root.go diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index 8a700741f81..a104ca826af 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -18,7 +18,6 @@ package cli import ( "fmt" - "log" "os" "strings" @@ -95,15 +94,13 @@ 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...) @@ -111,35 +108,19 @@ func New(opts ...Option) (CLI, error) { return nil, err } - // Get project version and plugin keys. - if err := c.getInfo(); err != nil { - return nil, err + // Build the cmd tree. + if err := c.buildCmd(); err != nil { + c.cmd.RunE = errCmdFunc(err) + return c, nil } - // Resolve plugins for project version and plugin keys. - if err := c.resolve(); err != nil { - return nil, err - } - - // 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,27 +147,25 @@ 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) // 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:]) - - // 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 + if err := fs.Parse(os.Args[1:]); err != nil { + return "", []string{}, err + } // Split the comma-separated plugins var pluginSet []string @@ -196,7 +175,7 @@ func (c *cli) getInfoFromFlags() (string, []string) { } } - return projectVersion, pluginSet + return projectVersion, pluginSet, nil } // getInfoFromConfigFile obtains the project version and plugin keys from the project config file. @@ -293,14 +272,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 +380,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 +395,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. +// buildCmd creates the underlying cobra command and stores it internally. +func (c *cli) buildCmd() error { + c.cmd = c.newRootCmd() -Typical project lifecycle: - -- initialize a project: - - %[1]s init --domain example.com --license apache2 --owner "The Kubernetes authors" - -- create one or more a new resource APIs and add your code to them: - - %[1]s create api --group --version --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())) + } } } diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index da76194bc6d..ab7acb68909 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -50,6 +50,10 @@ func setFlag(flag, value string) { os.Args = append(os.Args, "subcommand", "--"+flag, value) } +func setBoolFlag(flag string) { + os.Args = append(os.Args, "subcommand", "--"+flag) +} + // nolint:unparam func setProjectVersionFlag(value string) { setFlag(projectVersionFlag, value) @@ -74,6 +78,7 @@ var _ = Describe("CLI", func() { var ( projectVersion string plugins []string + err error c *cli ) @@ -87,7 +92,8 @@ var _ = Describe("CLI", func() { When("no flag is set", func() { It("should success", func() { - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("")) Expect(len(plugins)).To(Equal(0)) }) @@ -96,7 +102,8 @@ var _ = Describe("CLI", func() { When(fmt.Sprintf("--%s flag is set", projectVersionFlag), func() { It("should success", func() { setProjectVersionFlag("2") - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("2")) Expect(len(plugins)).To(Equal(0)) }) @@ -105,21 +112,24 @@ var _ = Describe("CLI", func() { When(fmt.Sprintf("--%s flag is set", pluginsFlag), func() { It("should success using one plugin key", func() { setPluginsFlag("go/v1") - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("")) Expect(plugins).To(Equal([]string{"go/v1"})) }) It("should success using more than one plugin key", func() { setPluginsFlag("go/v1,example/v2,test/v1") - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("")) Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) }) It("should success using more than one plugin key with spaces", func() { setPluginsFlag("go/v1 , example/v2 , test/v1") - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("")) Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) }) @@ -129,7 +139,8 @@ var _ = Describe("CLI", func() { It("should success using one plugin key", func() { setProjectVersionFlag("2") setPluginsFlag("go/v1") - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("2")) Expect(plugins).To(Equal([]string{"go/v1"})) }) @@ -137,7 +148,8 @@ var _ = Describe("CLI", func() { It("should success using more than one plugin keys", func() { setProjectVersionFlag("2") setPluginsFlag("go/v1,example/v2,test/v1") - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("2")) Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) }) @@ -145,7 +157,8 @@ var _ = Describe("CLI", func() { It("should success using more than one plugin keys with spaces", func() { setProjectVersionFlag("2") setPluginsFlag("go/v1 , example/v2 , test/v1") - projectVersion, plugins = c.getInfoFromFlags() + projectVersion, plugins, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("2")) Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) }) @@ -154,7 +167,15 @@ var _ = Describe("CLI", func() { When("additional flags are set", func() { It("should not fail", func() { setFlag("extra-flag", "extra-value") - c.getInfoFromFlags() + _, _, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) + }) + + // `--help` is not captured by the whitelist, so we need to special case it + It("should not fail for `--help`", func() { + setBoolFlag("help") + _, _, err = c.getInfoFromFlags() + Expect(err).NotTo(HaveOccurred()) }) }) }) @@ -707,10 +728,11 @@ var _ = Describe("CLI", func() { BeforeEach(func() { args = os.Args }) AfterEach(func() { os.Args = args }) - It("should return an error", func() { + It("should return a CLI that returns an error", func() { setPluginsFlag("foo") - _, err = New() - Expect(err).To(HaveOccurred()) + c, err = New() + Expect(err).NotTo(HaveOccurred()) + Expect(c.Run()).NotTo(Succeed()) }) }) diff --git a/pkg/cli/init.go b/pkg/cli/init.go index 284fecee551..1822d9a0c3d 100644 --- a/pkg/cli/init.go +++ b/pkg/cli/init.go @@ -52,10 +52,6 @@ func (c cli) newInitCmd() *cobra.Command { fmt.Sprintf("Available plugins: (%s)", strings.Join(c.getAvailablePlugins(), ", "))) } - if c.doHelp { - return cmd - } - // Lookup the plugin for projectVersion and bind it to the command. c.bindInit(ctx, cmd) return cmd diff --git a/pkg/cli/root.go b/pkg/cli/root.go new file mode 100644 index 00000000000..728f8cbcfc2 --- /dev/null +++ b/pkg/cli/root.go @@ -0,0 +1,116 @@ +/* +Copyright 2021 The Kubernetes 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 cli + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" +) + +const ( + pluginKeysHeader = "Plugin keys" + projectVersionsHeader = "Supported project versions" +) + +func (c cli) newRootCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: c.commandName, + Long: `CLI tool for building Kubernetes extensions and tools. +`, + Example: c.rootExamples(), + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() + }, + } + + // Register --project-version and --plugin on the dynamically created command + // so that it shows up in help and does not cause a parse error. + cmd.Flags().String(projectVersionFlag, c.defaultProjectVersion, "project version") + cmd.Flags().StringSlice(pluginsFlag, nil, "plugin keys of the plugin to initialize the project with") + + return cmd +} + +// rootExamples builds the examples string for the root command +func (c cli) rootExamples() string { + str := fmt.Sprintf(`The first step is to initialize your project: + %[1]s init --project-version= --plugins= + + is a comma-separated list of plugin keys from the following table +and a supported project version for these plugins. + +%[2]s + +For more specific help for the init command of a certain plugins and project version +configuration please run: + %[1]s init --help --project-version= --plugins= +`, + c.commandName, c.getPluginTable()) + + if c.defaultProjectVersion != "" { + str += fmt.Sprintf("\nDefault project version: %s\n", c.defaultProjectVersion) + + if defaultPlugins, hasDefaultPlugins := c.defaultPlugins[c.defaultProjectVersion]; hasDefaultPlugins { + str += fmt.Sprintf("Default plugin keys: %q\n", strings.Join(defaultPlugins, ",")) + } + } + + str += fmt.Sprintf(` +After the project has been initialized, run + %[1]s --help +to obtain further info about available commands.`, + c.commandName) + + return str +} + +// getPluginTable returns an ASCII table of the available plugins and their supported project versions. +func (c cli) getPluginTable() string { + var ( + maxPluginKeyLength = len(pluginKeysHeader) + pluginKeys = make([]string, 0, len(c.plugins)) + maxProjectVersionLength = len(projectVersionsHeader) + projectVersions = make([]string, 0, len(c.plugins)) + ) + + for pluginKey, plugin := range c.plugins { + if len(pluginKey) > maxPluginKeyLength { + maxPluginKeyLength = len(pluginKey) + } + pluginKeys = append(pluginKeys, pluginKey) + supportedProjectVersions := strings.Join(plugin.SupportedProjectVersions(), ", ") + if len(supportedProjectVersions) > maxProjectVersionLength { + maxProjectVersionLength = len(supportedProjectVersions) + } + projectVersions = append(projectVersions, supportedProjectVersions) + } + + lines := make([]string, 0, len(c.plugins)+2) + lines = append(lines, fmt.Sprintf(" %[1]*[2]s | %[3]*[4]s", + maxPluginKeyLength, pluginKeysHeader, maxProjectVersionLength, projectVersionsHeader)) + lines = append(lines, strings.Repeat("-", maxPluginKeyLength+2)+"+"+ + strings.Repeat("-", maxProjectVersionLength+2)) + for i, pluginKey := range pluginKeys { + supportedProjectVersions := projectVersions[i] + lines = append(lines, fmt.Sprintf(" %[1]*[2]s | %[3]*[4]s", + maxPluginKeyLength, pluginKey, maxProjectVersionLength, supportedProjectVersions)) + } + + return strings.Join(lines, "\n") +}