From 13f197cabedc1e478ce97d8ecbbc0e9be7dc9e3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 13 Jul 2020 18:35:30 +0200 Subject: [PATCH 01/10] feat: Add plugin listing to "kn --help" This works on all levels. Test needs to be expanded still. Fixes #266 --- cmd/kn/main.go | 2 +- hack/generate-docs.go | 2 +- pkg/kn/plugin/manager.go | 74 +++++++++++++++++++++++++++ pkg/kn/root/root.go | 5 +- pkg/kn/root/root_test.go | 6 +-- pkg/templates/command_groups.go | 9 ++-- pkg/templates/command_groups_test.go | 12 ++++- pkg/templates/template_engine.go | 22 ++++++-- pkg/templates/template_engine_test.go | 13 ++--- pkg/templates/templates.go | 8 +++ 10 files changed, 129 insertions(+), 24 deletions(-) diff --git a/cmd/kn/main.go b/cmd/kn/main.go index 876cd50fe1..67248972a9 100644 --- a/cmd/kn/main.go +++ b/cmd/kn/main.go @@ -67,7 +67,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 } 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..d6ea536bb9 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,29 @@ 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) + 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 @@ -212,6 +243,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/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, From 0f32b9ba943a182971784d141b8e1f8499b2523d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 13 Jul 2020 18:49:50 +0200 Subject: [PATCH 02/10] chore: Fix test --- cmd/kn/main_test.go | 2 +- hack/build.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/kn/main_test.go b/cmd/kn/main_test.go index 278573788a..246856ce6b 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) diff --git a/hack/build.sh b/hack/build.sh index 7a150afc5b..53cb26c694 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}" From e157a027a6a13f9bb40e6eb00b767618be141ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 11:22:50 +0200 Subject: [PATCH 03/10] Add test and ported #910 over. --- cmd/kn/main.go | 40 +++++++++++++++-------- cmd/kn/main_test.go | 27 ++++++++++++++-- pkg/kn/plugin/manager.go | 5 +-- pkg/kn/plugin/manager_test.go | 52 ++++++++++++++++++++++++++++++ test/e2e/plugins_test.go | 60 ++++++++++++++++++++++++----------- 5 files changed, 147 insertions(+), 37 deletions(-) diff --git a/cmd/kn/main.go b/cmd/kn/main.go index 67248972a9..a08feda479 100644 --- a/cmd/kn/main.go +++ b/cmd/kn/main.go @@ -35,59 +35,68 @@ func init() { } func main() { - err := run(os.Args[1:]) - if err != nil && len(os.Args) > 1 { - printError(err) + runError := run(os.Args[1:]) + if runError.err != nil && len(os.Args) > 1 { + printError(runError) // This is the only point from where to exit when an error occurs os.Exit(1) } } +// Type for not printing a help message +type runError struct { + // Return encapsulate error + err error + + // Whether to show a help message or not + showUsageHint bool +} + // Run the main program. Args are the args as given on the command line (excluding the program name itself) -func run(args []string) error { +func run(args []string) runError { // Parse config & plugin flags early to read in configuration file // and bind to viper. After that you can access all configuration and // global options via methods on config.GlobalConfig err := config.BootstrapConfig() if err != nil { - return err + return runError{err, true} } // Strip of all flags to get the non-flag commands only commands, err := stripFlags(args) if err != nil { - return err + return runError{err, true} } // Find plugin with the commands arguments pluginManager := plugin.NewManager(config.GlobalConfig.PluginsDir(), config.GlobalConfig.LookupPluginsInPath()) plugin, err := pluginManager.FindPlugin(commands) if err != nil { - return err + return runError{err, true} } // Create kn root command and all sub-commands rootCmd, err := root.NewRootCommand(pluginManager.HelpTemplateFuncs()) if err != nil { - return err + return runError{err, true} } if plugin != nil { // Validate & Execute plugin err = validatePlugin(rootCmd, plugin) if err != nil { - return err + return runError{err, true} } - return plugin.Execute(argsWithoutCommands(args, plugin.CommandParts())) + return runError{err, false} } else { // Validate args for root command err = validateRootCommand(rootCmd) if err != nil { - return err + return runError{err, true} } // Execute kn root command, args are taken from os.Args directly - return rootCmd.Execute() + return runError{rootCmd.Execute(), true} } } @@ -188,9 +197,12 @@ func validateRootCommand(cmd *cobra.Command) error { } // printError prints out any given error -func printError(err error) { +func printError(runErr runError) { + err := runErr.err 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])) + if runErr.showUsageHint { + 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 246856ce6b..5ec3fd18c7 100644 --- a/cmd/kn/main_test.go +++ b/cmd/kn/main_test.go @@ -244,7 +244,7 @@ func TestRunWithError(t *testing.T) { } for _, d := range data { capture := test.CaptureOutput(t) - printError(errors.New(d.given)) + printError(runError{errors.New(d.given), true}) stdOut, errOut := capture.Close() assert.Equal(t, stdOut, "") @@ -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), false}) + 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 @@ -265,6 +288,6 @@ func TestRun(t *testing.T) { err := run(os.Args[1:]) out, _ := capture.Close() - assert.NilError(t, err) + assert.NilError(t, err.err) assert.Assert(t, util.ContainsAllIgnoreCase(out, "version", "build", "git")) } diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index d6ea536bb9..b567a0e957 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -207,8 +207,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. 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/test/e2e/plugins_test.go b/test/e2e/plugins_test.go index 2a93fd56a5..4c436ac152 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.knPluginPath, "--help") + assert.NilError(t, result.Error) + assert.Assert(t, util.ContainsAll(result.Stdout, "Plugins:", "helloe2e", "kn-helloe2e")) + + result = test.Kn{}.Run("--plugins-dir", pc.knPluginPath, "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(pc.knPluginsDir3, 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.Stdout, "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()) From ccc3ec1e3d9d9563b9a6e33d6b67b6c9867e51ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 11:24:17 +0200 Subject: [PATCH 04/10] changelog update --- CHANGELOG.adoc | 4 ++++ 1 file changed, 4 insertions(+) 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] From 4ff9911dfe14d5516d21270018f320355dccac99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 13:07:00 +0200 Subject: [PATCH 05/10] fix test --- cmd/kn/main.go | 42 ++++++++++++++++++++++------------------ test/e2e/plugins_test.go | 4 ++-- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/cmd/kn/main.go b/cmd/kn/main.go index a08feda479..bcd61e9299 100644 --- a/cmd/kn/main.go +++ b/cmd/kn/main.go @@ -35,68 +35,71 @@ func init() { } func main() { - runError := run(os.Args[1:]) - if runError.err != nil && len(os.Args) > 1 { - printError(runError) + err := run(os.Args[1:]) + if err != nil && len(os.Args) > 1 { + printError(err) // This is the only point from where to exit when an error occurs os.Exit(1) } } -// Type for not printing a help message +// 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 { - // Return encapsulate error err error +} - // Whether to show a help message or not - showUsageHint bool +// 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) runError { +func run(args []string) error { // Parse config & plugin flags early to read in configuration file // and bind to viper. After that you can access all configuration and // global options via methods on config.GlobalConfig err := config.BootstrapConfig() if err != nil { - return runError{err, true} + return err } // Strip of all flags to get the non-flag commands only commands, err := stripFlags(args) if err != nil { - return runError{err, true} + return err } // Find plugin with the commands arguments pluginManager := plugin.NewManager(config.GlobalConfig.PluginsDir(), config.GlobalConfig.LookupPluginsInPath()) plugin, err := pluginManager.FindPlugin(commands) if err != nil { - return runError{err, true} + return err } // Create kn root command and all sub-commands rootCmd, err := root.NewRootCommand(pluginManager.HelpTemplateFuncs()) if err != nil { - return runError{err, true} + return err } if plugin != nil { // Validate & Execute plugin err = validatePlugin(rootCmd, plugin) if err != nil { - return runError{err, true} + return err } - return runError{err, false} + err := plugin.Execute(argsWithoutCommands(args, plugin.CommandParts())) + return &runError{err: err} } else { // Validate args for root command err = validateRootCommand(rootCmd) if err != nil { - return runError{err, true} + return err } // Execute kn root command, args are taken from os.Args directly - return runError{rootCmd.Execute(), true} + return rootCmd.Execute() } } @@ -197,10 +200,11 @@ func validateRootCommand(cmd *cobra.Command) error { } // printError prints out any given error -func printError(runErr runError) { - err := runErr.err +func printError(err error) { fmt.Fprintf(os.Stderr, "Error: %s\n", cleanupErrorMessage(err.Error())) - if runErr.showUsageHint { + 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])) } } diff --git a/test/e2e/plugins_test.go b/test/e2e/plugins_test.go index 4c436ac152..3e9082e74d 100644 --- a/test/e2e/plugins_test.go +++ b/test/e2e/plugins_test.go @@ -115,11 +115,11 @@ func TestPluginInHelpMessage(t *testing.T) { assert.NilError(t, pc.setup()) defer pc.teardown() - result := test.Kn{}.Run("--plugins-dir", pc.knPluginPath, "--help") + 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.knPluginPath, "service", "--help") + 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")) } From cd8ec5c5dee90508e5c242ca1e0d3feb90f20ea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 13:17:17 +0200 Subject: [PATCH 06/10] Fix test --- cmd/kn/main_test.go | 6 +++--- hack/build.sh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/kn/main_test.go b/cmd/kn/main_test.go index 5ec3fd18c7..e83d6c3fb5 100644 --- a/cmd/kn/main_test.go +++ b/cmd/kn/main_test.go @@ -244,7 +244,7 @@ func TestRunWithError(t *testing.T) { } for _, d := range data { capture := test.CaptureOutput(t) - printError(runError{errors.New(d.given), true}) + printError(errors.New(d.given)) stdOut, errOut := capture.Close() assert.Equal(t, stdOut, "") @@ -266,7 +266,7 @@ func TestRunWithPluginError(t *testing.T) { for _, d := range data { capture := test.CaptureOutput(t) // displayHelp argument is false for plugin error - printError(runError{errors.New(d.given), false}) + printError(&runError{errors.New(d.given)}) stdOut, errOut := capture.Close() assert.Equal(t, stdOut, "") @@ -288,6 +288,6 @@ func TestRun(t *testing.T) { err := run(os.Args[1:]) out, _ := capture.Close() - assert.NilError(t, err.err) + assert.NilError(t, err) assert.Assert(t, util.ContainsAllIgnoreCase(out, "version", "build", "git")) } diff --git a/hack/build.sh b/hack/build.sh index 53cb26c694..a67b3f2a1f 100755 --- a/hack/build.sh +++ b/hack/build.sh @@ -152,7 +152,7 @@ go_test() { echo "๐Ÿงช ${X}Test" set +e - go test -v .cmd/... ./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}" From 052b94fbf46eb2ddfccf85a1852bdde0071ec355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 14:58:22 +0200 Subject: [PATCH 07/10] fix integration tests --- cmd/kn/main.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/kn/main.go b/cmd/kn/main.go index bcd61e9299..9a5538c13a 100644 --- a/cmd/kn/main.go +++ b/cmd/kn/main.go @@ -91,7 +91,10 @@ func run(args []string) error { } err := plugin.Execute(argsWithoutCommands(args, plugin.CommandParts())) - return &runError{err: err} + if err != nil { + return &runError{err: err} + } + return nil } else { // Validate args for root command err = validateRootCommand(rootCmd) From 917c2ade7cd11755648611d772398bf75126ee8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 15:33:20 +0200 Subject: [PATCH 08/10] fix test --- test/e2e/plugins_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/plugins_test.go b/test/e2e/plugins_test.go index 3e9082e74d..2177d7b7a8 100644 --- a/test/e2e/plugins_test.go +++ b/test/e2e/plugins_test.go @@ -188,7 +188,7 @@ func TestExecutePluginInPathWithError(t *testing.T) { t.Log("execute plugin in $PATH that returns error") pluginsDir := filepath.Join(pc.knConfigDir, "plugins3") - err = os.MkdirAll(pc.knPluginsDir3, test.FileModeExecutable) + err = os.MkdirAll(pluginsDir, test.FileModeExecutable) assert.NilError(t, err) _, err = test.CreateFile("kn-hello3e2e", TestPluginCodeErr, pluginsDir, test.FileModeExecutable) assert.NilError(t, err) From fc3a93b3e4294dbe876941b2a7a35aae2bffc2a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 15:56:12 +0200 Subject: [PATCH 09/10] chore: Add some explanatory comments --- pkg/kn/plugin/manager.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index b567a0e957..a136c33fc5 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -171,6 +171,12 @@ func isPartOfCommandGroup(commandGroupParts []string, name string) bool { } 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 } From 7a7a93e77b18bb81aeda56fa3771962fd0e5b84e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Tue, 14 Jul 2020 17:49:50 +0200 Subject: [PATCH 10/10] fix test --- test/e2e/plugins_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/plugins_test.go b/test/e2e/plugins_test.go index 2177d7b7a8..3d6df3bd1f 100644 --- a/test/e2e/plugins_test.go +++ b/test/e2e/plugins_test.go @@ -197,7 +197,7 @@ func TestExecutePluginInPathWithError(t *testing.T) { out := test.Kn{}.Run("--lookup-plugins=true", "hello3e2e") r.AssertError(out) - assert.Check(r.T(), util.ContainsAll(out.Stdout, "Error: exit status 1")) + assert.Check(r.T(), util.ContainsAll(out.Stderr, "Error: exit status 1")) assert.Check(r.T(), util.ContainsNone(out.Stderr, "Run", "kn --help", "usage")) }