diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index ba7bcd5178..51a6c30f1a 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -18,6 +18,10 @@ |=== | | Description | PR +| ๐ŸŽ +| Add available plugins to help messages +| https://github.com/knative/client/pull/929[#929] + | ๐ŸŽ | Add "url" output format to return service url in service describe | https://github.com/knative/client/pull/916[#916] diff --git a/cmd/kn/main.go b/cmd/kn/main.go index 876cd50fe1..9a5538c13a 100644 --- a/cmd/kn/main.go +++ b/cmd/kn/main.go @@ -43,6 +43,17 @@ func main() { } } +// runError is used when during the execution of a command/plugin an error occurs and +// so no extra usage message should be shown. +type runError struct { + err error +} + +// Error implements the error() interface +func (e *runError) Error() string { + return e.err.Error() +} + // Run the main program. Args are the args as given on the command line (excluding the program name itself) func run(args []string) error { // Parse config & plugin flags early to read in configuration file @@ -67,7 +78,7 @@ func run(args []string) error { } // Create kn root command and all sub-commands - rootCmd, err := root.NewRootCommand() + rootCmd, err := root.NewRootCommand(pluginManager.HelpTemplateFuncs()) if err != nil { return err } @@ -79,7 +90,11 @@ func run(args []string) error { return err } - return plugin.Execute(argsWithoutCommands(args, plugin.CommandParts())) + err := plugin.Execute(argsWithoutCommands(args, plugin.CommandParts())) + if err != nil { + return &runError{err: err} + } + return nil } else { // Validate args for root command err = validateRootCommand(rootCmd) @@ -190,7 +205,11 @@ func validateRootCommand(cmd *cobra.Command) error { // printError prints out any given error func printError(err error) { fmt.Fprintf(os.Stderr, "Error: %s\n", cleanupErrorMessage(err.Error())) - fmt.Fprintf(os.Stderr, "Run '%s --help' for usage\n", extractCommandPathFromErrorMessage(err.Error(), os.Args[0])) + var runError *runError + if !errors.As(err, &runError) { + // Print help hint only if its not a runError occurred when executing a command + fmt.Fprintf(os.Stderr, "Run '%s --help' for usage\n", extractCommandPathFromErrorMessage(err.Error(), os.Args[0])) + } } // extractCommandPathFromErrorMessage tries to extract the command name from an error message diff --git a/cmd/kn/main_test.go b/cmd/kn/main_test.go index 278573788a..e83d6c3fb5 100644 --- a/cmd/kn/main_test.go +++ b/cmd/kn/main_test.go @@ -158,7 +158,7 @@ func TestUnknownCommands(t *testing.T) { } for _, d := range data { args := append([]string{"kn"}, d.givenCmdArgs...) - rootCmd, err := root.NewRootCommand() + rootCmd, err := root.NewRootCommand(nil) os.Args = args assert.NilError(t, err) err = validateRootCommand(rootCmd) @@ -253,6 +253,29 @@ func TestRunWithError(t *testing.T) { } } +func TestRunWithPluginError(t *testing.T) { + data := []struct { + given string + expected string + }{ + { + "exit status 1", + "Error: exit status 1", + }, + } + for _, d := range data { + capture := test.CaptureOutput(t) + // displayHelp argument is false for plugin error + printError(&runError{errors.New(d.given)}) + stdOut, errOut := capture.Close() + + assert.Equal(t, stdOut, "") + assert.Assert(t, strings.Contains(errOut, d.expected)) + // check that --help message isn't displayed + assert.Assert(t, util.ContainsNone(errOut, "Run", "--help", "usage")) + } +} + // Smoke test func TestRun(t *testing.T) { oldArgs := os.Args diff --git a/hack/build.sh b/hack/build.sh index 7a150afc5b..a67b3f2a1f 100755 --- a/hack/build.sh +++ b/hack/build.sh @@ -152,7 +152,7 @@ go_test() { echo "๐Ÿงช ${X}Test" set +e - go test -v ./pkg/... >$test_output 2>&1 + go test -v ./cmd/... ./pkg/... >$test_output 2>&1 local err=$? if [ $err -ne 0 ]; then echo "๐Ÿ”ฅ ${red}Failure${reset}" diff --git a/hack/generate-docs.go b/hack/generate-docs.go index f87b9a38fc..ed33a14145 100644 --- a/hack/generate-docs.go +++ b/hack/generate-docs.go @@ -27,7 +27,7 @@ import ( ) func main() { - rootCmd, err := root.NewRootCommand() + rootCmd, err := root.NewRootCommand(nil) if err != nil { log.Panicf("can not create root command: %v", err) } diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index df20ba22b3..a136c33fc5 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -23,9 +23,11 @@ import ( "runtime" "sort" "strings" + "text/template" homedir "github.com/mitchellh/go-homedir" "github.com/pkg/errors" + "github.com/spf13/cobra" ) // Interface describing a plugin @@ -108,6 +110,12 @@ func (manager *Manager) FindPlugin(parts []string) (Plugin, error) { // ListPlugins lists all plugins that can be found in the plugin directory or in the path (if configured) func (manager *Manager) ListPlugins() (PluginList, error) { + return manager.ListPluginsForCommandGroup(nil) +} + +// ListPluginsForCommandGroup lists all plugins that can be found in the plugin directory or in the path (if configured), +// and which fits to a command group +func (manager *Manager) ListPluginsForCommandGroup(commandGroupParts []string) (PluginList, error) { var plugins []Plugin dirs, err := manager.pluginLookupDirectories() @@ -135,6 +143,11 @@ func (manager *Manager) ListPlugins() (PluginList, error) { continue } + // Check if plugin matches a command group + if !isPartOfCommandGroup(commandGroupParts, f.Name()) { + continue + } + // Ignore all plugins that are shadowed if _, ok := hasSeen[name]; !ok { plugins = append(plugins, &plugin{ @@ -146,11 +159,35 @@ func (manager *Manager) ListPlugins() (PluginList, error) { } } } + // Sort according to name sort.Sort(PluginList(plugins)) return plugins, nil } +func isPartOfCommandGroup(commandGroupParts []string, name string) bool { + if commandGroupParts == nil { + return true + } + + commandParts := extractPluginCommandFromFileName(name) + + // commandParts must be one more element then the parts of the command group + // it belongs to. E.g. for the command "service", "log" (2 elements) the containing + // group only has one element ("service"). This condition is here for + // shortcut and ensure that we don't run in an out-of-bound array error + // in the loop below. + if len(commandParts) != len(commandGroupParts)+1 { + return false + } + for i := range commandGroupParts { + if commandParts[i] != commandGroupParts[i] { + return false + } + } + return true +} + // PluginsDir returns the configured directory holding plugins func (manager *Manager) PluginsDir() string { return manager.pluginsDir @@ -176,8 +213,9 @@ func (plugin *plugin) Execute(args []string) error { // Return a description of the plugin (if support by the plugin binary) func (plugin *plugin) Description() (string, error) { // TODO: Call out to the plugin to find a description. - // For now just use the plugin name - return strings.Join(plugin.commandParts, "-"), nil + // For now just use the path to the plugin + return plugin.path, nil + // return strings.Join(plugin.commandParts, "-"), nil } // The the command path leading to this plugin. @@ -212,6 +250,49 @@ func (manager *Manager) pluginLookupDirectories() ([]string, error) { return dirs, nil } +// HelpTemplateFuncs returns a function map which can be used in templates for resolving +// plugin related help messages +func (manager *Manager) HelpTemplateFuncs() *template.FuncMap { + ret := template.FuncMap{ + "listPlugins": manager.listPluginsHelpMessage(), + } + + return &ret +} + +// listPluginsHelpMessage returns a function which returns all plugins that are directly below the given +// command as a properly formatted string +func (manager *Manager) listPluginsHelpMessage() func(cmd *cobra.Command) string { + return func(cmd *cobra.Command) string { + if !cmd.HasSubCommands() { + return "" + } + list, err := manager.ListPluginsForCommandGroup(extractCommandGroup(cmd, []string{})) + if err != nil || len(list) == 0 { + // We don't show plugins if there is an error + return "" + } + var plugins []string + for _, pl := range list { + t := fmt.Sprintf(" %%-%ds %%s", cmd.NamePadding()) + desc, _ := pl.Description() + command := (pl.CommandParts())[len(pl.CommandParts())-1] + help := fmt.Sprintf(t, command, desc) + plugins = append(plugins, help) + } + return strings.Join(plugins, "\n") + } +} + +// extractCommandGroup constructs the command path as array of strings +func extractCommandGroup(cmd *cobra.Command, parts []string) []string { + if cmd.HasParent() { + parts = extractCommandGroup(cmd.Parent(), parts) + parts = append(parts, cmd.Name()) + } + return parts +} + // uniquePathsList deduplicates a given slice of strings without // sorting or otherwise altering its order in any way. func uniquePathsList(paths []string) []string { diff --git a/pkg/kn/plugin/manager_test.go b/pkg/kn/plugin/manager_test.go index 16efba8f3b..66c319b26d 100644 --- a/pkg/kn/plugin/manager_test.go +++ b/pkg/kn/plugin/manager_test.go @@ -19,9 +19,11 @@ import ( "io/ioutil" "os" "path/filepath" + "regexp" "runtime" "testing" + "github.com/spf13/cobra" "gotest.tools/assert" ) @@ -110,6 +112,56 @@ func TestPluginExecute(t *testing.T) { assert.Equal(t, out, "OK arg1 arg2\n") } +func TestPluginListForCommandGroup(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + createTestPlugin(t, "kn-service-log_2", ctx) + + pluginList, err := ctx.pluginManager.ListPluginsForCommandGroup([]string{"service"}) + assert.NilError(t, err) + assert.Assert(t, pluginList.Len() == 1) + assert.Equal(t, pluginList[0].Name(), "kn-service-log_2") + pluginList, err = ctx.pluginManager.ListPluginsForCommandGroup([]string{}) + assert.NilError(t, err) + assert.Assert(t, pluginList.Len() == 0) +} + +func TestPluginHelpMessage(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + createTestPlugin(t, "kn-service-log_2", ctx) + createTestPlugin(t, "kn-admin", ctx) + + funcs := *ctx.pluginManager.HelpTemplateFuncs() + f := funcs["listPlugins"] + assert.Assert(t, f != nil) + listPluginsFunc := ctx.pluginManager.listPluginsHelpMessage() + + root := &cobra.Command{ + Use: "kn", + } + serviceCmd := &cobra.Command{ + Use: "service", + } + serviceCreateCmd := &cobra.Command{ + Use: "create", + } + serviceCmd.AddCommand(serviceCreateCmd) + root.AddCommand(serviceCmd) + + helpRoot := listPluginsFunc(root) + re := regexp.MustCompile("^\\s*admin\\s.*admin") + assert.Assert(t, re.MatchString(helpRoot)) + + helpService := listPluginsFunc(serviceCmd) + println(helpService) + re = regexp.MustCompile("^\\s*log-2\\s.*kn-service-log_2") + assert.Assert(t, re.MatchString(helpService)) + + helpServiceCreate := listPluginsFunc(serviceCreateCmd) + assert.Assert(t, len(helpServiceCreate) == 0) +} + func TestPluginList(t *testing.T) { ctx := setup(t) defer cleanup(t, ctx) diff --git a/pkg/kn/root/root.go b/pkg/kn/root/root.go index 17a77528cc..12ddef6780 100644 --- a/pkg/kn/root/root.go +++ b/pkg/kn/root/root.go @@ -18,6 +18,7 @@ import ( "flag" "fmt" "strings" + "text/template" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -41,7 +42,7 @@ import ( ) // NewRootCommand creates the default `kn` command with a default plugin handler -func NewRootCommand() (*cobra.Command, error) { +func NewRootCommand(helpFuncs *template.FuncMap) (*cobra.Command, error) { p := &commands.KnParams{} p.Initialize() @@ -107,7 +108,7 @@ func NewRootCommand() (*cobra.Command, error) { groups.AddTo(rootCmd) // Initialize default `help` cmd early to prevent unknown command errors - groups.SetRootUsage(rootCmd) + groups.SetRootUsage(rootCmd, helpFuncs) // Add the "options" commands for showing all global options rootCmd.AddCommand(options.NewOptionsCommand()) diff --git a/pkg/kn/root/root_test.go b/pkg/kn/root/root_test.go index 7a32048628..fff5ef97b6 100644 --- a/pkg/kn/root/root_test.go +++ b/pkg/kn/root/root_test.go @@ -26,7 +26,7 @@ import ( ) func TestNewRootCommand(t *testing.T) { - rootCmd, err := NewRootCommand() + rootCmd, err := NewRootCommand(nil) assert.NilError(t, err) assert.Assert(t, rootCmd != nil) @@ -51,13 +51,13 @@ func TestNewRootCommand(t *testing.T) { } func TestSubCommands(t *testing.T) { - rootCmd, err := NewRootCommand() + rootCmd, err := NewRootCommand(nil) assert.NilError(t, err) checkLeafCommand(t, "version", rootCmd) } func TestCommandGroup(t *testing.T) { - rootCmd, err := NewRootCommand() + rootCmd, err := NewRootCommand(nil) assert.NilError(t, err) commandGroups := []string{ "service", "revision", "plugin", "source", "source apiserver", diff --git a/pkg/templates/command_groups.go b/pkg/templates/command_groups.go index 5866842379..5af9af6ec0 100644 --- a/pkg/templates/command_groups.go +++ b/pkg/templates/command_groups.go @@ -15,6 +15,8 @@ package templates import ( + "text/template" + "github.com/spf13/cobra" ) @@ -40,11 +42,8 @@ func (g CommandGroups) AddTo(cmd *cobra.Command) { } // SetRootUsage sets our own help and usage function messages to the root command -func (g CommandGroups) SetRootUsage(rootCmd *cobra.Command) { - engine := &templateEngine{ - RootCmd: rootCmd, - CommandGroups: g, - } +func (g CommandGroups) SetRootUsage(rootCmd *cobra.Command, extraTemplateFunctions *template.FuncMap) { + engine := newTemplateEngine(rootCmd, g, extraTemplateFunctions) setHelpFlagsToSubCommands(rootCmd) rootCmd.SetUsageFunc(engine.usageFunc()) rootCmd.SetHelpFunc(engine.helpFunc()) diff --git a/pkg/templates/command_groups_test.go b/pkg/templates/command_groups_test.go index 011241a447..59026eab94 100644 --- a/pkg/templates/command_groups_test.go +++ b/pkg/templates/command_groups_test.go @@ -17,6 +17,7 @@ package templates import ( "fmt" "testing" + "text/template" "github.com/spf13/cobra" "gotest.tools/assert" @@ -48,7 +49,7 @@ func TestAddTo(t *testing.T) { func TestSetUsage(t *testing.T) { rootCmd := &cobra.Command{Use: "root", Short: "root", Run: func(cmd *cobra.Command, args []string) {}} groups.AddTo(rootCmd) - groups.SetRootUsage(rootCmd) + groups.SetRootUsage(rootCmd, getTestFuncMap()) for _, cmd := range rootCmd.Commands() { assert.Assert(t, cmd.DisableFlagsInUseLine) @@ -67,3 +68,12 @@ func TestSetUsage(t *testing.T) { assert.Equal(t, stdErr, "") assert.Assert(t, util.ContainsAll(stdOut, "root", "header-1", "header-2")) } + +func getTestFuncMap() *template.FuncMap { + fMap := template.FuncMap{ + "listPlugins": func(c *cobra.Command) string { + return "" + }, + } + return &fMap +} diff --git a/pkg/templates/template_engine.go b/pkg/templates/template_engine.go index a927078788..af84d29164 100644 --- a/pkg/templates/template_engine.go +++ b/pkg/templates/template_engine.go @@ -30,14 +30,30 @@ import ( type templateEngine struct { RootCmd *cobra.Command CommandGroups + functions template.FuncMap } // Get the function to show the global options func NewGlobalOptionsFunc() func(command *cobra.Command) error { - return templateEngine{}.optionsFunc() + return newTemplateEngine(nil, nil, nil).optionsFunc() } -func (e templateEngine) usageFunc() func(command *cobra.Command) error { +// Create new template engine +func newTemplateEngine(rootCmd *cobra.Command, g CommandGroups, extraFunctions *template.FuncMap) templateEngine { + engine := templateEngine{ + RootCmd: rootCmd, + CommandGroups: g, + } + engine.functions = engine.templateFunctions() + if extraFunctions != nil { + for name, function := range *extraFunctions { + engine.functions[name] = function + } + } + return engine +} + +func (e templateEngine) usageFunc() func(*cobra.Command) error { return func(c *cobra.Command) error { return e.fillTemplate("usage", c, usageTemplate()) } @@ -60,7 +76,7 @@ func (e templateEngine) optionsFunc() func(command *cobra.Command) error { func (e templateEngine) fillTemplate(name string, c *cobra.Command, templ string) error { t := template.New(name) - t.Funcs(e.templateFunctions()) + t.Funcs(e.functions) _, err := t.Parse(templ) if err != nil { fmt.Fprintf(c.ErrOrStderr(), "\nINTERNAL: >>>>> %v\n", err) diff --git a/pkg/templates/template_engine_test.go b/pkg/templates/template_engine_test.go index 2ce0b13c5d..844ec09f58 100644 --- a/pkg/templates/template_engine_test.go +++ b/pkg/templates/template_engine_test.go @@ -33,7 +33,7 @@ type testData struct { } func TestUsageFunc(t *testing.T) { - rootCmd, engine := newTemplateEngine() + rootCmd, engine := newTestTemplateEngine() subCmdWithSubs, _, _ := rootCmd.Find([]string{"g1.1"}) subCmd, _, _ := rootCmd.Find([]string{"g2.1"}) @@ -71,7 +71,7 @@ func TestUsageFunc(t *testing.T) { } func TestHelpFunc(t *testing.T) { - rootCmd, engine := newTemplateEngine() + rootCmd, engine := newTestTemplateEngine() subCmd := rootCmd.Commands()[0] data := []testData{ @@ -101,7 +101,7 @@ func TestHelpFunc(t *testing.T) { } func TestOptionsFunc(t *testing.T) { - rootCmd, _ := newTemplateEngine() + rootCmd, _ := newTestTemplateEngine() subCmd := rootCmd.Commands()[0] capture := test.CaptureOutput(t) err := NewGlobalOptionsFunc()(subCmd) @@ -135,7 +135,7 @@ func validateSubUsageOutput(t *testing.T, stdOut string, cmd *cobra.Command) { assert.Assert(t, util.ContainsAll(stdOut, "Use", "root", "options", "global")) } -func newTemplateEngine() (*cobra.Command, templateEngine) { +func newTestTemplateEngine() (*cobra.Command, templateEngine) { rootCmd := &cobra.Command{Use: "root", Short: "desc-root", Long: "longdesc-root"} rootCmd.PersistentFlags().String("global-opt", "", "global option") cmdGroups := CommandGroups{ @@ -148,10 +148,7 @@ func newTemplateEngine() (*cobra.Command, templateEngine) { []*cobra.Command{newCmd("g2.1"), newCmd("g2.2"), newCmd("g2.3")}, }, } - engine := templateEngine{ - RootCmd: rootCmd, - CommandGroups: cmdGroups, - } + engine := newTemplateEngine(rootCmd, cmdGroups, getTestFuncMap()) cmdGroups.AddTo(rootCmd) // Add a sub-command to first command diff --git a/pkg/templates/templates.go b/pkg/templates/templates.go index 51bc1625ea..06b7ff9129 100644 --- a/pkg/templates/templates.go +++ b/pkg/templates/templates.go @@ -46,6 +46,13 @@ const ( {{end}}` + // sectionPlugins lists all plugins (if any) + sectionPlugins = `{{$plugins := listPlugins .}}{{ if ne (len $plugins) 0}}Plugins: +{{trimRight $plugins}} + +{{end}} +` + // sectionFlags is the help template section that displays the command's flags. sectionFlags = `{{$visibleFlags := visibleFlags .}}{{ if $visibleFlags.HasFlags}}Options: {{trimRight (flagsUsages $visibleFlags)}} @@ -74,6 +81,7 @@ func usageTemplate() string { sectionExamples, sectionCommandGroups, sectionSubCommands, + sectionPlugins, sectionFlags, sectionUsage, sectionTipsHelp, diff --git a/test/e2e/plugins_test.go b/test/e2e/plugins_test.go index 2a93fd56a5..3d6df3bd1f 100644 --- a/test/e2e/plugins_test.go +++ b/test/e2e/plugins_test.go @@ -22,7 +22,6 @@ import ( "fmt" "io/ioutil" "os" - "os/exec" "path/filepath" "testing" @@ -38,11 +37,14 @@ const ( echo "Hello Knative, I'm a Kn plugin" echo " My plugin file is $0" echo " I received arguments: $1 $2 $3 $4"` + + TestPluginCodeErr string = `#!/bin/bash +exit 1` ) type pluginTestConfig struct { - knConfigDir, knPluginsDir, knPluginsDir2 string - knConfigPath, knPluginPath, knPluginPath2 string + knConfigDir, knPluginsDir, knPluginsDir2, knPluginsDir3 string + knConfigPath, knPluginPath, knPluginPath2, knPluginsPath3 string } func (pc *pluginTestConfig) setup() error { @@ -108,17 +110,18 @@ func TestPluginWithoutLookup(t *testing.T) { listPlugin(r, knFlags, []string{pc.knPluginPath}, []string{pc.knPluginPath2}) } -func execute(command string, args ...string) string { - cmd := exec.Command(command, args...) - r, w, _ := os.Pipe() - cmd.Stdout = w - cmd.Stderr = os.Stderr - cmd.Stdin = os.Stdin - cmd.Env = os.Environ() - cmd.Run() - w.Close() - ret, _ := ioutil.ReadAll(r) - return string(ret) +func TestPluginInHelpMessage(t *testing.T) { + pc := pluginTestConfig{} + assert.NilError(t, pc.setup()) + defer pc.teardown() + + result := test.Kn{}.Run("--plugins-dir", pc.knPluginsDir, "--help") + assert.NilError(t, result.Error) + assert.Assert(t, util.ContainsAll(result.Stdout, "Plugins:", "helloe2e", "kn-helloe2e")) + + result = test.Kn{}.Run("--plugins-dir", pc.knPluginsDir, "service", "--help") + assert.NilError(t, result.Error) + assert.Assert(t, util.ContainsNone(result.Stdout, "Plugins:", "helloe2e", "kn-helloe2e")) } func TestPluginWithLookup(t *testing.T) { @@ -172,14 +175,33 @@ func TestExecutePluginInPath(t *testing.T) { runPlugin(r, knFlags, "hello2e2e", []string{}, []string{"Hello Knative, I'm a Kn plugin"}) } -// Private +func TestExecutePluginInPathWithError(t *testing.T) { + it, err := test.NewKnTest() + assert.NilError(t, err) -func createPluginFile(fileName, fileContent, filePath string, fileMode os.FileMode) (string, error) { - file := filepath.Join(filePath, fileName) - err := ioutil.WriteFile(file, []byte(fileContent), fileMode) - return file, err + r := test.NewKnRunResultCollector(t, it) + defer r.DumpIfFailed() + + pc := pluginTestConfig{} + assert.NilError(t, pc.setup()) + oldPath := os.Getenv("PATH") + + t.Log("execute plugin in $PATH that returns error") + pluginsDir := filepath.Join(pc.knConfigDir, "plugins3") + err = os.MkdirAll(pluginsDir, test.FileModeExecutable) + assert.NilError(t, err) + _, err = test.CreateFile("kn-hello3e2e", TestPluginCodeErr, pluginsDir, test.FileModeExecutable) + assert.NilError(t, err) + assert.NilError(t, os.Setenv("PATH", fmt.Sprintf("%s:%s", oldPath, pluginsDir))) + defer tearDownWithPath(pc, oldPath) + + out := test.Kn{}.Run("--lookup-plugins=true", "hello3e2e") + r.AssertError(out) + assert.Check(r.T(), util.ContainsAll(out.Stderr, "Error: exit status 1")) + assert.Check(r.T(), util.ContainsNone(out.Stderr, "Run", "kn --help", "usage")) } +// Private func setupPluginTestConfigWithNewPath(t *testing.T) (pluginTestConfig, string) { pc := pluginTestConfig{} assert.NilError(t, pc.setup())