Skip to content

Commit

Permalink
do not check for plugin executable if none on path (knative#846)
Browse files Browse the repository at this point in the history
  • Loading branch information
danielhelfand authored Jun 2, 2020
1 parent b7fc4c3 commit 834ee79
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 15 deletions.
18 changes: 8 additions & 10 deletions pkg/kn/commands/plugin/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package plugin

import (
"errors"
"fmt"
"os"
"os/exec"
Expand Down Expand Up @@ -127,10 +126,11 @@ func HandlePluginCommand(pluginHandler PluginHandler, cmdArgs []string) error {
remainingArgs := []string{}

for idx := range cmdArgs {
if strings.HasPrefix(cmdArgs[idx], "-") {
cmdArg := cmdArgs[idx]
if strings.HasPrefix(cmdArg, "-") {
continue
}
remainingArgs = append(remainingArgs, strings.Replace(cmdArgs[idx], "-", "_", -1))
remainingArgs = append(remainingArgs, strings.Replace(cmdArg, "-", "_", -1))
}

foundBinaryPath := ""
Expand All @@ -147,16 +147,14 @@ func HandlePluginCommand(pluginHandler PluginHandler, cmdArgs []string) error {
break
}

if len(foundBinaryPath) == 0 {
return errors.New("Could not find plugin to execute")
}

// invoke cmd binary relaying the current environment and args given
// remainingArgs will always have at least one element.
// execve will make remainingArgs[0] the "binary name".
err := pluginHandler.Execute(foundBinaryPath, append([]string{foundBinaryPath}, cmdArgs[len(remainingArgs):]...), os.Environ())
if err != nil {
return err
if len(foundBinaryPath) != 0 {
err := pluginHandler.Execute(foundBinaryPath, append([]string{foundBinaryPath}, cmdArgs[len(remainingArgs):]...), os.Environ())
if err != nil {
return err
}
}

return nil
Expand Down
8 changes: 8 additions & 0 deletions pkg/kn/commands/plugin/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@ func TestPluginHandler(t *testing.T) {
err = HandlePluginCommand(tPluginHandler, []string{"bogus"})
assert.Assert(t, err != nil, fmt.Sprintf("test plugin %s expected to fail executing", "bogus"))
})

t.Run("doesn't return error with -h", func(t *testing.T) {
setup(t)
defer cleanup(t)

err = HandlePluginCommand(tPluginHandler, []string{"source", "-h"})
assert.Assert(t, err == nil, fmt.Sprintf("test plugin command with -h failed executing: %s", err.Error()))
})
})
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/core/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ func showSubcommands(cmd *cobra.Command, args []string, innerArg string) string
for _, subcmd := range cmd.Commands() {
strs = append(strs, subcmd.Name())
}
return fmt.Sprintf("Error: unknown subcommand '%s' for '%s'. Available subcommands: %s\nRun 'kn --help' for usage.\n", innerArg, getCommands(args, innerArg), strings.Join(strs, ", "))
return fmt.Sprintf("Error: unknown subcommand '%s' for '%s'.\nAvailable subcommands: %s\nRun 'kn --help' for usage.\n", innerArg, getCommands(args, innerArg), strings.Join(strs, ", "))
}

func helpOptionsPresent(args []string) bool {
Expand Down
7 changes: 3 additions & 4 deletions test/e2e/basic_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,11 @@ func TestWrongCommand(t *testing.T) {
r := test.NewKnRunResultCollector(t, it)
defer r.DumpIfFailed()

out := test.Kn{}.Run("source", "apiserver", "noverb", "--tag=0.13")
assert.Check(t, util.ContainsAll(out.Stderr, "Error", "unknown sub-command", "noverb"))
out := test.Kn{}.Run("source", "apiserver", "noverb")
assert.Check(t, util.ContainsAll(out.Stderr, "unknown sub-command", "noverb"))
r.AssertError(out)

out = test.Kn{}.Run("rev")
assert.Check(t, util.ContainsAll(out.Stderr, "unknown command", "rev"))
assert.Check(t, util.ContainsAll(out.Stderr, "unknown command", "rev", "revision"))
r.AssertError(out)

}

0 comments on commit 834ee79

Please sign in to comment.