From 66daa37ccbdb2897e51478f0e2f5ad329eb3bece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Sat, 6 Jun 2020 23:20:49 +0200 Subject: [PATCH 1/8] Refactor main flow, plugin and configuration handling * The plugin handling has been moved out of the `KnDefaultCommand` constructor where it was executed as a side-effect. The original code from `kubectl` suffers from the same issue that plugin handling is not a top-level concern but was very likely introduced as an after-thought. Instead, the plugin handling is done now by a `PluginManager` which is explicitly called in `main()`. * Configuration and bootstrap option handling is centralized in the package `option`. After the bootstrap happened, the content of the configuration file, as well as any other global configuration, can be obtained from methods on `config.GlobalConfig`. Also, all flag handling is delegated to cobra so that no own parsing is needed. * Many of the logic in `pkg/kn/commands/plugin` for plugin management has been moved up to `pkg/kn/plugin` as this code is not only relevant for `plugin list` but also for the bootstrap process. --- cmd/kn/main.go | 136 +++++- cmd/kn/main_test.go | 194 ++++++++ docs/cmd/kn.md | 10 +- docs/cmd/kn_completion.md | 4 +- docs/cmd/kn_plugin.md | 8 +- docs/cmd/kn_plugin_list.md | 15 +- docs/cmd/kn_revision.md | 4 +- docs/cmd/kn_revision_delete.md | 4 +- docs/cmd/kn_revision_describe.md | 4 +- docs/cmd/kn_revision_list.md | 4 +- docs/cmd/kn_route.md | 4 +- docs/cmd/kn_route_describe.md | 4 +- docs/cmd/kn_route_list.md | 4 +- docs/cmd/kn_service.md | 4 +- docs/cmd/kn_service_create.md | 4 +- docs/cmd/kn_service_delete.md | 4 +- docs/cmd/kn_service_describe.md | 4 +- docs/cmd/kn_service_export.md | 4 +- docs/cmd/kn_service_list.md | 4 +- docs/cmd/kn_service_update.md | 4 +- docs/cmd/kn_source.md | 4 +- docs/cmd/kn_source_apiserver.md | 4 +- docs/cmd/kn_source_apiserver_create.md | 4 +- docs/cmd/kn_source_apiserver_delete.md | 4 +- docs/cmd/kn_source_apiserver_describe.md | 4 +- docs/cmd/kn_source_apiserver_list.md | 4 +- docs/cmd/kn_source_apiserver_update.md | 4 +- docs/cmd/kn_source_binding.md | 4 +- docs/cmd/kn_source_binding_create.md | 4 +- docs/cmd/kn_source_binding_delete.md | 4 +- docs/cmd/kn_source_binding_describe.md | 4 +- docs/cmd/kn_source_binding_list.md | 4 +- docs/cmd/kn_source_binding_update.md | 4 +- docs/cmd/kn_source_list-types.md | 4 +- docs/cmd/kn_source_list.md | 4 +- docs/cmd/kn_source_ping.md | 4 +- docs/cmd/kn_source_ping_create.md | 4 +- docs/cmd/kn_source_ping_delete.md | 4 +- docs/cmd/kn_source_ping_describe.md | 4 +- docs/cmd/kn_source_ping_list.md | 4 +- docs/cmd/kn_source_ping_update.md | 4 +- docs/cmd/kn_trigger.md | 4 +- docs/cmd/kn_trigger_create.md | 4 +- docs/cmd/kn_trigger_delete.md | 4 +- docs/cmd/kn_trigger_describe.md | 4 +- docs/cmd/kn_trigger_list.md | 4 +- docs/cmd/kn_trigger_update.md | 4 +- docs/cmd/kn_version.md | 4 +- docs/dev/developer-guide.md | 152 +++++++ go.mod | 1 + hack/generate-docs.go | 10 +- pkg/kn/commands/completion/completion_test.go | 79 ++-- pkg/kn/commands/flags/sink.go | 28 +- pkg/kn/commands/plugin/handler.go | 161 ------- pkg/kn/commands/plugin/handler_test.go | 198 --------- pkg/kn/commands/plugin/list.go | 132 +++--- pkg/kn/commands/plugin/list_flags_test.go | 47 -- pkg/kn/commands/plugin/list_test.go | 314 +++++-------- pkg/kn/commands/plugin/plugin.go | 23 +- pkg/kn/commands/plugin/plugin_test.go | 56 +-- pkg/kn/commands/plugin/plugin_test_helper.go | 69 --- pkg/kn/commands/plugin/verifier.go | 304 ------------- pkg/kn/commands/plugin/verifier_test.go | 241 ---------- pkg/kn/commands/plugin/verifier_windows.go | 191 -------- pkg/kn/commands/testing_helper.go | 125 ++---- pkg/kn/commands/testing_helper_test.go | 123 ++---- pkg/kn/commands/types.go | 45 -- pkg/kn/config/config.go | 252 +++++++++++ pkg/kn/config/config_test.go | 141 ++++++ pkg/kn/config/testing.go | 33 ++ pkg/kn/config/testing_test.go | 37 ++ pkg/kn/config/types.go | 71 +++ pkg/kn/core/root.go | 416 ------------------ pkg/kn/core/root_test.go | 242 ---------- pkg/kn/plugin/manager.go | 350 +++++++++++++++ pkg/kn/plugin/manager_test.go | 221 ++++++++++ .../plugin/list_flags.go => plugin/stat.go} | 28 +- pkg/kn/plugin/stat_windows.go | 24 + pkg/kn/plugin/verify.go | 259 +++++++++++ pkg/kn/plugin/verify_test.go | 187 ++++++++ pkg/kn/root/root.go | 140 ++++++ pkg/kn/root/root_test.go | 132 ++++++ test/e2e/plugins_test.go | 24 +- 83 files changed, 2739 insertions(+), 2652 deletions(-) create mode 100644 cmd/kn/main_test.go create mode 100644 docs/dev/developer-guide.md delete mode 100644 pkg/kn/commands/plugin/handler.go delete mode 100644 pkg/kn/commands/plugin/handler_test.go delete mode 100644 pkg/kn/commands/plugin/list_flags_test.go delete mode 100644 pkg/kn/commands/plugin/plugin_test_helper.go delete mode 100644 pkg/kn/commands/plugin/verifier.go delete mode 100644 pkg/kn/commands/plugin/verifier_test.go delete mode 100644 pkg/kn/commands/plugin/verifier_windows.go create mode 100644 pkg/kn/config/config.go create mode 100644 pkg/kn/config/config_test.go create mode 100644 pkg/kn/config/testing.go create mode 100644 pkg/kn/config/testing_test.go create mode 100644 pkg/kn/config/types.go delete mode 100644 pkg/kn/core/root.go delete mode 100644 pkg/kn/core/root_test.go create mode 100644 pkg/kn/plugin/manager.go create mode 100644 pkg/kn/plugin/manager_test.go rename pkg/kn/{commands/plugin/list_flags.go => plugin/stat.go} (52%) create mode 100644 pkg/kn/plugin/stat_windows.go create mode 100644 pkg/kn/plugin/verify.go create mode 100644 pkg/kn/plugin/verify_test.go create mode 100644 pkg/kn/root/root.go create mode 100644 pkg/kn/root/root_test.go diff --git a/cmd/kn/main.go b/cmd/kn/main.go index b7e889361f..4377742cd6 100644 --- a/cmd/kn/main.go +++ b/cmd/kn/main.go @@ -18,37 +18,143 @@ import ( "fmt" "math/rand" "os" + "strings" "time" - "github.com/spf13/viper" - "knative.dev/client/pkg/kn/core" + "github.com/pkg/errors" + "github.com/spf13/cobra" + + "knative.dev/client/pkg/kn/config" + "knative.dev/client/pkg/kn/plugin" + "knative.dev/client/pkg/kn/root" ) func init() { - core.InitializeConfig() + rand.Seed(time.Now().UnixNano()) } -var err error - func main() { - defer cleanup() - rand.Seed(time.Now().UnixNano()) - kn, err := core.NewDefaultKnCommand() + err := run(os.Args[1:]) if err != nil { - fmt.Fprintln(os.Stderr, err) + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + // This is the only point from where to exit when an error occurs os.Exit(1) } +} + +// 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 + // 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 + } + + // Strip of all flags to get the non-flag commands only + commands, err := stripFlags(args) + if err != nil { + 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 err + } + + // Create kn root command and all sub-commands + rootCmd, err := root.NewRootCommand() + if err != nil { + return err + } - if err := kn.Execute(); err != nil { - if err.Error() != "subcommand is required" { - fmt.Fprintln(os.Stderr, err) + if plugin != nil { + // Validate & Execute plugin + err = validatePlugin(rootCmd, plugin) + if err != nil { + return err } - os.Exit(1) + + return plugin.Execute(argsWithoutCommands(args, plugin.CommandParts())) + } else { + // Execute kn root command + return rootCmd.Execute() } } -func cleanup() { +// Get only the args provided but no options. The extraction +// process is a bit tricky as Cobra doesn't provide such +// functionality out of the box +func stripFlags(args []string) ([]string, error) { + // Store all command + commandsFound := &[]string{} + + // Use a canary command that allows all options and only extracts + // commands. Doesn't work with arbitrary boolean flags but is good enough + // for us here + extractCommand := cobra.Command{ + Run: func(cmd *cobra.Command, args []string) { + for _, arg := range args { + *commandsFound = append(*commandsFound, arg) + } + }, + } + + // Filter out --help and -h options to avoid special treatment which we don't + // need here + extractCommand.SetArgs(filterHelpOptions(args)) + + // Adding all global flags here + config.AddBootstrapFlags(extractCommand.Flags()) + + // Allow all options + extractCommand.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: true} + + // Execute to get to the command args + err := extractCommand.Execute() + if err != nil { + return nil, err + } + return *commandsFound, nil +} + +// Strip all plugin commands before calling out to the plugin +func argsWithoutCommands(cmdArgs []string, pluginCommandsParts []string) []string { + var ret []string + for _, arg := range cmdArgs { + if len(pluginCommandsParts) > 0 && pluginCommandsParts[0] == arg { + pluginCommandsParts = pluginCommandsParts[1:] + continue + } + ret = append(ret, arg) + } + return ret +} + +// Remove all help options +func filterHelpOptions(args []string) []string { + var ret []string + for _, arg := range args { + if arg != "-h" && arg != "--help" { + ret = append(ret, arg) + } + } + return ret +} + +// Check if the plugin collides with any command specified in the root command +func validatePlugin(root *cobra.Command, plugin plugin.Plugin) error { + // Check if a given plugin can be identified as a command + cmd, args, err := root.Find(plugin.CommandParts()) + if err == nil { - viper.WriteConfig() + if !cmd.HasSubCommands() || // a leaf command can't be overridden + cmd.HasSubCommands() && len(args) == 0 { // a group can't be overridden either + return errors.Errorf("plugin %s is overriding built-in command '%s' which is not allowed", plugin.Path(), strings.Join(plugin.CommandParts(), " ")) + } } + return nil } diff --git a/cmd/kn/main_test.go b/cmd/kn/main_test.go new file mode 100644 index 0000000000..6051cc6b41 --- /dev/null +++ b/cmd/kn/main_test.go @@ -0,0 +1,194 @@ +// Copyright © 2018 The Knative 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 main + +import ( + "fmt" + "os" + "testing" + + "github.com/spf13/cobra" + "gotest.tools/assert" + + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/util" +) + +func TestValidatePlugin(t *testing.T) { + + // Build up simple command hierarchy + root := cobra.Command{} + one := &cobra.Command{Use: "one"} + one.AddCommand(&cobra.Command{Use: "eins"}, &cobra.Command{Use: "zwei"}) + two := &cobra.Command{Use: "two"} + root.AddCommand(one, two) + + data := []struct { + givenPluginCommandParts []string + expectedErrors []string + }{ + { + // Allowed because it add a new top-level plugin + []string{"test"}, + nil, + }, + { + // Allowed because it adds to an existing command-group + []string{"one", "drei"}, + nil, + }, + { + // Forbidden because it overrides an command-group + []string{"one"}, + []string{"pluginPath", "one"}, + }, + { + // Forbidden because it overrides a leaf-command + []string{"one", "zwei"}, + []string{"pluginPath", "one", "zwei"}, + }, + { + // Forbidden because it would mis-use a leaf-comman to a command-group + []string{"one", "zwei", "trois"}, + []string{"pluginPath", "one", "zwei"}, + }, + { + // Forbidden because it overrides a (top-level) leaf-command + []string{"two"}, + []string{"pluginPath", "two"}, + }, + { + // Forbidden because it would add to a leaf command + []string{"two", "deux", "and", "more"}, + []string{"pluginPath", "two", "deux"}, + }, + } + + for i, d := range data { + step := fmt.Sprintf("Check %d", i) + err := validatePlugin(&root, commandPartsOnlyPlugin(d.givenPluginCommandParts)) + if len(d.expectedErrors) == 0 { + assert.NilError(t, err, step) + } else { + assert.Assert(t, err != nil, step) + assert.Assert(t, util.ContainsAll(err.Error(), d.expectedErrors...), step) + } + } + +} + +// Used above for wrapping the command part to check +type commandPartsOnlyPlugin []string + +func (f commandPartsOnlyPlugin) CommandParts() []string { return f } +func (f commandPartsOnlyPlugin) Name() string { return "" } +func (f commandPartsOnlyPlugin) Execute(args []string) error { return nil } +func (f commandPartsOnlyPlugin) Description() (string, error) { return "", nil } +func (f commandPartsOnlyPlugin) Path() string { return "pluginPath" } + +func TestArgsWithoutCommands(t *testing.T) { + data := []struct { + givenCmdArgs []string + givenPluginCommandParts []string + expectedResult []string + }{ + { + []string{"--option", "val", "one", "second", "rest"}, + []string{"one", "second"}, + []string{"--option", "val", "rest"}, + }, + { + []string{"--option", "val", "one", "second", "rest"}, + []string{"second", "one"}, + []string{"--option", "val", "one", "rest"}, + }, + { + []string{"--option", "val", "one", "second", "third", "one", "rest"}, + []string{"second", "one"}, + []string{"--option", "val", "one", "third", "rest"}, + }, + } + for _, d := range data { + result := argsWithoutCommands(d.givenCmdArgs, d.givenPluginCommandParts) + assert.DeepEqual(t, result, d.expectedResult) + } +} + +func TestStripFlags(t *testing.T) { + + data := []struct { + givenArgs []string + expectedCommands []string + expectedError string + }{ + { + []string{"test", "-h", "second", "--bla"}, + []string{"test", "second"}, + "", + }, + { + []string{"--help", "test"}, + []string{"test"}, + "", + }, + { + []string{"--unknown-option", "bla", "test", "second"}, + []string{"test", "second"}, + "", + }, + { + []string{"--lookup-plugins", "bla", "test", "second"}, + []string{"bla", "test", "second"}, + "", + }, + { + []string{"--config-file", "bla", "test", "second"}, + []string{"test", "second"}, + "", + }, + { + []string{"test"}, + []string{"test"}, + "", + }, + } + + for i, f := range data { + step := fmt.Sprintf("Check %d", i) + commands, err := stripFlags(f.givenArgs) + assert.DeepEqual(t, commands, f.expectedCommands) + if f.expectedError != "" { + assert.ErrorContains(t, err, f.expectedError, step) + } else { + assert.NilError(t, err, step) + } + } +} + +// Smoke test +func TestRun(t *testing.T) { + oldArgs := os.Args + os.Args = []string{"kn", "--config", "/no/config/please.yaml", "version"} + defer (func() { + os.Args = oldArgs + })() + + capture := commands.CaptureStdout(t) + err := run(os.Args[1:]) + out := capture.Close() + + assert.NilError(t, err) + assert.Assert(t, util.ContainsAllIgnoreCase(out, "version", "build", "git")) +} diff --git a/docs/cmd/kn.md b/docs/cmd/kn.md index b25f5100d9..3c45095896 100644 --- a/docs/cmd/kn.md +++ b/docs/cmd/kn.md @@ -12,12 +12,10 @@ Manage your Knative building blocks: ### Options ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - -h, --help help for kn - --kubeconfig string kubectl config file (default is ~/.kube/config) - --log-http log http traffic - --lookup-plugins look for kn plugins in $PATH - --plugins-dir string kn plugins directory (default "~/.config/kn/plugins") + --config string kn configuration file (default: ~/.config/kn/config.yaml) + -h, --help help for kn + --kubeconfig string kubectl configuration file (default: ~/.kube/config) + --log-http log http traffic ``` ### SEE ALSO diff --git a/docs/cmd/kn_completion.md b/docs/cmd/kn_completion.md index c7d9030c2e..bf751f438f 100644 --- a/docs/cmd/kn_completion.md +++ b/docs/cmd/kn_completion.md @@ -37,8 +37,8 @@ kn completion [SHELL] [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_plugin.md b/docs/cmd/kn_plugin.md index f3b8f521ed..e5fbcccd0e 100644 --- a/docs/cmd/kn_plugin.md +++ b/docs/cmd/kn_plugin.md @@ -16,16 +16,14 @@ kn plugin [flags] ### Options ``` - -h, --help help for plugin - --lookup-plugins look for kn plugins in $PATH - --plugins-dir string kn plugins directory (default "~/.config/kn/plugins") + -h, --help help for plugin ``` ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_plugin_list.md b/docs/cmd/kn_plugin_list.md index 3fde1fe27e..f424b55053 100644 --- a/docs/cmd/kn_plugin_list.md +++ b/docs/cmd/kn_plugin_list.md @@ -9,8 +9,8 @@ List all installed plugins. Available plugins are those that are: - executable - begin with "kn-" -- Kn's plugin directory ~/.config/kn/plugins -- Anywhere in the execution $PATH (if lookupInPath config variable is enabled) +- Kn's plugin directory +- Anywhere in the execution $PATH (if plugins.path-lookup config variable is enabled) ``` kn plugin list [flags] @@ -19,18 +19,15 @@ kn plugin list [flags] ### Options ``` - -h, --help help for list - --lookup-plugins look for kn plugins in $PATH - --name-only If true, display only the binary name of each plugin, rather than its full path - --plugins-dir string kn plugins directory (default "~/.config/kn/plugins") - --verbose verbose output + -h, --help help for list + --verbose verbose output ``` ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_revision.md b/docs/cmd/kn_revision.md index 07e85c765d..5097d39c32 100644 --- a/docs/cmd/kn_revision.md +++ b/docs/cmd/kn_revision.md @@ -19,8 +19,8 @@ kn revision [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_revision_delete.md b/docs/cmd/kn_revision_delete.md index f12e129ef4..1d7b55e7d6 100644 --- a/docs/cmd/kn_revision_delete.md +++ b/docs/cmd/kn_revision_delete.md @@ -32,8 +32,8 @@ kn revision delete NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_revision_describe.md b/docs/cmd/kn_revision_describe.md index 48a6b2c58d..933625bd2e 100644 --- a/docs/cmd/kn_revision_describe.md +++ b/docs/cmd/kn_revision_describe.md @@ -24,8 +24,8 @@ kn revision describe NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_revision_list.md b/docs/cmd/kn_revision_list.md index 3683058176..4984a990b5 100644 --- a/docs/cmd/kn_revision_list.md +++ b/docs/cmd/kn_revision_list.md @@ -43,8 +43,8 @@ kn revision list [name] [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_route.md b/docs/cmd/kn_route.md index 98439f6165..d108848434 100644 --- a/docs/cmd/kn_route.md +++ b/docs/cmd/kn_route.md @@ -19,8 +19,8 @@ kn route [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_route_describe.md b/docs/cmd/kn_route_describe.md index 87e58af6b5..1989872462 100644 --- a/docs/cmd/kn_route_describe.md +++ b/docs/cmd/kn_route_describe.md @@ -24,8 +24,8 @@ kn route describe NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_route_list.md b/docs/cmd/kn_route_list.md index e9aa390d02..bda0b62704 100644 --- a/docs/cmd/kn_route_list.md +++ b/docs/cmd/kn_route_list.md @@ -39,8 +39,8 @@ kn route list NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_service.md b/docs/cmd/kn_service.md index bd4794f9fd..d7e4563627 100644 --- a/docs/cmd/kn_service.md +++ b/docs/cmd/kn_service.md @@ -19,8 +19,8 @@ kn service [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 8519b29dfc..ff970191f8 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -94,8 +94,8 @@ kn service create NAME --image IMAGE [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_service_delete.md b/docs/cmd/kn_service_delete.md index deaf837776..56fed786e8 100644 --- a/docs/cmd/kn_service_delete.md +++ b/docs/cmd/kn_service_delete.md @@ -39,8 +39,8 @@ kn service delete NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_service_describe.md b/docs/cmd/kn_service_describe.md index 7d2e862e65..fe5db58aea 100644 --- a/docs/cmd/kn_service_describe.md +++ b/docs/cmd/kn_service_describe.md @@ -24,8 +24,8 @@ kn service describe NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_service_export.md b/docs/cmd/kn_service_export.md index e439bc657d..ffa9d20c65 100644 --- a/docs/cmd/kn_service_export.md +++ b/docs/cmd/kn_service_export.md @@ -39,8 +39,8 @@ kn service export NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_service_list.md b/docs/cmd/kn_service_list.md index 6e7c92a7dc..0e4fca7666 100644 --- a/docs/cmd/kn_service_list.md +++ b/docs/cmd/kn_service_list.md @@ -39,8 +39,8 @@ kn service list [name] [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index e4ea08e070..9d90996501 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -84,8 +84,8 @@ kn service update NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source.md b/docs/cmd/kn_source.md index 5872fe46f3..c375311906 100644 --- a/docs/cmd/kn_source.md +++ b/docs/cmd/kn_source.md @@ -19,8 +19,8 @@ kn source [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_apiserver.md b/docs/cmd/kn_source_apiserver.md index c252364de0..89e9cd5d01 100644 --- a/docs/cmd/kn_source_apiserver.md +++ b/docs/cmd/kn_source_apiserver.md @@ -19,8 +19,8 @@ kn source apiserver [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_apiserver_create.md b/docs/cmd/kn_source_apiserver_create.md index f13c76ba93..e1eea6dd5d 100644 --- a/docs/cmd/kn_source_apiserver_create.md +++ b/docs/cmd/kn_source_apiserver_create.md @@ -36,8 +36,8 @@ kn source apiserver create NAME --resource RESOURCE --service-account ACCOUNTNAM ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_apiserver_delete.md b/docs/cmd/kn_source_apiserver_delete.md index a21c5dd0aa..45820828e6 100644 --- a/docs/cmd/kn_source_apiserver_delete.md +++ b/docs/cmd/kn_source_apiserver_delete.md @@ -28,8 +28,8 @@ kn source apiserver delete NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_apiserver_describe.md b/docs/cmd/kn_source_apiserver_describe.md index 1477f0d46a..d214ed9eed 100644 --- a/docs/cmd/kn_source_apiserver_describe.md +++ b/docs/cmd/kn_source_apiserver_describe.md @@ -29,8 +29,8 @@ kn source apiserver describe NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_apiserver_list.md b/docs/cmd/kn_source_apiserver_list.md index ff2c9c000d..5fc4c20407 100644 --- a/docs/cmd/kn_source_apiserver_list.md +++ b/docs/cmd/kn_source_apiserver_list.md @@ -36,8 +36,8 @@ kn source apiserver list [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_apiserver_update.md b/docs/cmd/kn_source_apiserver_update.md index d238a8356c..959efccb0d 100644 --- a/docs/cmd/kn_source_apiserver_update.md +++ b/docs/cmd/kn_source_apiserver_update.md @@ -36,8 +36,8 @@ kn source apiserver update NAME --resource RESOURCE --service-account ACCOUNTNAM ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_binding.md b/docs/cmd/kn_source_binding.md index e934818c33..cbe18cc8e7 100644 --- a/docs/cmd/kn_source_binding.md +++ b/docs/cmd/kn_source_binding.md @@ -19,8 +19,8 @@ kn source binding [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_binding_create.md b/docs/cmd/kn_source_binding_create.md index 93ccbccdf1..040e40a182 100644 --- a/docs/cmd/kn_source_binding_create.md +++ b/docs/cmd/kn_source_binding_create.md @@ -31,8 +31,8 @@ kn source binding create NAME --subject SUBJECT --sink SINK --ce-override KEY=VA ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_binding_delete.md b/docs/cmd/kn_source_binding_delete.md index cb4e6e28a9..349a75c85f 100644 --- a/docs/cmd/kn_source_binding_delete.md +++ b/docs/cmd/kn_source_binding_delete.md @@ -28,8 +28,8 @@ kn source binding delete NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_binding_describe.md b/docs/cmd/kn_source_binding_describe.md index e40ac69aaa..b0a0ee1717 100644 --- a/docs/cmd/kn_source_binding_describe.md +++ b/docs/cmd/kn_source_binding_describe.md @@ -29,8 +29,8 @@ kn source binding describe NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_binding_list.md b/docs/cmd/kn_source_binding_list.md index 29af0ffaaa..9400f435c0 100644 --- a/docs/cmd/kn_source_binding_list.md +++ b/docs/cmd/kn_source_binding_list.md @@ -36,8 +36,8 @@ kn source binding list [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_binding_update.md b/docs/cmd/kn_source_binding_update.md index a620d4b745..b537e45331 100644 --- a/docs/cmd/kn_source_binding_update.md +++ b/docs/cmd/kn_source_binding_update.md @@ -31,8 +31,8 @@ kn source binding update NAME --subject SCHEDULE --sink SINK --ce-override OVERR ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_list-types.md b/docs/cmd/kn_source_list-types.md index f5e119b424..f844b24fb0 100644 --- a/docs/cmd/kn_source_list-types.md +++ b/docs/cmd/kn_source_list-types.md @@ -35,8 +35,8 @@ kn source list-types [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_list.md b/docs/cmd/kn_source_list.md index b9bfd5572b..1686ec94b3 100644 --- a/docs/cmd/kn_source_list.md +++ b/docs/cmd/kn_source_list.md @@ -40,8 +40,8 @@ kn source list [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_ping.md b/docs/cmd/kn_source_ping.md index 7e64945ac9..3e4337b490 100644 --- a/docs/cmd/kn_source_ping.md +++ b/docs/cmd/kn_source_ping.md @@ -19,8 +19,8 @@ kn source ping [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_ping_create.md b/docs/cmd/kn_source_ping_create.md index 0869c9e60a..39d308e279 100644 --- a/docs/cmd/kn_source_ping_create.md +++ b/docs/cmd/kn_source_ping_create.md @@ -32,8 +32,8 @@ kn source ping create NAME --schedule SCHEDULE --sink SINK --data DATA [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_ping_delete.md b/docs/cmd/kn_source_ping_delete.md index e654eabbee..9ba26aa3a8 100644 --- a/docs/cmd/kn_source_ping_delete.md +++ b/docs/cmd/kn_source_ping_delete.md @@ -28,8 +28,8 @@ kn source ping delete NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_ping_describe.md b/docs/cmd/kn_source_ping_describe.md index 5a007dff8a..29b11ae32b 100644 --- a/docs/cmd/kn_source_ping_describe.md +++ b/docs/cmd/kn_source_ping_describe.md @@ -29,8 +29,8 @@ kn source ping describe NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_ping_list.md b/docs/cmd/kn_source_ping_list.md index beb1741d29..d7835c7c12 100644 --- a/docs/cmd/kn_source_ping_list.md +++ b/docs/cmd/kn_source_ping_list.md @@ -36,8 +36,8 @@ kn source ping list [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_source_ping_update.md b/docs/cmd/kn_source_ping_update.md index c2c8445581..ffdef3c543 100644 --- a/docs/cmd/kn_source_ping_update.md +++ b/docs/cmd/kn_source_ping_update.md @@ -32,8 +32,8 @@ kn source ping update NAME --schedule SCHEDULE --sink SERVICE --data DATA [flags ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_trigger.md b/docs/cmd/kn_trigger.md index 9ff2c3989d..f80c8fb3f3 100644 --- a/docs/cmd/kn_trigger.md +++ b/docs/cmd/kn_trigger.md @@ -19,8 +19,8 @@ kn trigger [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_trigger_create.md b/docs/cmd/kn_trigger_create.md index d480786b6e..036079f795 100644 --- a/docs/cmd/kn_trigger_create.md +++ b/docs/cmd/kn_trigger_create.md @@ -35,8 +35,8 @@ kn trigger create NAME --broker BROKER --sink SINK [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_trigger_delete.md b/docs/cmd/kn_trigger_delete.md index b610cf0405..b54419ea3a 100644 --- a/docs/cmd/kn_trigger_delete.md +++ b/docs/cmd/kn_trigger_delete.md @@ -28,8 +28,8 @@ kn trigger delete NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_trigger_describe.md b/docs/cmd/kn_trigger_describe.md index e6bdf8b402..9fb992c596 100644 --- a/docs/cmd/kn_trigger_describe.md +++ b/docs/cmd/kn_trigger_describe.md @@ -29,8 +29,8 @@ kn trigger describe NAME [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_trigger_list.md b/docs/cmd/kn_trigger_list.md index 16ca65127f..2c1c124ed9 100644 --- a/docs/cmd/kn_trigger_list.md +++ b/docs/cmd/kn_trigger_list.md @@ -36,8 +36,8 @@ kn trigger list [name] [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_trigger_update.md b/docs/cmd/kn_trigger_update.md index 4dab1adb5d..c4fe3a1867 100644 --- a/docs/cmd/kn_trigger_update.md +++ b/docs/cmd/kn_trigger_update.md @@ -39,8 +39,8 @@ kn trigger update NAME --filter KEY=VALUE --sink SINK [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/cmd/kn_version.md b/docs/cmd/kn_version.md index fcf26b8400..2ad1433643 100644 --- a/docs/cmd/kn_version.md +++ b/docs/cmd/kn_version.md @@ -20,8 +20,8 @@ kn version [flags] ### Options inherited from parent commands ``` - --config string kn config file (default is ~/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is ~/.kube/config) + --config string kn configuration file (default: ~/.config/kn/config.yaml) + --kubeconfig string kubectl configuration file (default: ~/.kube/config) --log-http log http traffic ``` diff --git a/docs/dev/developer-guide.md b/docs/dev/developer-guide.md new file mode 100644 index 0000000000..19308d1b6c --- /dev/null +++ b/docs/dev/developer-guide.md @@ -0,0 +1,152 @@ +# `kn` Developer Guide + +This guide described the high-level architectural concepts of the Knative CLI client `kn`. +It is for all developers who contribute to `kn` and want to understand some essential concepts. +For any contributor to `kn`, it is highly recommended reading this document. + +_This guide is not complete and has still many gaps that we are going to fill over timer. Contributions are highly appreciated ;-)_ + +## Flow + +The journey starts at [main.go]() which is the entry point when you call `kn`. +You find here the main control flow, which can be roughly divided into three phases: + + - _Bootstrap_ is about retrieving essential configuration parameters from command-line flags, and a configuration file. + - _Plugin Lookup_ finds out whether the user wants to execute a plugin or a built-in command + - _Execution_ : Execute a plugin or the `RootCommand` for built-in commands + +Only the code in the `main` function can use `os.Exit` for leaving the process. +It evaluates any error that bubbles up and prints it out as an error message before exiting. +There is no exception to this rule. + +Let's talk now about the three phases separately. + +### Bootstrap + +The bootstrap performed by [config.BootstrapConfig()]() extracts all the options relevant for config file detection and plugin configuration. +The bootstrap process does not fully parse all arguments but only those that are relevant for starting up and for looking up any plugin. +The configuration can be either provided via a `--config` flag or is picked up from a default location. +The default configuration location conforms to the [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html) and is different for Unix systems and Windows systems. + +[Viper](https://github.com/spf13/viper) loads the configuration file (if any) form this location, and the `config` package makes all the bootstrap and other configuration information available vial the `config.GlobalConfig` singleton (which btw can be easily mocked for unit tests by just replacing it with another implementation of `config.Config` interface) + +### Plugin Lookup + +In the next step, a `PluginManager` checks whether the given command-line arguments are pointing to a plugin. +All non-flag arguments are extracted and then used to lookup via [plugin.PluginManager.FindPlugin()]() in the plugin directory (and the execution `$PATH` if configured) calculated in the _Bootstrap_ phase. + +### Execution + +If `kn` detects a plugin, it is first validated and then executed. +If not, the flow executes the `RootCommand` which carries all builtin `kn` commands. + +However, before `kn` executes a plugin, it is first validated whether it does not clash with a builtin command or command group. +If the validation succeeds, the `PluginManager` calls out to the identified plugin via the `Plugin.Execute()` interface method. + +If `kn` does not detect a plugin, the `RootCommand` is executed, which is internally dispatched, based on the given commands, to the matching sub-command. +In the case that no sub-command matches the provided command arguments on the command line, kn throws an error. + +## Configuration + +For any command that is executed by `kn`, the configuration can come from two places: + +* Arguments and flags as provided on the command line +* Configuration stored in a configuration file + +`kn` has a _bootstrap configuration_ which consists of these variables: + +* `configFile` is the location of the configuration file which by default is `~/.config/kn/config.yaml` on Unix like systems and `%APPDATA%\kn` on Windows of now overridden with the environment variable `XDG_CONFIG_HOME`. It can be specified on the command line with `--config`. +* `pluginsDir` is the location of the directory holding `kn` plugins. By default, this is `${configFile}/plugins`. The default can be overridden in with the field `plugins.directory` in the configuration file. +* `lookupPluginsInPath` is a boolean flag that, when set to `true`, specifies that plugins should also be looked up in the execution `$PATH` (default: `false`). Use `plugins.lookup-path` in the configuration file to override it. + + +The main flow calls out to `config.BootstrapConfig()` to set these three variables by evaluating the location of the configuration file first and then reading in the configuration. + +All other configuration that is stored in the configuration is also available after the bootstrap. +To access this information, any other code can access the methods of `config.GlobalConfig` which holds an implementation of the `config.Config` interface. +For testing purposes, `config.GlobalConfig` can be replaced by a mock implementation while running a test. Use `config.TestConfig` for simple use cases. + +## Flags + +_This section will hold some information about how flag parsing of commands is supposed to work, including using helper functions for enforcing the conventions defined in [cli conventions](https://github.com/knative/client/blob/master/conventions/cli.md)_ + +## Clients for accessing the backend + +_In this section we are going to describe how commands can access the backend via the `kn` public API which provides simplified access mechanism to the Knative serving and eventing backends_ + +## Main Packages + +Here is a rough overview (as of 2020-06) of the directories hierarchy and packages that organized the `kn` codebase. + +### `cmd/kn` + +This directory holds the `main` package and is the entry point for calling `kn`. +The `main` package is responsible for the overall flow as described above in the section "Flow". + +Potential future CLI commands should be added on the `cmd/` level. + +### `pkg/kn` + +The directory `kn` holds the code that is specific to the `kn` builtin commands. +Everything below `pkg/kn` is not supposed to be shared outside of `knative/client`. + +The following essential packages/directories are stored here: + + - `root` is the package that defines the `RootCommand` which is the top-level [cobra](https://github.com/spf13/cobra) command. The `RootCommand` contains all other command groups and leaf commands. + - `commands` is a directory that contains all `kn` command definitions that are registered at the `RootCommand`. Here, we distinguish two kinds of commands: + + **Command Groups** have sub-commands but no code to run on its own. Example: `kn service` + + **Leaf Commands** have no sub-commands and execute the business logic. Example: `kn service create` + - `config` is the package for all global configuration handling. The global configuration allows access to the kn configuration file and all top-level options that apply to all commands. + - `flags` is the package for all flag-parsing utility methods shared across all commands. Some of its content might end up in the `kn public API` packages (see below for more about the `kn` public API). + - `plugin` has the code for finding, listing and executing plugins. + - `traffic` has functions for dealing with traffic split. + +### `pkg` (APIs) + +Besides the `pkg/kn` packages, `pkg` contains packages that are the current public API of `kn` for reuse outside the `knative/client` project. +In the future, these packages are grouped more explicitly in a "public API" package (see below for more on information on this). + +The following packages provide simplified access to the corresponding backend API and add some extra functionality like synchronous wait for the completion of an operation like creating a Knative service. + + - `serving` interface to Knative Serving with support for synchronous operations + - `eventing` interface for access Knative Eventing backend objects like Triggers + - `dynamic` is a dynamic client for interacting with the backend in a typeless way + - `sources` has the client access code to the built-in source `ApiServerSource`, `PingSource` and `SinkBinding`. + +These packages also determine the API version used for the backend APIs which typically over multiple API versions. +All packages also contain a record-replay style mocking library to be used in Mock testing. + +### `pkg` (Misc) + +Finally, `pkg` contains also some miscellaneous package for different purposes. +Some of them might end up in the public API, too. + + - `util` : Kitchen-sink for everything that does not find in any other category. Please try to avoid adding files here except for good reasons. + - `wait` : Helper methods for implementing synchronous calls to backends + - `printers` : Helper for printing out CLI output in a kn specific way + +## Plugins + +You can find the central plugin handling in the package `pkg/kn/plugin`. +The entry point here is the `PluginManager`, which is responsible for finding plugins by name and for listing all visible plugins. +An interface `Plugin` describes a plugin. +This interface has currently a single implementation that uses an `exec` system call to run an external plugin command following a naming convention. +Only the `main()` flow looks up and evaluates plugins, except the commands from the `plugin` group. + +## Public API + +_Describe here the public API package that we also expose to external clients, like e.g. plugins. This public API package is not available yet but will contain `kn` abstraction for accessing backend APIs (serving, eventing, sources, dynamic), the flag-parsing library for enforcing the CLI conventions and any other (utility) code to share with other parties_ + +## Testing + +### Mock Testing + +_Describe here the way how our mock testing framework works and that is the preferred way of testing and newly written tests are supposed to use the mock testing style_ + +### Fake Testing + +Fake testing uses the auto-generated fake services provided by the client libraries. Some older tests use these directly but also tests involving the dynamic client. This test couples the code tightly to the external client's API and its version, which we try to avoid so that all our code goes through the client-side APIs that we provide. + +_Explain Fake testing more in this section_ + + diff --git a/go.mod b/go.mod index 94df4d8471..c2d9f2eae9 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.14 require ( github.com/mitchellh/go-homedir v1.1.0 + github.com/pkg/errors v0.9.1 github.com/spf13/cobra v0.0.6 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.6.2 diff --git a/hack/generate-docs.go b/hack/generate-docs.go index 528b4bad55..fe52b10e76 100644 --- a/hack/generate-docs.go +++ b/hack/generate-docs.go @@ -23,22 +23,24 @@ import ( "strings" "github.com/spf13/cobra/doc" - "knative.dev/client/pkg/kn/core" + "knative.dev/client/pkg/kn/root" ) func main() { - rootCmd := core.NewKnCommand() + rootCmd, err := root.NewRootCommand() + if err != nil { + log.Panicf("Can not create root command: %v", err) + } dir := "." if len(os.Args) > 1 { dir = os.Args[1] } var withFrontMatter bool - var err error if len(os.Args) > 2 { withFrontMatter, err = strconv.ParseBool(os.Args[2]) if err != nil { - log.Panicf("Invalid argument %s, has to be boolean to switch on/off generation of frontmatter", os.Args[2]) + log.Panicf("Invalid argument %s, has to be boolean to switch on/off generation of frontmatter (%v)", os.Args[2], err) } } prependFunc := emptyString diff --git a/pkg/kn/commands/completion/completion_test.go b/pkg/kn/commands/completion/completion_test.go index 01b2d3ee0e..1bc821e36c 100644 --- a/pkg/kn/commands/completion/completion_test.go +++ b/pkg/kn/commands/completion/completion_test.go @@ -24,60 +24,35 @@ import ( "gotest.tools/assert" ) -func TestCompletion(t *testing.T) { - var ( - fakeRootCmd, completionCmd *cobra.Command - knParams *commands.KnParams - ) - - setup := func() { - knParams = &commands.KnParams{} - completionCmd = NewCompletionCommand(knParams) +func TestCompletionUsage(t *testing.T) { + completionCmd := NewCompletionCommand(&commands.KnParams{}) + assert.Assert(t, util.ContainsAllIgnoreCase(completionCmd.Use, "completion")) + assert.Assert(t, util.ContainsAllIgnoreCase(completionCmd.Short, "completion", "shell")) + assert.Assert(t, completionCmd.RunE == nil) +} - fakeRootCmd = &cobra.Command{} - fakeRootCmd.AddCommand(completionCmd) +func TestCompletionGeneration(t *testing.T) { + for _, shell := range []string{"bash", "zsh"} { + completionCmd := NewCompletionCommand(&commands.KnParams{}) + c := commands.CaptureStdout(t) + completionCmd.Run(&cobra.Command{}, []string{shell}) + out := c.Close() + assert.Assert(t, out != "") } +} - t.Run("creates a CompletionCommand", func(t *testing.T) { - setup() - assert.Equal(t, completionCmd.Use, "completion [SHELL]") - assert.Equal(t, completionCmd.Short, "Output shell completion code") - assert.Assert(t, completionCmd.RunE == nil) - }) - - t.Run("returns completion code for BASH", func(t *testing.T) { - setup() - commands.CaptureStdout(t) - defer commands.ReleaseStdout(t) - - completionCmd.Run(fakeRootCmd, []string{"bash"}) - assert.Assert(t, commands.ReadStdout(t) != "") - }) - - t.Run("returns completion code for Zsh", func(t *testing.T) { - setup() - commands.CaptureStdout(t) - defer commands.ReleaseStdout(t) - - completionCmd.Run(fakeRootCmd, []string{"zsh"}) - assert.Assert(t, commands.ReadStdout(t) != "") - }) - - t.Run("returns error on command without args", func(t *testing.T) { - setup() - commands.CaptureStdout(t) - defer commands.ReleaseStdout(t) - - completionCmd.Run(fakeRootCmd, []string{}) - assert.Assert(t, commands.ReadStdout(t) == "accepts one argument either 'bash' or 'zsh'\n") - }) - - t.Run("returns error on command with invalid args", func(t *testing.T) { - setup() - commands.CaptureStdout(t) - defer commands.ReleaseStdout(t) +func TestCompletionNoArg(t *testing.T) { + completionCmd := NewCompletionCommand(&commands.KnParams{}) + c := commands.CaptureStdout(t) + completionCmd.Run(&cobra.Command{}, []string{}) + out := c.Close() + assert.Assert(t, util.ContainsAll(out, "bash", "zsh", "one", "argument")) +} - completionCmd.Run(fakeRootCmd, []string{"sh"}) - assert.Check(t, util.ContainsAll(commands.ReadStdout(t), "only supports 'bash' or 'zsh' shell completion")) - }) +func TestCompletionWrongArg(t *testing.T) { + completionCmd := NewCompletionCommand(&commands.KnParams{}) + c := commands.CaptureStdout(t) + completionCmd.Run(&cobra.Command{}, []string{"sh"}) + out := c.Close() + assert.Assert(t, util.ContainsAll(out, "bash", "zsh", "only", "supports")) } diff --git a/pkg/kn/commands/flags/sink.go b/pkg/kn/commands/flags/sink.go index 4bc673e47c..d8732b8b64 100644 --- a/pkg/kn/commands/flags/sink.go +++ b/pkg/kn/commands/flags/sink.go @@ -25,7 +25,7 @@ import ( duckv1 "knative.dev/pkg/apis/duck/v1" clientdynamic "knative.dev/client/pkg/dynamic" - "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/config" ) type SinkFlags struct { @@ -34,10 +34,19 @@ type SinkFlags struct { func (i *SinkFlags) Add(cmd *cobra.Command) { cmd.Flags().StringVarP(&i.sink, "sink", "s", "", "Addressable sink for events") + + for _, p := range config.GlobalConfig.SinkMappings() { + //user configration might override the default configuration + sinkMappings[p.Prefix] = schema.GroupVersionResource{ + Resource: p.Resource, + Group: p.Group, + Version: p.Version, + } + } } -// SinkPrefixes maps prefixes used for sinks to their GroupVersionResources. -var SinkPrefixes = map[string]schema.GroupVersionResource{ +// sinkPrefixes maps prefixes used for sinks to their GroupVersionResources. +var sinkMappings = map[string]schema.GroupVersionResource{ "broker": { Resource: "brokers", Group: "eventing.knative.dev", @@ -56,17 +65,6 @@ var SinkPrefixes = map[string]schema.GroupVersionResource{ }, } -func ConfigSinkPrefixes(prefixes []commands.SinkPrefixConfig) { - for _, p := range prefixes { - //user configration might override the default configuration - SinkPrefixes[p.Prefix] = schema.GroupVersionResource{ - Resource: p.Resource, - Group: p.Group, - Version: p.Version, - } - } -} - // ResolveSink returns the Destination referred to by the flags in the acceptor. // It validates that any object the user is referring to exists. func (i *SinkFlags) ResolveSink(knclient clientdynamic.KnDynamicClient, namespace string) (*duckv1.Destination, error) { @@ -84,7 +82,7 @@ func (i *SinkFlags) ResolveSink(knclient clientdynamic.KnDynamicClient, namespac } return &duckv1.Destination{URI: uri}, nil } - typ, ok := SinkPrefixes[prefix] + typ, ok := sinkMappings[prefix] if !ok { return nil, fmt.Errorf("unsupported sink type: %s", i.sink) } diff --git a/pkg/kn/commands/plugin/handler.go b/pkg/kn/commands/plugin/handler.go deleted file mode 100644 index 7945f4bab5..0000000000 --- a/pkg/kn/commands/plugin/handler.go +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright © 2019 The Knative 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 plugin - -import ( - "fmt" - "os" - "os/exec" - "path/filepath" - "runtime" - "strings" - "syscall" - - homedir "github.com/mitchellh/go-homedir" -) - -// PluginHandler is capable of parsing command line arguments -// and performing executable filename lookups to search -// for valid plugin files, and execute found plugins. -type PluginHandler interface { - // exists at the given filename, or a boolean false. - // Lookup will iterate over a list of given prefixes - // in order to recognize valid plugin filenames. - // The first filepath to match a prefix is returned. - Lookup(name string) (string, bool) - // Execute receives an executable's filepath, a slice - // of arguments, and a slice of environment variables - // to relay to the executable. - Execute(executablePath string, cmdArgs, environment []string) error -} - -// DefaultPluginHandler implements PluginHandler -type DefaultPluginHandler struct { - ValidPrefixes []string - PluginsDir string - LookupPluginsInPath bool -} - -// NewDefaultPluginHandler instantiates the DefaultPluginHandler with a list of -// given filename prefixes used to identify valid plugin filenames. -func NewDefaultPluginHandler(validPrefixes []string, pluginsDir string, lookupPluginsInPath bool) *DefaultPluginHandler { - return &DefaultPluginHandler{ - ValidPrefixes: validPrefixes, - PluginsDir: pluginsDir, - LookupPluginsInPath: lookupPluginsInPath, - } -} - -// Lookup implements PluginHandler -// TODO: The current error handling is not optimal, and some errors may be lost. We should refactor the code in the future. -func (h *DefaultPluginHandler) Lookup(name string) (string, bool) { - for _, prefix := range h.ValidPrefixes { - pluginPath := fmt.Sprintf("%s-%s", prefix, name) - - // Try to find plugin in pluginsDir - pluginDir, err := homedir.Expand(h.PluginsDir) - if err != nil { - return "", false - } - - pluginDirPluginPath := filepath.Join(pluginDir, pluginPath) - _, err = os.Stat(pluginDirPluginPath) - if err == nil { - return pluginDirPluginPath, true - } - - // Try to match well-known file extensions on Windows - if runtime.GOOS == "windows" { - for _, ext := range []string{".bat", ".cmd", ".com", ".exe", ".ps1"} { - pathWithExt := pluginDirPluginPath + ext - _, err = os.Stat(pathWithExt) - if err == nil { - return pathWithExt, true - } - } - } - - // No plugins found in pluginsDir, try in PATH of that's an option - if h.LookupPluginsInPath { - pluginPath, err = exec.LookPath(pluginPath) - if err != nil { - continue - } - - if pluginPath != "" { - return pluginPath, true - } - } - } - - return "", false -} - -// Execute implements PluginHandler -func (h *DefaultPluginHandler) Execute(executablePath string, cmdArgs, environment []string) error { - if runtime.GOOS == "windows" { - cmd := exec.Command(executablePath, cmdArgs...) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - cmd.Stdin = os.Stdin - cmd.Env = environment - err := cmd.Run() - if err != nil { - return err - } - return nil - } - return syscall.Exec(executablePath, cmdArgs, environment) -} - -// HandlePluginCommand receives a pluginHandler and command-line arguments and attempts to find -// a plugin executable that satisfies the given arguments. -func HandlePluginCommand(pluginHandler PluginHandler, cmdArgs []string) error { - remainingArgs := []string{} - - for idx := range cmdArgs { - cmdArg := cmdArgs[idx] - if strings.HasPrefix(cmdArg, "-") { - continue - } - remainingArgs = append(remainingArgs, strings.Replace(cmdArg, "-", "_", -1)) - } - - foundBinaryPath := "" - - // attempt to find binary, starting at longest possible name with given cmdArgs - for len(remainingArgs) > 0 { - path, found := pluginHandler.Lookup(strings.Join(remainingArgs, "-")) - if !found { - remainingArgs = remainingArgs[:len(remainingArgs)-1] - continue - } - - foundBinaryPath = path - break - } - - // 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". - if len(foundBinaryPath) != 0 { - err := pluginHandler.Execute(foundBinaryPath, append([]string{foundBinaryPath}, cmdArgs[len(remainingArgs):]...), os.Environ()) - if err != nil { - return err - } - } - - return nil -} diff --git a/pkg/kn/commands/plugin/handler_test.go b/pkg/kn/commands/plugin/handler_test.go deleted file mode 100644 index 27086a36eb..0000000000 --- a/pkg/kn/commands/plugin/handler_test.go +++ /dev/null @@ -1,198 +0,0 @@ -// Copyright © 2018 The Knative 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 plugin - -import ( - "fmt" - "io/ioutil" - "os" - "path/filepath" - "runtime" - "testing" - - "gotest.tools/assert" -) - -func TestPluginHandler(t *testing.T) { - var ( - pluginHandler, tPluginHandler PluginHandler - pluginPath, pluginName, tmpPathDir, pluginsDir string - lookupPluginsInPath bool - err error - ) - - setup := func(t *testing.T) { - tmpPathDir, err = ioutil.TempDir("", "plugin_list") - assert.Assert(t, err == nil) - pluginsDir = tmpPathDir - } - - cleanup := func(t *testing.T) { - err = os.RemoveAll(tmpPathDir) - assert.Assert(t, err == nil) - } - - beforeEach := func(t *testing.T) { - pluginName = "fake" - if runtime.GOOS == "windows" { - pluginName += ".bat" - } - pluginPath = CreateTestPluginInPath(t, "kn-"+pluginName, KnTestPluginScript, FileModeExecutable, tmpPathDir) - assert.Assert(t, pluginPath != "") - - pluginHandler = &DefaultPluginHandler{ - ValidPrefixes: []string{"kn"}, - PluginsDir: pluginsDir, - LookupPluginsInPath: lookupPluginsInPath, - } - assert.Assert(t, pluginHandler != nil) - - tPluginHandler = NewTestPluginHandler(pluginHandler) - assert.Assert(t, tPluginHandler != nil) - } - - t.Run("#NewDefaultPluginHandler", func(t *testing.T) { - setup(t) - defer cleanup(t) - - pHandler := NewDefaultPluginHandler([]string{"kn"}, pluginPath, false) - assert.Assert(t, pHandler != nil) - }) - - t.Run("#Lookup", func(t *testing.T) { - t.Run("when plugin in pluginsDir", func(t *testing.T) { - t.Run("returns the first filepath matching prefix", func(t *testing.T) { - setup(t) - defer cleanup(t) - beforeEach(t) - - path, exists := pluginHandler.Lookup(pluginName) - assert.Assert(t, path != "", fmt.Sprintf("no path when Lookup(%s)", pluginName)) - assert.Assert(t, exists == true, fmt.Sprintf("could not Lookup(%s)", pluginName)) - }) - - t.Run("returns empty filepath when no matching prefix found", func(t *testing.T) { - setup(t) - defer cleanup(t) - - path, exists := pluginHandler.Lookup("bogus-plugin-name") - assert.Assert(t, path == "", fmt.Sprintf("unexpected plugin: kn-bogus-plugin-name")) - assert.Assert(t, exists == false, fmt.Sprintf("unexpected plugin: kn-bogus-plugin-name")) - }) - }) - - t.Run("when plugin is in $PATH", func(t *testing.T) { - t.Run("--lookup-plugins=true", func(t *testing.T) { - setup(t) - defer cleanup(t) - - pluginsDir = filepath.Join(tmpPathDir, "bogus") - err = os.Setenv("PATH", tmpPathDir) - assert.Assert(t, err == nil) - lookupPluginsInPath = true - - beforeEach(t) - - path, exists := pluginHandler.Lookup(pluginName) - assert.Assert(t, path != "", fmt.Sprintf("no path when Lookup(%s)", pluginName)) - assert.Assert(t, exists == true, fmt.Sprintf("could not Lookup(%s)", pluginName)) - }) - - t.Run("--lookup-plugins=false", func(t *testing.T) { - setup(t) - defer cleanup(t) - - pluginsDir = filepath.Join(tmpPathDir, "bogus") - err = os.Setenv("PATH", tmpPathDir) - assert.Assert(t, err == nil) - lookupPluginsInPath = false - - beforeEach(t) - - path, exists := pluginHandler.Lookup(pluginName) - assert.Assert(t, path == "") - assert.Assert(t, exists == false) - }) - }) - }) - - t.Run("#Execute", func(t *testing.T) { - t.Run("fails executing bogus plugin name", func(t *testing.T) { - setup(t) - defer cleanup(t) - beforeEach(t) - - bogusPath := filepath.Join(filepath.Dir(pluginPath), "kn-bogus-plugin-name") - err = pluginHandler.Execute(bogusPath, []string{bogusPath}, os.Environ()) - assert.Assert(t, err != nil, fmt.Sprintf("bogus plugin in path %s unexpectedly executed OK", bogusPath)) - }) - t.Run("executing fake plugin successfully", func(t *testing.T) { - setup(t) - defer cleanup(t) - beforeEach(t) - - err = pluginHandler.Execute(pluginPath, []string{}, os.Environ()) - assert.Assert(t, err == nil, fmt.Sprintf("fail to execute fake plugin in path %s ", pluginPath)) - }) - }) - - t.Run("HandlePluginCommand", func(t *testing.T) { - t.Run("success handling", func(t *testing.T) { - setup(t) - defer cleanup(t) - beforeEach(t) - - err = HandlePluginCommand(tPluginHandler, []string{pluginName}) - assert.Assert(t, err == nil, fmt.Sprintf("test plugin %s failed executing", fmt.Sprintf("kn-%s", pluginName))) - }) - - t.Run("fails handling", func(t *testing.T) { - setup(t) - defer cleanup(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())) - }) - }) -} - -// TestPluginHandler - needed to mock Execute() call - -type testPluginHandler struct { - pluginHandler PluginHandler -} - -func NewTestPluginHandler(pluginHandler PluginHandler) PluginHandler { - return &testPluginHandler{ - pluginHandler: pluginHandler, - } -} - -func (tHandler *testPluginHandler) Lookup(name string) (string, bool) { - return tHandler.pluginHandler.Lookup(name) -} - -func (tHandler *testPluginHandler) Execute(executablePath string, cmdArgs, environment []string) error { - // Always success (avoids doing syscall.Exec which exits tests framework) - return nil -} diff --git a/pkg/kn/commands/plugin/list.go b/pkg/kn/commands/plugin/list.go index a2280e5d0d..df99444ac2 100644 --- a/pkg/kn/commands/plugin/list.go +++ b/pkg/kn/commands/plugin/list.go @@ -16,20 +16,25 @@ package plugin import ( "fmt" - "io/ioutil" "os" - "path/filepath" "strings" - homedir "github.com/mitchellh/go-homedir" + "github.com/pkg/errors" "github.com/spf13/cobra" "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/config" + "knative.dev/client/pkg/kn/plugin" ) // ValidPluginFilenamePrefixes controls the prefix for all kn plugins var ValidPluginFilenamePrefixes = []string{"kn"} +// pluginListFlags contains all plugin commands flags +type pluginListFlags struct { + verbose bool +} + // NewPluginListCommand creates a new `kn plugin list` command func NewPluginListCommand(p *commands.KnParams) *cobra.Command { @@ -42,117 +47,68 @@ func NewPluginListCommand(p *commands.KnParams) *cobra.Command { Available plugins are those that are: - executable - begin with "kn-" -- Kn's plugin directory ` + commands.Cfg.DefaultPluginDir + ` -- Anywhere in the execution $PATH (if lookupInPath config variable is enabled)`, +- Kn's plugin directory +- Anywhere in the execution $PATH (if plugins.path-lookup config variable is enabled)`, RunE: func(cmd *cobra.Command, args []string) error { return listPlugins(cmd, plFlags) }, } - AddPluginFlags(pluginListCommand) - BindPluginsFlagToViper(pluginListCommand) - plFlags.AddPluginListFlags(pluginListCommand) + // Plugin flags + pluginListCommand.Flags().BoolVar(&plFlags.verbose, "verbose", false, "verbose output") return pluginListCommand } // List plugins by looking up in plugin directory and path func listPlugins(cmd *cobra.Command, flags pluginListFlags) error { - pluginPath, err := homedir.Expand(commands.Cfg.PluginsDir) + factory := plugin.NewManager(config.GlobalConfig.PluginsDir(), config.GlobalConfig.LookupPluginsInPath()) + + pluginsFound, err := factory.ListPlugins() if err != nil { - return err - } - if *commands.Cfg.LookupPlugins { - pluginPath = pluginPath + string(os.PathListSeparator) + os.Getenv("PATH") + return errors.Wrap(err, fmt.Sprintf("cannot list plugins in %s (lookup plugins in $PATH: %t)", factory.PluginsDir(), factory.LookupInPath())) } - pluginsFound, eaw := lookupPlugins(pluginPath) - out := cmd.OutOrStdout() - if flags.verbose { fmt.Fprintf(out, "The following plugins are available, using options:\n") - fmt.Fprintf(out, " - plugins dir: '%s'%s\n", commands.Cfg.PluginsDir, extraLabelIfPathNotExists(pluginPath)) - fmt.Fprintf(out, " - lookup plugins in $PATH: '%t'\n", *commands.Cfg.LookupPlugins) + fmt.Fprintf(out, " plugins dir: '%s'%s\n", factory.PluginsDir(), extraLabelIfPathNotExists(factory.PluginsDir())) + fmt.Fprintf(out, " lookup plugins in $PATH: %t\n\n", factory.LookupInPath()) } if len(pluginsFound) == 0 { if flags.verbose { - fmt.Fprintf(out, "No plugins found in path '%s'.\n", pluginPath) + fmt.Fprintf(out, "No plugins found in path '%s'.\n", factory.PluginsDir()) } else { fmt.Fprintln(out, "No plugins found.") } return nil } - verifier := newPluginVerifier(cmd.Root()) - for _, plugin := range pluginsFound { - name := plugin - if flags.nameOnly { - name = filepath.Base(plugin) - } - fmt.Fprintf(out, "%s\n", name) - eaw = verifier.verify(eaw, plugin) - } - eaw.printWarningsAndErrors(out) - return eaw.combinedError() -} - -func lookupPlugins(pluginPath string) ([]string, errorsAndWarnings) { - pluginsFound := make([]string, 0) - eaw := errorsAndWarnings{} - - for _, dir := range uniquePathsList(filepath.SplitList(pluginPath)) { - - files, err := ioutil.ReadDir(dir) + eaw := factory.Verify() + eaw = addErrorIfOverwritingExistingCommand(eaw, cmd.Root(), pluginsFound) - // Ignore non-existing directories - if os.IsNotExist(err) { - continue - } - - if err != nil { - eaw.addError("unable to read directory '%s' from your plugin path: %v", dir, err) - continue + for _, pl := range pluginsFound { + desc, _ := pl.Description() + if desc != "" { + fmt.Fprintf(out, "- %s : %s", pl.Name(), desc) + } else { + fmt.Fprintf(out, "- %s", pl.Name()) } - - // Check for plugins within given directory - for _, f := range files { - if f.IsDir() { - continue - } - if !hasValidPrefix(f.Name(), ValidPluginFilenamePrefixes) { - continue - } - pluginsFound = append(pluginsFound, filepath.Join(dir, f.Name())) + if flags.verbose { + fmt.Fprintf(out, " (%s)\n", pl.Path()) + } else { + fmt.Fprintln(out, "") } } - return pluginsFound, eaw -} - -func hasValidPrefix(filepath string, validPrefixes []string) bool { - for _, prefix := range validPrefixes { - if !strings.HasPrefix(filepath, prefix+"-") { - continue - } - return true + if !eaw.IsEmpty() { + fmt.Fprintln(out, "") + eaw.PrintWarningsAndErrors(out) } - return false -} - -// uniquePathsList deduplicates a given slice of strings without -// sorting or otherwise altering its order in any way. -func uniquePathsList(paths []string) []string { - seen := map[string]bool{} - var newPaths []string - for _, p := range paths { - if seen[p] { - continue - } - seen[p] = true - newPaths = append(newPaths, p) + if eaw.HasErrors() { + return errors.Errorf("plugin validation errors") } - return newPaths + return nil } // create an info label which can be appended to an verbose output @@ -166,3 +122,17 @@ func extraLabelIfPathNotExists(path string) string { } return "" } + +func addErrorIfOverwritingExistingCommand(eaw plugin.VerificationErrorsAndWarnings, rootCmd *cobra.Command, plugins []plugin.Plugin) plugin.VerificationErrorsAndWarnings { + + for _, plugin := range plugins { + cmd, args, err := rootCmd.Find(plugin.CommandParts()) + if err == nil { + if !cmd.HasSubCommands() || // a leaf command can't be overridden + cmd.HasSubCommands() && len(args) == 0 { // a group can't be overridden either + eaw.AddError("%s overwrites existing built-in command: '%s'", plugin.Path(), strings.Join(plugin.CommandParts(), " ")) + } + } + } + return eaw +} diff --git a/pkg/kn/commands/plugin/list_flags_test.go b/pkg/kn/commands/plugin/list_flags_test.go deleted file mode 100644 index 9684b4b000..0000000000 --- a/pkg/kn/commands/plugin/list_flags_test.go +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright © 2018 The Knative 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 plugin - -import ( - "testing" - - "github.com/spf13/cobra" - "gotest.tools/assert" -) - -func TestAddPluginListFlags(t *testing.T) { - var ( - pFlags *pluginListFlags - cmd *cobra.Command - ) - - t.Run("adds plugin flag", func(t *testing.T) { - pFlags = &pluginListFlags{} - - cmd = &cobra.Command{} - pFlags.AddPluginListFlags(cmd) - - assert.Assert(t, pFlags != nil) - assert.Assert(t, cmd.Flags() != nil) - - nameOnly, err := cmd.Flags().GetBool("name-only") - assert.NilError(t, err) - assert.Assert(t, nameOnly == false) - - verbose, err := cmd.Flags().GetBool("verbose") - assert.NilError(t, err) - assert.Assert(t, verbose == false) - }) -} diff --git a/pkg/kn/commands/plugin/list_test.go b/pkg/kn/commands/plugin/list_test.go index 230cd0a159..cc7eb7c27e 100644 --- a/pkg/kn/commands/plugin/list_test.go +++ b/pkg/kn/commands/plugin/list_test.go @@ -16,7 +16,6 @@ package plugin import ( "bytes" - "fmt" "io/ioutil" "os" "path/filepath" @@ -25,223 +24,146 @@ import ( "testing" "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/config" "knative.dev/client/pkg/util" "github.com/spf13/cobra" "gotest.tools/assert" ) -type testContext struct { - pluginsDir string - pathDir string - rootCmd *cobra.Command - out *bytes.Buffer - origPath string -} +func TestPluginListBasic(t *testing.T) { + + pluginListCmd := NewPluginListCommand(&commands.KnParams{}) + assert.Assert(t, pluginListCmd != nil) -func (ctx *testContext) execute(args ...string) error { - ctx.rootCmd.SetArgs(append(args, fmt.Sprintf("--plugins-dir=%s", ctx.pluginsDir))) - return ctx.rootCmd.Execute() + assert.Assert(t, pluginListCmd != nil) + assert.Assert(t, pluginListCmd.Use == "list") + assert.Assert(t, pluginListCmd.Short == "List plugins") + assert.Assert(t, strings.Contains(pluginListCmd.Long, "List all installed plugins")) + assert.Assert(t, pluginListCmd.RunE != nil) } -func (ctx *testContext) output() string { - return ctx.out.String() +func TestPluginListOutput(t *testing.T) { + pluginDir, cleanupFunc := prepareTestSetup(t, "kn-test1", 0777, "kn-test2", 0644) + defer cleanupFunc() + + for _, verbose := range []bool{false, true} { + outBuf := bytes.Buffer{} + testCmd := cobra.Command{ + Use: "kn", + } + testCmd.SetOut(&outBuf) + testCmd.AddCommand(&cobra.Command{Use: "children"}) + + err := listPlugins(&testCmd, pluginListFlags{verbose: verbose}) + assert.NilError(t, err) + + out := outBuf.String() + + assert.Assert(t, util.ContainsAll(out, "kn-test1", "kn-test2")) + + if verbose { + assert.Assert(t, util.ContainsAll(out, pluginDir)) + } + + if runtime.GOOS != "windows" { + assert.Assert(t, util.ContainsAll(out, "WARNING", "not executable")) + } + } } -func (ctx *testContext) cleanup() { - os.RemoveAll(ctx.pluginsDir) - os.RemoveAll(ctx.pathDir) - os.Setenv("PATH", ctx.origPath) +func TestPluginListNoPlugins(t *testing.T) { + pluginDir, cleanupFunc := prepareTestSetup(t) + defer cleanupFunc() + + for _, verbose := range []bool{false, true} { + outBuf := bytes.Buffer{} + testCmd := cobra.Command{} + testCmd.SetOut(&outBuf) + + err := listPlugins(&testCmd, pluginListFlags{verbose: verbose}) + assert.NilError(t, err) + + out := outBuf.String() + assert.Assert(t, util.ContainsAll(out, "No", "found")) + if verbose { + assert.Assert(t, util.ContainsAll(out, pluginDir)) + } + } } -func (ctx *testContext) createTestPlugin(pluginName string, fileMode os.FileMode, inPath bool) error { - path := ctx.pluginsDir - if inPath { - path = ctx.pathDir +func TestPluginListOverridingBuiltinCommand(t *testing.T) { + pluginDir, cleanupFunc := prepareTestSetup(t, "kn-existing", 0777) + defer cleanupFunc() + + outBuf := bytes.Buffer{} + testCmd := cobra.Command{ + Use: "kn", } - return ctx.createTestPluginWithPath(pluginName, fileMode, path) + testCmd.AddCommand(&cobra.Command{Use: "existing"}) + testCmd.SetOut(&outBuf) + err := listPlugins(&testCmd, pluginListFlags{verbose: false}) + assert.Assert(t, err != nil) + + out := outBuf.String() + assert.Assert(t, util.ContainsAll(out, "ERROR", "'existing'", "overwrites", pluginDir, "kn-existing")) } -func (ctx *testContext) createTestPluginWithPath(pluginName string, fileMode os.FileMode, path string) error { - if runtime.GOOS == "windows" { - pluginName += ".bat" +func TestPluginListExtendingBuiltinCommandGroup(t *testing.T) { + _, cleanupFunc := prepareTestSetup(t, "kn-existing-addon", 0777) + defer cleanupFunc() + + outBuf := bytes.Buffer{} + testCmd := cobra.Command{ + Use: "kn", } - fullPath := filepath.Join(path, pluginName) - return ioutil.WriteFile(fullPath, []byte(KnTestPluginScript), fileMode) + testGroup := &cobra.Command{Use: "existing"} + testGroup.AddCommand(&cobra.Command{Use: "builtin"}) + testCmd.AddCommand(testGroup) + testCmd.SetOut(&outBuf) + err := listPlugins(&testCmd, pluginListFlags{verbose: false}) + assert.NilError(t, err) + + out := outBuf.String() + assert.Assert(t, util.ContainsAll(out, "kn-existing-addon")) + assert.Assert(t, !strings.Contains(out, "ERROR")) } -func TestPluginList(t *testing.T) { - setup := func(t *testing.T) *testContext { - knParams := &commands.KnParams{} - pluginCmd := NewPluginCommand(knParams) +// Private - rootCmd, _, out := commands.CreateTestKnCommand(pluginCmd, knParams) - pluginsDir, err := ioutil.TempDir("", "plugin-list-plugindir") - assert.NilError(t, err) - pathDir, err := ioutil.TempDir("", "plugin-list-pathdir") - assert.NilError(t, err) +func prepareTestSetup(t *testing.T, args ...interface{}) (string, func()) { + tmpPathDir, err := ioutil.TempDir("", "plugin_list") + assert.NilError(t, err) - origPath := os.Getenv("PATH") - assert.NilError(t, os.Setenv("PATH", pathDir)) + // Prepare configuration to for our test + oldConfig := config.GlobalConfig + config.GlobalConfig = &config.TestConfig{ + TestPluginsDir: tmpPathDir, + TestLookupPluginsInPath: false, + } - return &testContext{ - rootCmd: rootCmd, - out: out, - pluginsDir: pluginsDir, - pathDir: pathDir, - origPath: origPath, - } + for i := 0; i < len(args); i += 2 { + name := args[i].(string) + perm := args[i+1].(int) + createTestPlugin(t, name, tmpPathDir, os.FileMode(perm)) + } + + return tmpPathDir, func() { + config.GlobalConfig = oldConfig + os.RemoveAll(tmpPathDir) } +} - t.Run("creates a new cobra.Command", func(t *testing.T) { - pluginCmd := NewPluginCommand(&commands.KnParams{}) - pluginListCmd := FindSubCommand(t, pluginCmd, "list") - assert.Assert(t, pluginListCmd != nil) - - assert.Assert(t, pluginListCmd != nil) - assert.Assert(t, pluginListCmd.Use == "list") - assert.Assert(t, pluginListCmd.Short == "List plugins") - assert.Assert(t, strings.Contains(pluginListCmd.Long, "List all installed plugins")) - assert.Assert(t, pluginListCmd.Flags().Lookup("plugins-dir") != nil) - assert.Assert(t, pluginListCmd.RunE != nil) - }) - - t.Run("when plugins-dir does not include any plugins", func(t *testing.T) { - t.Run("when --lookup-plugins is true", func(t *testing.T) { - t.Run("no plugins installed", func(t *testing.T) { - - t.Run("warns user that no plugins found in verbose mode", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - err := ctx.execute("plugin", "list", "--lookup-plugins=true", "--verbose") - assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(ctx.output(), "No plugins found")) - }) - - t.Run("no output when no plugins found", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - err := ctx.execute("plugin", "list", "--lookup-plugins=true") - assert.NilError(t, err) - assert.Equal(t, ctx.output(), "No plugins found.\n") - }) - }) - - t.Run("plugins installed", func(t *testing.T) { - t.Run("with valid plugin in $PATH", func(t *testing.T) { - - t.Run("list plugins in $PATH", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - - err := ctx.createTestPlugin(KnTestPluginName, FileModeExecutable, true) - assert.NilError(t, err) - - err = ctx.execute("plugin", "list", "--lookup-plugins=true") - assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(ctx.output(), KnTestPluginName)) - }) - }) - - t.Run("with non-executable plugin", func(t *testing.T) { - t.Run("warns user plugin invalid", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - - err := ctx.createTestPlugin(KnTestPluginName, FileModeReadable, false) - assert.NilError(t, err) - - err = ctx.execute("plugin", "list", "--lookup-plugins=false") - assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(ctx.output(), "WARNING", "not executable", "current user")) - }) - }) - - t.Run("with plugins with same name", func(t *testing.T) { - t.Run("warns user about second (in $PATH) plugin shadowing first", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - - err := ctx.createTestPlugin(KnTestPluginName, FileModeExecutable, true) - assert.NilError(t, err) - - tmpPathDir2, err := ioutil.TempDir("", "plugins_list") - assert.NilError(t, err) - defer os.RemoveAll(tmpPathDir2) - - err = os.Setenv("PATH", ctx.pathDir+string(os.PathListSeparator)+tmpPathDir2) - assert.NilError(t, err) - - err = ctx.createTestPluginWithPath(KnTestPluginName, FileModeExecutable, tmpPathDir2) - assert.NilError(t, err) - - err = ctx.execute("plugin", "list", "--lookup-plugins=true") - assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(ctx.output(), "WARNING", "shadowed", "ignored")) - }) - }) - - t.Run("with plugins with name of existing command", func(t *testing.T) { - t.Run("warns user about overwriting existing command", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - - fakeCmd := &cobra.Command{ - Use: "fake", - } - ctx.rootCmd.AddCommand(fakeCmd) - defer ctx.rootCmd.RemoveCommand(fakeCmd) - - err := ctx.createTestPlugin("kn-fake", FileModeExecutable, true) - assert.NilError(t, err) - - err = ctx.execute("plugin", "list", "--lookup-plugins=true") - assert.ErrorContains(t, err, "overwrite", "built-in") - assert.Assert(t, util.ContainsAll(ctx.output(), "ERROR", "overwrite", "built-in")) - }) - - t.Run("allows plugin under the `source` command group", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - - sourceCmd := &cobra.Command{ - Use: "source", - } - ctx.rootCmd.AddCommand(sourceCmd) - defer ctx.rootCmd.RemoveCommand(sourceCmd) - - err := ctx.createTestPlugin("kn-source-fake", FileModeExecutable, true) - assert.NilError(t, err) - - err = ctx.execute("plugin", "list", "--lookup-plugins=true") - assert.NilError(t, err) - }) - - }) - }) - }) - }) - - t.Run("when plugins-dir has plugins", func(t *testing.T) { - t.Run("list plugins in --plugins-dir", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - - err := ctx.createTestPlugin(KnTestPluginName, FileModeExecutable, false) - assert.NilError(t, err) - - err = ctx.execute("plugin", "list") - assert.NilError(t, err) - assert.Assert(t, util.ContainsAll(ctx.output(), KnTestPluginName)) - }) - - t.Run("no plugins installed", func(t *testing.T) { - ctx := setup(t) - defer ctx.cleanup() - - err := ctx.execute("plugin", "list") - assert.NilError(t, err) - assert.Equal(t, ctx.output(), "No plugins found.\n") - }) - }) +// CreateTestPluginInPath with name, path, script, and fileMode and return the tmp random path +func createTestPlugin(t *testing.T, name string, dir string, perm os.FileMode) string { + var nameExt string + if runtime.GOOS == "windows" { + nameExt = name + ".bat" + } else { + nameExt = name + } + fullPath := filepath.Join(dir, nameExt) + err := ioutil.WriteFile(fullPath, []byte{}, perm) + assert.NilError(t, err) + return fullPath } diff --git a/pkg/kn/commands/plugin/plugin.go b/pkg/kn/commands/plugin/plugin.go index 03b252c53d..b608484f73 100644 --- a/pkg/kn/commands/plugin/plugin.go +++ b/pkg/kn/commands/plugin/plugin.go @@ -16,7 +16,7 @@ package plugin import ( "github.com/spf13/cobra" - "github.com/spf13/viper" + "knative.dev/client/pkg/kn/commands" ) @@ -30,28 +30,7 @@ Plugins provide extended functionality that is not part of the core kn command-l Please refer to the documentation and examples for more information about how write your own plugins.`, } - AddPluginFlags(pluginCmd) - BindPluginsFlagToViper(pluginCmd) - pluginCmd.AddCommand(NewPluginListCommand(p)) return pluginCmd } - -// AddPluginFlags plugins-dir and lookup-plugins to cmd -func AddPluginFlags(cmd *cobra.Command) { - cmd.Flags().StringVar(&commands.Cfg.PluginsDir, "plugins-dir", commands.Cfg.DefaultPluginDir, "kn plugins directory") - cmd.Flags().BoolVar(commands.Cfg.LookupPlugins, "lookup-plugins", false, "look for kn plugins in $PATH") -} - -// BindPluginsFlagToViper bind and set default with viper for plugins flags -func BindPluginsFlagToViper(cmd *cobra.Command) { - viper.BindPFlag("plugins-dir", cmd.Flags().Lookup("plugins-dir")) - viper.BindPFlag("lookup-plugins", cmd.Flags().Lookup("lookup-plugins")) - - viper.SetDefault("plugins-dir", commands.Cfg.DefaultPluginDir) - viper.SetDefault("lookup-plugins", false) -} - -// CoreCommandNames names of all core `kn` commands -var CoreCommandNames = []string{} diff --git a/pkg/kn/commands/plugin/plugin_test.go b/pkg/kn/commands/plugin/plugin_test.go index fb34cfb59b..332c24d350 100644 --- a/pkg/kn/commands/plugin/plugin_test.go +++ b/pkg/kn/commands/plugin/plugin_test.go @@ -15,60 +15,20 @@ package plugin import ( - "strings" "testing" - "github.com/spf13/cobra" "gotest.tools/assert" + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/util" ) -const PluginCommandUsage = `Provides utilities for interacting and managing with kn plugins. - -Plugins provide extended functionality that is not part of the core kn command-line distribution. -Please refer to the documentation and examples for more information about how write your own plugins. - -Usage: - kn plugin [flags] - kn plugin [command] - -Available Commands: - list List all visible plugin executables - -Flags: - -h, --help help for plugin - --lookup-plugins look for kn plugins in $PATH - --plugins-dir string kn plugins directory (default "~/.config/kn/plugins") - -Global Flags: - --config string kn config file (default is $HOME/.config/kn/config.yaml) - --kubeconfig string kubectl config file (default is $HOME/.kube/config) - -Use "kn plugin [command] --help" for more information about a command.` - func TestNewPluginCommand(t *testing.T) { - var ( - rootCmd, pluginCmd *cobra.Command - ) - - setup := func(t *testing.T) { - knParams := &commands.KnParams{} - pluginCmd = NewPluginCommand(knParams) - assert.Assert(t, pluginCmd != nil) - - rootCmd, _, _ = commands.CreateTestKnCommand(pluginCmd, knParams) - assert.Assert(t, rootCmd != nil) - } - - t.Run("creates a new cobra.Command", func(t *testing.T) { - setup(t) + pluginCmd := NewPluginCommand(&commands.KnParams{}) + assert.Assert(t, pluginCmd != nil) - assert.Assert(t, pluginCmd != nil) - assert.Assert(t, pluginCmd.Use == "plugin") - assert.Assert(t, pluginCmd.Short == "Plugin command group") - assert.Assert(t, strings.Contains(pluginCmd.Long, "Provides utilities for interacting and managing with kn plugins.")) - assert.Assert(t, pluginCmd.Flags().Lookup("plugins-dir") != nil) - assert.Assert(t, pluginCmd.Flags().Lookup("lookup-plugins") != nil) - assert.Assert(t, pluginCmd.Args == nil) - }) + assert.Assert(t, pluginCmd != nil) + assert.Equal(t, pluginCmd.Use, "plugin") + assert.Assert(t, util.ContainsAllIgnoreCase(pluginCmd.Short, "plugin")) + assert.Assert(t, util.ContainsAllIgnoreCase(pluginCmd.Long, "plugins")) } diff --git a/pkg/kn/commands/plugin/plugin_test_helper.go b/pkg/kn/commands/plugin/plugin_test_helper.go deleted file mode 100644 index c080d4602a..0000000000 --- a/pkg/kn/commands/plugin/plugin_test_helper.go +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright © 2018 The Knative 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 plugin - -import ( - "io/ioutil" - "os" - "path/filepath" - "testing" - - "github.com/spf13/cobra" - "gotest.tools/assert" -) - -const ( - KnTestPluginName = "kn-test" - KnTestPluginScript = `#!/bin/bash - -echo "I am a test Kn plugin" -exit 0 -` - FileModeReadable = 0644 - FileModeExecutable = 0777 -) - -// FindSubCommand return the sub-command by name -func FindSubCommand(t *testing.T, rootCmd *cobra.Command, name string) *cobra.Command { - for _, subCmd := range rootCmd.Commands() { - if subCmd.Name() == name { - return subCmd - } - } - - return nil -} - -// CreateTestPlugin with name, script, and fileMode and return the tmp random path -func CreateTestPlugin(t *testing.T, name, script string, fileMode os.FileMode) string { - path, err := ioutil.TempDir("", "plugin") - assert.Assert(t, err == nil) - - return CreateTestPluginInPath(t, name, script, fileMode, path) -} - -// CreateTestPluginInPath with name, path, script, and fileMode and return the tmp random path -func CreateTestPluginInPath(t *testing.T, name, script string, fileMode os.FileMode, path string) string { - fullPath := filepath.Join(path, name) - err := ioutil.WriteFile(fullPath, []byte(script), fileMode) - assert.NilError(t, err) - return fullPath -} - -// DeleteTestPlugin with path -func DeleteTestPlugin(t *testing.T, path string) { - err := os.RemoveAll(filepath.Dir(path)) - assert.Assert(t, err == nil) -} diff --git a/pkg/kn/commands/plugin/verifier.go b/pkg/kn/commands/plugin/verifier.go deleted file mode 100644 index a541851899..0000000000 --- a/pkg/kn/commands/plugin/verifier.go +++ /dev/null @@ -1,304 +0,0 @@ -// Copyright © 2019 The Knative 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. - -// +build !windows - -// This file doesn't compile for Windows platform, it defines respective build tag (build tag), -// the respective functionality is present in plugin_verifier_windows.go file in same dir, -// which only compiles for Windows platform. - -package plugin - -import ( - "errors" - "fmt" - "io" - "os" - "path/filepath" - "runtime" - "strings" - "syscall" - - "github.com/spf13/cobra" -) - -// pluginVerifier verifies that existing kn commands are not overridden -type pluginVerifier struct { - root *cobra.Command - seenPlugins map[string]string -} - -// collect errors and warnings on the way -type errorsAndWarnings struct { - errors []string - warnings []string -} - -// Create new verifier -func newPluginVerifier(root *cobra.Command) *pluginVerifier { - return &pluginVerifier{ - root: root, - seenPlugins: make(map[string]string), - } -} - -// permission bits for execute -const ( - UserExecute = 1 << 6 - GroupExecute = 1 << 3 - OtherExecute = 1 << 0 -) - -// Verify implements pathVerifier and determines if a given path -// is valid depending on whether or not it overwrites an existing -// kn command path, or a previously seen plugin. -// This method is not idempotent and must be called for each path only once. -func (v *pluginVerifier) verify(eaw errorsAndWarnings, path string) errorsAndWarnings { - if v.root == nil { - return eaw.addError("unable to verify path with nil root") - } - - // Verify that plugin actually exists - fileInfo, err := os.Stat(path) - if err != nil { - if err == os.ErrNotExist { - return eaw.addError("cannot find plugin in %s", path) - } - return eaw.addError("cannot stat %s: %v", path, err) - } - - eaw = v.addErrorIfWrongPrefix(eaw, path) - eaw = v.addWarningIfNotExecutable(eaw, path, fileInfo) - eaw = v.addWarningIfAlreadySeen(eaw, path) - eaw = v.addErrorIfOverwritingExistingCommand(eaw, path) - - // Remember each verified plugin for duplicate check - v.seenPlugins[filepath.Base(path)] = path - - return eaw -} - -func (v *pluginVerifier) addWarningIfAlreadySeen(eaw errorsAndWarnings, path string) errorsAndWarnings { - fileName := filepath.Base(path) - if existingPath, ok := v.seenPlugins[fileName]; ok { - return eaw.addWarning("%s is ignored because it is shadowed by an equally named plugin: %s.", path, existingPath) - } - return eaw -} - -func (v *pluginVerifier) addErrorIfOverwritingExistingCommand(eaw errorsAndWarnings, path string) errorsAndWarnings { - fileName := filepath.Base(path) - cmds := strings.Split(fileName, "-") - if len(cmds) < 2 { - return eaw.addError("%s is not a valid plugin filename as its missing a prefix", fileName) - } - cmds = cmds[1:] - - // Check both, commands with underscore and with dash because plugins can be called with both - overwrittenCommands := make(map[string]bool) - for _, c := range [][]string{cmds, convertUnderscoresToDashes(cmds)} { - cmd, _, err := v.root.Find(c) - if err == nil { - if !inAllowedExtensibleCommandGroups(cmd.Name()) { - overwrittenCommands[cmd.CommandPath()] = true - } - } - } - for command := range overwrittenCommands { - eaw.addError("%s overwrites existing built-in command: %s", fileName, command) - } - return eaw -} - -func (v *pluginVerifier) addErrorIfWrongPrefix(eaw errorsAndWarnings, path string) errorsAndWarnings { - fileName := filepath.Base(path) - // Only pick the first prefix as it is very like that it will be reduced to - // a single prefix anyway (PR pending) - prefix := ValidPluginFilenamePrefixes[0] - if !strings.HasPrefix(fileName, prefix+"-") { - eaw.addWarning("%s plugin doesn't start with plugin prefix %s", fileName, prefix) - } - return eaw -} - -func (v *pluginVerifier) addWarningIfNotExecutable(eaw errorsAndWarnings, path string, fileInfo os.FileInfo) errorsAndWarnings { - if runtime.GOOS == "windows" { - return checkForWindowsExecutable(eaw, fileInfo, path) - } - - mode := fileInfo.Mode() - if !mode.IsRegular() && !isSymlink(mode) { - return eaw.addWarning("%s is not a file", path) - } - perms := uint32(mode.Perm()) - - var sys *syscall.Stat_t - var ok bool - if sys, ok = fileInfo.Sys().(*syscall.Stat_t); !ok { - // We can check the files' owner/group - return eaw.addWarning("cannot check owner/group of file %s", path) - } - - isOwner := checkIfUserIsFileOwner(sys.Uid) - isInGroup, err := checkIfUserInGroup(sys.Gid) - if err != nil { - return eaw.addError("cannot get group ids for checking executable status of file %s", path) - } - - // User is owner and owner can execute - if canOwnerExecute(perms, isOwner) { - return eaw - } - - // User is in group which can execute, but user is not file owner - if canGroupExecute(perms, isOwner, isInGroup) { - return eaw - } - - // All can execute, and the user is not file owner and not in the file's perm group - if canOtherExecute(perms, isOwner, isInGroup) { - return eaw - } - - return eaw.addWarning("%s is not executable by current user", path) -} - -func checkForWindowsExecutable(eaw errorsAndWarnings, fileInfo os.FileInfo, path string) errorsAndWarnings { - fileExt := strings.ToLower(filepath.Ext(fileInfo.Name())) - - switch fileExt { - case ".bat", ".cmd", ".com", ".exe", ".ps1": - return eaw - } - return eaw.addWarning("%s is not executable as it does not have the proper extension", path) -} - -func checkIfUserInGroup(gid uint32) (bool, error) { - groups, err := os.Getgroups() - if err != nil { - return false, err - } - for _, g := range groups { - if int(gid) == g { - return true, nil - } - } - return false, nil -} - -func checkIfUserIsFileOwner(uid uint32) bool { - if int(uid) == os.Getuid() { - return true - } - return false -} - -// Check if all can execute, and the user is not file owner and not in the file's perm group -func canOtherExecute(perms uint32, isOwner bool, isInGroup bool) bool { - if perms&OtherExecute != 0 { - if os.Getuid() == 0 { - return true - } - if !isOwner && !isInGroup { - return true - } - } - return false -} - -// Check if user is owner and owner can execute -func canOwnerExecute(perms uint32, isOwner bool) bool { - if perms&UserExecute != 0 { - if os.Getuid() == 0 { - return true - } - if isOwner { - return true - } - } - return false -} - -// Check if user is in group which can execute, but user is not file owner -func canGroupExecute(perms uint32, isOwner bool, isInGroup bool) bool { - if perms&GroupExecute != 0 { - if os.Getuid() == 0 { - return true - } - if !isOwner && isInGroup { - return true - } - } - return false -} - -func (eaw *errorsAndWarnings) addError(format string, args ...interface{}) errorsAndWarnings { - eaw.errors = append(eaw.errors, fmt.Sprintf(format, args...)) - return *eaw -} - -func (eaw *errorsAndWarnings) addWarning(format string, args ...interface{}) errorsAndWarnings { - eaw.warnings = append(eaw.warnings, fmt.Sprintf(format, args...)) - return *eaw -} - -func (eaw *errorsAndWarnings) printWarningsAndErrors(out io.Writer) { - printSection(out, "ERROR", eaw.errors) - printSection(out, "WARNING", eaw.warnings) -} - -func (eaw *errorsAndWarnings) combinedError() error { - if len(eaw.errors) == 0 { - return nil - } - return errors.New(strings.Join(eaw.errors, ",")) -} - -func printSection(out io.Writer, label string, values []string) { - if len(values) > 0 { - printLabelWithConditionalPluralS(out, label, len(values)) - for _, value := range values { - fmt.Fprintf(out, " - %s\n", value) - } - } -} - -func printLabelWithConditionalPluralS(out io.Writer, label string, nr int) { - if nr == 1 { - fmt.Fprintf(out, "%s:\n", label) - } else { - fmt.Fprintf(out, "%ss:\n", label) - } -} - -func convertUnderscoresToDashes(cmds []string) []string { - ret := make([]string, len(cmds)) - for i := range cmds { - ret[i] = strings.ReplaceAll(cmds[i], "_", "-") - } - return ret -} - -func isSymlink(mode os.FileMode) bool { - return mode&os.ModeSymlink != 0 -} - -func inAllowedExtensibleCommandGroups(name string) bool { - for _, groupName := range CoreCommandNames { - if name == groupName { - return true - } - } - return false -} diff --git a/pkg/kn/commands/plugin/verifier_test.go b/pkg/kn/commands/plugin/verifier_test.go deleted file mode 100644 index d401169c9d..0000000000 --- a/pkg/kn/commands/plugin/verifier_test.go +++ /dev/null @@ -1,241 +0,0 @@ -// Copyright © 2018 The Knative 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 plugin - -import ( - "errors" - "io/ioutil" - "os" - "os/exec" - "os/user" - "path/filepath" - "runtime" - "strconv" - "testing" - - "knative.dev/client/pkg/kn/commands" - "knative.dev/client/pkg/util" - - "github.com/spf13/cobra" - "gotest.tools/assert" -) - -func TestPluginVerifier(t *testing.T) { - var ( - pluginPath string - rootCmd *cobra.Command - verifier *pluginVerifier - ) - - setup := func(t *testing.T) { - knParams := &commands.KnParams{} - rootCmd, _, _ = commands.CreateTestKnCommand(NewPluginCommand(knParams), knParams) - verifier = newPluginVerifier(rootCmd) - } - - cleanup := func(t *testing.T) { - if pluginPath != "" { - DeleteTestPlugin(t, pluginPath) - } - } - - t.Run("with nil root command", func(t *testing.T) { - t.Run("returns error verifying path", func(t *testing.T) { - setup(t) - defer cleanup(t) - verifier.root = nil - eaw := errorsAndWarnings{} - eaw = verifier.verify(eaw, pluginPath) - assert.Assert(t, len(eaw.errors) == 1) - assert.Assert(t, len(eaw.warnings) == 0) - assert.Assert(t, util.ContainsAll(eaw.errors[0], "nil root")) - }) - }) - - t.Run("with root command", func(t *testing.T) { - t.Run("whether plugin in path is executable (unix only)", func(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Skip test for windows as the permission check are for Unix only") - return - } - - setup(t) - defer cleanup(t) - - pluginDir, err := ioutil.TempDir("", "plugin") - assert.NilError(t, err) - defer os.RemoveAll(pluginDir) - pluginPath := filepath.Join(pluginDir, "kn-execution-test") - err = ioutil.WriteFile(pluginPath, []byte("#!/bin/sh\ntrue"), 0644) - assert.NilError(t, err, "can't create test plugin") - - for _, uid := range getExecTestUids() { - for _, gid := range getExecTestGids() { - for _, userPerm := range []int{0, UserExecute} { - for _, groupPerm := range []int{0, GroupExecute} { - for _, otherPerm := range []int{0, OtherExecute} { - perm := os.FileMode(userPerm | groupPerm | otherPerm + 0444) - assert.NilError(t, prepareFile(pluginPath, uid, gid, perm), "prepare plugin file, uid: %d, gid: %d, perm: %03o", uid, gid, perm) - - eaw := errorsAndWarnings{} - eaw = newPluginVerifier(rootCmd).verify(eaw, pluginPath) - - if isExecutable(pluginPath) { - assert.Assert(t, len(eaw.warnings) == 0, "executable: perm %03o | uid %d | gid %d | %v", perm, uid, gid, eaw.warnings) - assert.Assert(t, len(eaw.errors) == 0) - } else { - assert.Assert(t, len(eaw.warnings) == 1, "not executable: perm %03o | uid %d | gid %d | %v", perm, uid, gid, eaw.warnings) - assert.Assert(t, len(eaw.errors) == 0) - assert.Assert(t, util.ContainsAll(eaw.warnings[0], pluginPath)) - } - - } - } - } - } - } - }) - - t.Run("when kn plugin in path is executable", func(t *testing.T) { - setup(t) - defer cleanup(t) - pluginPath = CreateTestPlugin(t, KnTestPluginName, KnTestPluginScript, FileModeExecutable) - - t.Run("when kn plugin in path shadows another", func(t *testing.T) { - var shadowPluginPath = CreateTestPlugin(t, KnTestPluginName, KnTestPluginScript, FileModeExecutable) - verifier.seenPlugins[KnTestPluginName] = pluginPath - defer DeleteTestPlugin(t, shadowPluginPath) - - t.Run("fails with overshadowed error", func(t *testing.T) { - eaw := errorsAndWarnings{} - eaw = verifier.verify(eaw, shadowPluginPath) - assert.Assert(t, len(eaw.errors) == 0) - assert.Assert(t, len(eaw.warnings) == 1) - assert.Assert(t, util.ContainsAll(eaw.warnings[0], "shadowed", "ignored")) - }) - }) - }) - - t.Run("when kn plugin in path overwrites existing command", func(t *testing.T) { - setup(t) - defer cleanup(t) - var overwritingPluginPath = CreateTestPlugin(t, "kn-plugin", KnTestPluginScript, FileModeExecutable) - defer DeleteTestPlugin(t, overwritingPluginPath) - - t.Run("fails with overwrites error", func(t *testing.T) { - eaw := errorsAndWarnings{} - eaw = verifier.verify(eaw, overwritingPluginPath) - assert.Assert(t, len(eaw.errors) == 1) - assert.Assert(t, len(eaw.warnings) == 0) - assert.Assert(t, util.ContainsAll(eaw.errors[0], "overwrite", "kn-plugin")) - }) - }) - - t.Run("when kn plugin in path overwrites 'source' command (which is allowed)", func(t *testing.T) { - setup(t) - defer cleanup(t) - var overwritingPluginPath = CreateTestPlugin(t, "kn-source-test", KnTestPluginScript, FileModeExecutable) - defer DeleteTestPlugin(t, overwritingPluginPath) - - t.Run("runs successfully", func(t *testing.T) { - eaw := errorsAndWarnings{} - eaw = verifier.verify(eaw, overwritingPluginPath) - assert.Assert(t, len(eaw.errors) == 0) - assert.Assert(t, len(eaw.warnings) == 0) - }) - }) - }) -} - -func isExecutable(plugin string) bool { - _, err := exec.Command(plugin).Output() - return err == nil -} - -func getExecTestUids() []int { - currentUser := os.Getuid() - // Only root can switch ownership of a file - if currentUser == 0 { - foreignUser, err := lookupForeignUser() - if err == nil { - return []int{currentUser, foreignUser} - } - } - return []int{currentUser} -} - -func getExecTestGids() []int { - currentUser := os.Getuid() - currentGroup := os.Getgid() - // Only root can switch group of a file - if currentUser == 0 { - foreignGroup, err := lookupForeignGroup() - if err == nil { - return []int{currentGroup, foreignGroup} - } - } - return []int{currentGroup} -} - -func lookupForeignUser() (int, error) { - for _, probe := range []string{"daemon", "nobody", "_unknown"} { - u, err := user.Lookup(probe) - if err != nil { - continue - } - uid, err := strconv.Atoi(u.Uid) - if err != nil { - continue - } - if uid != os.Getuid() { - return uid, nil - } - } - return 0, errors.New("could not find foreign user") -} - -func lookupForeignGroup() (int, error) { - gids, err := os.Getgroups() - if err != nil { - return 0, err - } -OUTER: - for _, probe := range []string{"daemon", "wheel", "nobody", "nogroup", "admin"} { - group, err := user.LookupGroup(probe) - if err != nil { - continue - } - gid, err := strconv.Atoi(group.Gid) - if err != nil { - continue - } - - for _, g := range gids { - if gid == g { - continue OUTER - } - } - return gid, nil - } - return 0, errors.New("could not find a foreign group") -} - -func prepareFile(file string, uid int, gid int, perm os.FileMode) error { - err := os.Chown(file, uid, gid) - if err != nil { - return err - } - return os.Chmod(file, perm) -} diff --git a/pkg/kn/commands/plugin/verifier_windows.go b/pkg/kn/commands/plugin/verifier_windows.go deleted file mode 100644 index 76e98026bc..0000000000 --- a/pkg/kn/commands/plugin/verifier_windows.go +++ /dev/null @@ -1,191 +0,0 @@ -// Copyright © 2019 The Knative 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. - -// This file is replacement of plugin_verifier.go file in same dir, and -// compiles only for Windows platform (its suffix defines that). - -package plugin - -import ( - "errors" - "fmt" - "io" - "os" - "path/filepath" - "runtime" - "strings" - - "github.com/spf13/cobra" -) - -// pluginVerifier verifies that existing kn commands are not overridden -type pluginVerifier struct { - root *cobra.Command - seenPlugins map[string]string -} - -// collect errors and warnings on the way -type errorsAndWarnings struct { - errors []string - warnings []string -} - -// Create new verifier -func newPluginVerifier(root *cobra.Command) *pluginVerifier { - return &pluginVerifier{ - root: root, - seenPlugins: make(map[string]string), - } -} - -// permission bits for execute -const ( - UserExecute = 1 << 6 - GroupExecute = 1 << 3 - OtherExecute = 1 << 0 -) - -// Verify implements pathVerifier and determines if a given path -// is valid depending on whether or not it overwrites an existing -// kn command path, or a previously seen plugin. -// This method is not idempotent and must be called for each path only once. -func (v *pluginVerifier) verify(eaw errorsAndWarnings, path string) errorsAndWarnings { - if v.root == nil { - return eaw.addError("unable to verify path with nil root") - } - - // Verify that plugin actually exists - fileInfo, err := os.Stat(path) - if err != nil { - if err == os.ErrNotExist { - return eaw.addError("cannot find plugin in %s", path) - } - return eaw.addError("cannot stat %s: %v", path, err) - } - - eaw = v.addErrorIfWrongPrefix(eaw, path) - eaw = v.addWarningIfNotExecutable(eaw, path, fileInfo) - eaw = v.addWarningIfAlreadySeen(eaw, path) - eaw = v.addErrorIfOverwritingExistingCommand(eaw, path) - - // Remember each verified plugin for duplicate check - v.seenPlugins[filepath.Base(path)] = path - - return eaw -} - -func (v *pluginVerifier) addWarningIfAlreadySeen(eaw errorsAndWarnings, path string) errorsAndWarnings { - fileName := filepath.Base(path) - if existingPath, ok := v.seenPlugins[fileName]; ok { - return eaw.addWarning("%s is ignored because it is shadowed by an equally named plugin: %s.", path, existingPath) - } - return eaw -} - -func (v *pluginVerifier) addErrorIfOverwritingExistingCommand(eaw errorsAndWarnings, path string) errorsAndWarnings { - fileName := filepath.Base(path) - cmds := strings.Split(fileName, "-") - if len(cmds) < 2 { - return eaw.addError("%s is not a valid plugin filename as its missing a prefix", fileName) - } - cmds = cmds[1:] - - // Check both, commands with underscore and with dash because plugins can be called with both - overwrittenCommands := make(map[string]bool) - for _, c := range [][]string{cmds, convertUnderscoresToDashes(cmds)} { - cmd, _, err := v.root.Find(c) - if err == nil { - overwrittenCommands[cmd.CommandPath()] = true - } - } - for command := range overwrittenCommands { - eaw.addError("%s overwrites existing built-in command: %s", fileName, command) - } - return eaw -} - -func (v *pluginVerifier) addErrorIfWrongPrefix(eaw errorsAndWarnings, path string) errorsAndWarnings { - fileName := filepath.Base(path) - // Only pick the first prefix as it is very like that it will be reduced to - // a single prefix anyway (PR pending) - prefix := ValidPluginFilenamePrefixes[0] - if !strings.HasPrefix(fileName, prefix+"-") { - eaw.addWarning("%s plugin doesn't start with plugin prefix %s", fileName, prefix) - } - return eaw -} - -func (v *pluginVerifier) addWarningIfNotExecutable(eaw errorsAndWarnings, path string, fileInfo os.FileInfo) errorsAndWarnings { - if runtime.GOOS == "windows" { - return checkForWindowsExecutable(eaw, fileInfo, path) - } - return eaw -} - -func checkForWindowsExecutable(eaw errorsAndWarnings, fileInfo os.FileInfo, path string) errorsAndWarnings { - fileExt := strings.ToLower(filepath.Ext(fileInfo.Name())) - - switch fileExt { - case ".bat", ".cmd", ".com", ".exe", ".ps1": - return eaw - } - return eaw.addWarning("%s is not executable as it does not have the proper extension", path) -} - -func (eaw *errorsAndWarnings) addError(format string, args ...interface{}) errorsAndWarnings { - eaw.errors = append(eaw.errors, fmt.Sprintf(format, args...)) - return *eaw -} - -func (eaw *errorsAndWarnings) addWarning(format string, args ...interface{}) errorsAndWarnings { - eaw.warnings = append(eaw.warnings, fmt.Sprintf(format, args...)) - return *eaw -} - -func (eaw *errorsAndWarnings) printWarningsAndErrors(out io.Writer) { - printSection(out, "ERROR", eaw.errors) - printSection(out, "WARNING", eaw.warnings) -} - -func (eaw *errorsAndWarnings) combinedError() error { - if len(eaw.errors) == 0 { - return nil - } - return errors.New(strings.Join(eaw.errors, ",")) -} - -func printSection(out io.Writer, label string, values []string) { - if len(values) > 0 { - printLabelWithConditionalPluralS(out, label, len(values)) - for _, value := range values { - fmt.Fprintf(out, " - %s\n", value) - } - } -} - -func printLabelWithConditionalPluralS(out io.Writer, label string, nr int) { - if nr == 1 { - fmt.Fprintf(out, "%s:\n", label) - } else { - fmt.Fprintf(out, "%ss:\n", label) - } -} - -func convertUnderscoresToDashes(cmds []string) []string { - ret := make([]string, len(cmds)) - for i := range cmds { - ret[i] = strings.ReplaceAll(cmds[i], "_", "-") - } - return ret -} diff --git a/pkg/kn/commands/testing_helper.go b/pkg/kn/commands/testing_helper.go index 94db2a64a5..13693d9049 100644 --- a/pkg/kn/commands/testing_helper.go +++ b/pkg/kn/commands/testing_helper.go @@ -16,13 +16,11 @@ package commands import ( "bytes" - "flag" - "io" + "io/ioutil" "os" "testing" "github.com/spf13/cobra" - "github.com/spf13/viper" "gotest.tools/assert" "k8s.io/apimachinery/pkg/runtime" clienttesting "k8s.io/client-go/testing" @@ -33,11 +31,9 @@ import ( "knative.dev/client/pkg/sources/v1alpha2" dynamicfake "k8s.io/client-go/dynamic/fake" - eventingv1beta1fake "knative.dev/eventing/pkg/client/clientset/versioned/typed/eventing/v1beta1/fake" sourcesv1alpha2fake "knative.dev/eventing/pkg/client/clientset/versioned/typed/sources/v1alpha2/fake" clientdynamic "knative.dev/client/pkg/dynamic" - eventingv1beta1 "knative.dev/client/pkg/eventing/v1beta1" ) const FakeNamespace = "current" @@ -59,7 +55,7 @@ func CreateTestKnCommand(cmd *cobra.Command, knParams *KnParams) (*cobra.Command return clientservingv1.NewKnServingClient(fakeServing, FakeNamespace), nil } knParams.fixedCurrentNamespace = FakeNamespace - knCommand := NewKnTestCommand(cmd, knParams) + knCommand := NewTestCommand(cmd, knParams) return knCommand, fakeServing, buf } @@ -67,37 +63,18 @@ func CreateTestKnCommand(cmd *cobra.Command, knParams *KnParams) (*cobra.Command func CreateSourcesTestKnCommand(cmd *cobra.Command, knParams *KnParams) (*cobra.Command, *sourcesv1alpha2fake.FakeSourcesV1alpha2, *bytes.Buffer) { buf := new(bytes.Buffer) // create fake serving client because the sink of source depends on serving client - fakeServing := &servingv1fake.FakeServingV1{&clienttesting.Fake{}} + fakeServing := &servingv1fake.FakeServingV1{Fake: &clienttesting.Fake{}} knParams.NewServingClient = func(namespace string) (clientservingv1.KnServingClient, error) { return clientservingv1.NewKnServingClient(fakeServing, FakeNamespace), nil } // create fake sources client - fakeEventing := &sourcesv1alpha2fake.FakeSourcesV1alpha2{&clienttesting.Fake{}} + fakeEventing := &sourcesv1alpha2fake.FakeSourcesV1alpha2{Fake: &clienttesting.Fake{}} knParams.Output = buf knParams.NewSourcesClient = func(namespace string) (v1alpha2.KnSourcesClient, error) { return v1alpha2.NewKnSourcesClient(fakeEventing, FakeNamespace), nil } knParams.fixedCurrentNamespace = FakeNamespace - knCommand := NewKnTestCommand(cmd, knParams) - return knCommand, fakeEventing, buf -} - -// CreateEventingTestKnCommand helper for creating test commands -func CreateEventingTestKnCommand(cmd *cobra.Command, knParams *KnParams) (*cobra.Command, *eventingv1beta1fake.FakeEventingV1beta1, *bytes.Buffer) { - buf := new(bytes.Buffer) - // create fake serving client because the sink of source depends on serving client - fakeServing := &servingv1fake.FakeServingV1{&clienttesting.Fake{}} - knParams.NewServingClient = func(namespace string) (clientservingv1.KnServingClient, error) { - return clientservingv1.NewKnServingClient(fakeServing, FakeNamespace), nil - } - // create fake sources client - fakeEventing := &eventingv1beta1fake.FakeEventingV1beta1{&clienttesting.Fake{}} - knParams.Output = buf - knParams.NewEventingClient = func(namespace string) (eventingv1beta1.KnEventingClient, error) { - return eventingv1beta1.NewKnEventingClient(fakeEventing, FakeNamespace), nil - } - knParams.fixedCurrentNamespace = FakeNamespace - knCommand := NewKnTestCommand(cmd, knParams) + knCommand := NewTestCommand(cmd, knParams) return knCommand, fakeEventing, buf } @@ -111,87 +88,51 @@ func CreateDynamicTestKnCommand(cmd *cobra.Command, knParams *KnParams, objects } knParams.fixedCurrentNamespace = FakeNamespace - knCommand := NewKnTestCommand(cmd, knParams) + knCommand := NewTestCommand(cmd, knParams) return knCommand, fakeDynamic, buf } -// CaptureStdout collects the current content of os.Stdout -func CaptureStdout(t *testing.T) { - oldStdout = os.Stdout - var err error - readFile, writeFile, err = os.Pipe() - assert.Assert(t, err == nil) - stdout = writeFile - os.Stdout = writeFile -} +type StdoutCapture struct { + r, w *os.File + t *testing.T -// ReleaseStdout releases the os.Stdout and restores to original -func ReleaseStdout(t *testing.T) { - output = ReadStdout(t) - os.Stdout = oldStdout + oldStdout *os.File } -// ReadStdout returns the collected os.Stdout content -func ReadStdout(t *testing.T) string { - outC := make(chan string) - go func() { - var buf bytes.Buffer - io.Copy(&buf, readFile) - outC <- buf.String() - }() - writeFile.Close() - output = <-outC - - CaptureStdout(t) - - return output +func CaptureStdout(t *testing.T) StdoutCapture { + ret := StdoutCapture{ + oldStdout: os.Stdout, + t: t, + } + var err error + ret.r, ret.w, err = os.Pipe() + assert.NilError(t, err) + os.Stdout = ret.w + return ret } -// Private +// CaptureStdout collects the current content of os.Stdout +func (c StdoutCapture) Close() string { + err := c.w.Close() + assert.NilError(c.t, err) + ret, err := ioutil.ReadAll(c.r) + assert.NilError(c.t, err) + os.Stdout = c.oldStdout + return string(ret) +} -// NewKnTestCommand needed since calling the one in core would cause a import cycle -func NewKnTestCommand(subCommand *cobra.Command, params *KnParams) *cobra.Command { +// NewTestCommand can be used by tes +func NewTestCommand(subCommand *cobra.Command, params *KnParams) *cobra.Command { rootCmd := &cobra.Command{ - Use: "kn", - Short: "Knative client", - Long: `Manage your Knative building blocks: - -Serving: Manage your services and release new software to them. -Build: Create builds and keep track of their results. -Eventing: Manage event subscriptions and channels. Connect up event sources.`, - - // Disable docs header - DisableAutoGenTag: true, - - // Affects children as well - SilenceUsage: true, - - // Prevents Cobra from dealing with errors as we deal with them in main.go - SilenceErrors: true, - + Use: "kn", PersistentPreRunE: func(cmd *cobra.Command, args []string) error { return flags.ReconcileBoolFlags(cmd.Flags()) }, } if params.Output != nil { - rootCmd.SetOutput(params.Output) + rootCmd.SetOut(params.Output) } - rootCmd.PersistentFlags().StringVar(&CfgFile, "config", "", "config file (default is ~/.config/kn/config.yaml)") - rootCmd.PersistentFlags().StringVar(¶ms.KubeCfgPath, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)") - - rootCmd.Flags().StringVar(&Cfg.PluginsDir, "plugins-dir", "~/.config/kn/plugins", "kn plugins directory") - rootCmd.Flags().BoolVar(Cfg.LookupPlugins, "lookup-plugins", false, "look for kn plugins in $PATH") - - viper.BindPFlag("plugins-dir", rootCmd.Flags().Lookup("plugins-dir")) - viper.BindPFlag("lookup-plugins", rootCmd.Flags().Lookup("lookup-plugins")) - - viper.SetDefault("plugins-dir", "~/.config/kn/plugins") - viper.SetDefault("lookup-plugins", false) - rootCmd.AddCommand(subCommand) - - // For glog parse error. - flag.CommandLine.Parse([]string{}) return rootCmd } diff --git a/pkg/kn/commands/testing_helper_test.go b/pkg/kn/commands/testing_helper_test.go index fedc2e73a9..3a729e1e8d 100644 --- a/pkg/kn/commands/testing_helper_test.go +++ b/pkg/kn/commands/testing_helper_test.go @@ -15,110 +15,49 @@ package commands import ( - "bytes" - "strings" + "fmt" "testing" "github.com/spf13/cobra" "gotest.tools/assert" - dynamicfake "k8s.io/client-go/dynamic/fake" - sourcesv1alpha2fake "knative.dev/eventing/pkg/client/clientset/versioned/typed/sources/v1alpha2/fake" - servingv1fake "knative.dev/serving/pkg/client/clientset/versioned/typed/serving/v1/fake" ) func TestCreateTestKnCommand(t *testing.T) { - var ( - knCmd *cobra.Command - serving *servingv1fake.FakeServingV1 - buffer *bytes.Buffer - ) - - setup := func(t *testing.T) { - knParams := &KnParams{} - knCmd, serving, buffer = CreateTestKnCommand(&cobra.Command{Use: "fake"}, knParams) - assert.Assert(t, knCmd != nil) - assert.Assert(t, len(knCmd.Commands()) == 1) - assert.Assert(t, knCmd.Commands()[0].Use == "fake") - assert.Assert(t, serving != nil) - assert.Assert(t, buffer != nil) - } - - t.Run("creates a new kn cobra.Command", func(t *testing.T) { - setup(t) - - assert.Assert(t, knCmd != nil) - assert.Assert(t, knCmd.Use == "kn") - assert.Assert(t, knCmd.Short == "Knative client") - assert.Assert(t, strings.Contains(knCmd.Long, "Manage your Knative building blocks:")) - assert.Assert(t, knCmd.RunE == nil) - assert.Assert(t, knCmd.DisableAutoGenTag == true) - assert.Assert(t, knCmd.SilenceUsage == true) - assert.Assert(t, knCmd.SilenceErrors == true) - }) + knParams := &KnParams{} + knCmd, serving, buffer := CreateTestKnCommand(&cobra.Command{Use: "fake"}, knParams) + assert.Assert(t, knCmd != nil) + assert.Assert(t, serving != nil) + assert.Assert(t, buffer != nil) + assert.Assert(t, len(knCmd.Commands()) == 1) + assert.Assert(t, knCmd.Commands()[0].Use == "fake") } func TestCreateSourcesTestKnCommand(t *testing.T) { - var ( - knCmd *cobra.Command - sources *sourcesv1alpha2fake.FakeSourcesV1alpha2 - buffer *bytes.Buffer - ) - - setup := func(t *testing.T) { - knParams := &KnParams{} - knCmd, sources, buffer = CreateSourcesTestKnCommand(&cobra.Command{Use: "fake"}, knParams) - assert.Assert(t, knCmd != nil) - assert.Assert(t, len(knCmd.Commands()) == 1) - assert.Assert(t, knCmd.Commands()[0].Use == "fake") - assert.Assert(t, sources != nil) - assert.Assert(t, buffer != nil) - } - - t.Run("creates a new kn cobra.Command", func(t *testing.T) { - setup(t) - - assert.Assert(t, knCmd != nil) - assert.Assert(t, knCmd.Use == "kn") - assert.Assert(t, knCmd.Short == "Knative client") - assert.Assert(t, strings.Contains(knCmd.Long, "Manage your Knative building blocks:")) - assert.Assert(t, knCmd.RunE == nil) - assert.Assert(t, knCmd.DisableAutoGenTag == true) - assert.Assert(t, knCmd.SilenceUsage == true) - assert.Assert(t, knCmd.SilenceErrors == true) - }) + knParams := &KnParams{} + knCmd, sources, buffer := CreateSourcesTestKnCommand(&cobra.Command{Use: "fake"}, knParams) + assert.Assert(t, knCmd != nil) + assert.Assert(t, sources != nil) + assert.Assert(t, buffer != nil) + assert.Assert(t, len(knCmd.Commands()) == 1) + assert.Assert(t, knCmd.Commands()[0].Use == "fake") } func TestCreateDynamicTestKnCommand(t *testing.T) { - var ( - knCmd *cobra.Command - dynamic *dynamicfake.FakeDynamicClient - buffer *bytes.Buffer - ) - - setup := func(t *testing.T) { - knParams := &KnParams{} - knCmd, dynamic, buffer = CreateDynamicTestKnCommand(&cobra.Command{Use: "fake"}, knParams) - assert.Assert(t, knCmd != nil) - assert.Assert(t, len(knCmd.Commands()) == 1) - assert.Assert(t, knCmd.Commands()[0].Use == "fake") - assert.Assert(t, dynamic != nil) - assert.Assert(t, buffer != nil) - client, err := knParams.NewDynamicClient("foo") - assert.NilError(t, err) - assert.Assert(t, client != nil) - } - - t.Run("creates a new kn cobra.Command", func(t *testing.T) { - setup(t) - - assert.Assert(t, knCmd != nil) - assert.Assert(t, knCmd.Use == "kn") - assert.Assert(t, knCmd.Short == "Knative client") - assert.Assert(t, strings.Contains(knCmd.Long, "Manage your Knative building blocks:")) - assert.Assert(t, knCmd.RunE == nil) - assert.Assert(t, knCmd.DisableAutoGenTag == true) - assert.Assert(t, knCmd.SilenceUsage == true) - assert.Assert(t, knCmd.SilenceErrors == true) - }) + knParams := &KnParams{} + knCmd, dynamic, buffer := CreateDynamicTestKnCommand(&cobra.Command{Use: "fake"}, knParams) + assert.Assert(t, knCmd != nil) + assert.Assert(t, dynamic != nil) + assert.Assert(t, buffer != nil) + assert.Assert(t, len(knCmd.Commands()) == 1) + assert.Assert(t, knCmd.Commands()[0].Use == "fake") + client, err := knParams.NewDynamicClient("foo") + assert.NilError(t, err) + assert.Assert(t, client != nil) +} +func TestCaptureStdout(t *testing.T) { + c := CaptureStdout(t) + fmt.Print("Hello World !") + out := c.Close() + assert.Equal(t, out, "Hello World !") } diff --git a/pkg/kn/commands/types.go b/pkg/kn/commands/types.go index bda7a52606..4c57cf3a4d 100644 --- a/pkg/kn/commands/types.go +++ b/pkg/kn/commands/types.go @@ -19,7 +19,6 @@ import ( "io" "os" "path/filepath" - "runtime" "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" @@ -37,34 +36,6 @@ import ( clientservingv1 "knative.dev/client/pkg/serving/v1" ) -// CfgFile is Kn's config file is the path for the Kubernetes config -var CfgFile string - -// Cfg is Kn's configuration values -var Cfg Config = Config{ - DefaultConfigDir: newDefaultConfigPath(""), - DefaultPluginDir: newDefaultConfigPath("plugins"), - PluginsDir: "", - LookupPlugins: newBoolP(false), -} - -// Config contains the variables for the Kn config -type Config struct { - DefaultConfigDir string - DefaultPluginDir string - PluginsDir string - LookupPlugins *bool - SinkPrefixes []SinkPrefixConfig -} - -// SinkPrefixConfig is the struct of sink prefix config in kn config -type SinkPrefixConfig struct { - Prefix string - Resource string - Group string - Version string -} - // KnParams for creating commands. Useful for inserting mocks for testing. type KnParams struct { Output io.Writer @@ -188,19 +159,3 @@ func (params *KnParams) GetClientConfig() (clientcmd.ClientConfig, error) { } return nil, fmt.Errorf("Config file '%s' can not be found", params.KubeCfgPath) } - -// Private - -// Returns a pointer to bool, hard to do better in Golang -func newBoolP(b bool) *bool { - aBool := b - return &aBool -} - -// Returns default config path based on target OS -func newDefaultConfigPath(subDir string) string { - if runtime.GOOS == "windows" { - return filepath.Join(os.Getenv("APPDATA"), "kn", subDir) - } - return filepath.Join("~", ".config", "kn", subDir) -} diff --git a/pkg/kn/config/config.go b/pkg/kn/config/config.go new file mode 100644 index 0000000000..48088e25cf --- /dev/null +++ b/pkg/kn/config/config.go @@ -0,0 +1,252 @@ +// Copyright © 2020 The Knative 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 config + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + + homedir "github.com/mitchellh/go-homedir" + "github.com/pkg/errors" + flag "github.com/spf13/pflag" + "github.com/spf13/viper" +) + +// bootstrapDefaults are the defaults values to use +type defaultConfig struct { + configFile string + pluginsDir string + lookupPluginsInPath bool +} + +// Initialize defaults +var bootstrapDefaults = initDefaults() + +// config contains the variables for the Kn config +type config struct { + // configFile is the config file location + configFile string + + // sinkMappings is a list of sink mapping + sinkMappings []SinkMapping +} + +// ConfigFile returns the config file which is either the default XDG conform +// config file location or the one set with --config +func (c *config) ConfigFile() string { + if c.configFile != "" { + return c.configFile + } + return bootstrapDefaults.configFile +} + +// PluginsDir returns the plugins' directory +func (c *config) PluginsDir() string { + if viper.IsSet(keyPluginsDirectory) { + return viper.GetString(keyPluginsDirectory) + } else if viper.IsSet(legacyKeyPluginsDirectory) { + // Remove that branch if legacy option is switched off + return viper.GetString(legacyKeyPluginsDirectory) + } else { + return bootstrapDefaults.pluginsDir + } +} + +// LookupPluginsInPath returns true if plugins should be also checked in the pat +func (c *config) LookupPluginsInPath() bool { + if viper.IsSet(keyPluginsLookupInPath) { + return viper.GetBool(keyPluginsLookupInPath) + } else if viper.IsSet(legacyKeyPluginsLookupInPath) { + // Remove that branch if legacy option is switched off + return viper.GetBool(legacyKeyPluginsLookupInPath) + } else { + // If legacy branch is removed, switch to setting the default to viper + // See TODO comment below. + return bootstrapDefaults.lookupPluginsInPath + } +} + +func (c *config) SinkMappings() []SinkMapping { + return c.sinkMappings +} + +// Config used for flag binding +var globalConfig = config{} + +// GlobalConfig is the global configuration available for every sub-command +var GlobalConfig Config = &globalConfig + +// bootstrapConfig reads in config file and boostrap options if set. +func BootstrapConfig() error { + + // Create a new FlagSet for the bootstrap flags and parse those. This will + // initialize the config file to use (obtained via GlobalConfig.ConfigFile()) + bootstrapFlagSet := flag.NewFlagSet("kn", flag.ContinueOnError) + AddBootstrapFlags(bootstrapFlagSet) + bootstrapFlagSet.ParseErrorsWhitelist = flag.ParseErrorsWhitelist{UnknownFlags: true} + bootstrapFlagSet.Usage = func() {} + err := bootstrapFlagSet.Parse(os.Args) + if err != nil && err != flag.ErrHelp { + return err + } + + // Bind flags so that options that have been provided have priority. + // Important: Always read options via GlobalConfig methods + err = viper.BindPFlag(keyPluginsDirectory, bootstrapFlagSet.Lookup(flagPluginsDir)) + if err != nil { + return err + } + err = viper.BindPFlag(keyPluginsLookupInPath, bootstrapFlagSet.Lookup(flagPluginsLookupInPath)) + if err != nil { + return err + } + + // Check if configfile exists. If not, just return + configFile := GlobalConfig.ConfigFile() + _, err = os.Lstat(configFile) + if err != nil { + if os.IsNotExist(err) { + // No config file to read + return nil + } + return errors.Wrap(err, fmt.Sprintf("cannot stat configfile %s", configFile)) + } + + viper.SetConfigFile(GlobalConfig.ConfigFile()) + viper.AutomaticEnv() // read in environment variables that match + + // Defaults are taken from the parsed flags, which in turn have bootstrapDefaults + // TODO: Re-enable when legacy handling for plugin config has been removed + // For now default handling is happening directly in the getter of GlobalConfig + // viper.SetDefault(keyPluginsDirectory, bootstrapDefaults.pluginsDir) + // viper.SetDefault(keyPluginsLookupInPath, bootstrapDefaults.lookupPluginsInPath) + + // If a config file is found, read it in. + err = viper.ReadInConfig() + if err != nil { + return err + } + + // Deserialize sink mappings if configured + err = parseSinkMappings() + return err +} + +// Add bootstrap flags use in a separate bootstrap proceeds +func AddBootstrapFlags(flags *flag.FlagSet) { + flags.StringVar(&globalConfig.configFile, "config", "", fmt.Sprintf("kn configuration file (default: %s)", defaultConfigFileForUsageMessage())) + flags.String(flagPluginsDir, "", "Directory holding kn plugins") + flags.Bool(flagPluginsLookupInPath, false, "Search kn plugins also in $PATH") + + // Let's try that and mark the flags as hidden: (as those configuration is a permanent choice of operation) + flags.MarkHidden(flagPluginsLookupInPath) + flags.MarkHidden(flagPluginsDir) +} + +// =========================================================================================================== + +// Initialize defaults. This happens lazily go allow to change the +// home directory for e.g. tests +func initDefaults() *defaultConfig { + return &defaultConfig{ + configFile: defaultConfigLocation("config.yaml"), + pluginsDir: defaultConfigLocation("plugins"), + lookupPluginsInPath: false, + } +} + +func defaultConfigLocation(subpath string) string { + var base string + if runtime.GOOS == "windows" { + base = defaultConfigDirWindows() + } else { + base = defaultConfigDirUnix() + } + return filepath.Join(base, subpath) +} + +func defaultConfigDirUnix() string { + home, err := homedir.Dir() + if err != nil { + home = "~" + } + + // Check the deprecated path first and fallback to it, add warning to error message + if configHome := filepath.Join(home, ".kn"); dirExists(configHome) { + migrationPath := filepath.Join(home, ".config", "kn") + fmt.Fprintf(os.Stderr, "WARNING: deprecated kn config directory '%s' detected. Please move your configuration to '%s'\n", configHome, migrationPath) + return configHome + } + + // Respect XDG_CONFIG_HOME if set + if xdgHome := os.Getenv("XDG_CONFIG_HOME"); xdgHome != "" { + return filepath.Join(xdgHome, "kn") + } + // Fallback to XDG default for both Linux and macOS + // ~/.config/kn + return filepath.Join(home, ".config", "kn") +} + +func defaultConfigDirWindows() string { + home, err := homedir.Dir() + if err != nil { + // Check the deprecated path first and fallback to it, add warning to error message + if configHome := filepath.Join(home, ".kn"); dirExists(configHome) { + migrationPath := filepath.Join(os.Getenv("APPDATA"), "kn") + fmt.Fprintf(os.Stderr, "WARNING: deprecated kn config directory '%s' detected. Please move your configuration to '%s'\n", configHome, migrationPath) + return configHome + } + } + + return filepath.Join(os.Getenv("APPDATA"), "kn") +} + +func dirExists(path string) bool { + if _, err := os.Stat(path); !os.IsNotExist(err) { + return true + } + return false +} + +// parse sink mappings and store them in the global configuration +func parseSinkMappings() error { + // Parse sink configuration + key := "" + if viper.IsSet(keySinkMappings) { + key = keySinkMappings + } + if key == "" && viper.IsSet(legacyKeySinkMappings) { + key = legacyKeySinkMappings + } + + if key != "" { + err := viper.UnmarshalKey(key, &globalConfig.sinkMappings) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("error while parsing sink mappings in configuration file %s", + viper.ConfigFileUsed())) + } + } + return nil +} + +// Prepare the default config file for the usage message +func defaultConfigFileForUsageMessage() string { + if runtime.GOOS == "windows" { + return "%APPDATA%\\kn\\config.yaml" + } + return "~/.config/kn/config.yaml" +} diff --git a/pkg/kn/config/config_test.go b/pkg/kn/config/config_test.go new file mode 100644 index 0000000000..53e5037c44 --- /dev/null +++ b/pkg/kn/config/config_test.go @@ -0,0 +1,141 @@ +// Copyright © 2020 The Knative 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 config + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + homedir "github.com/mitchellh/go-homedir" + "github.com/spf13/viper" + "gotest.tools/assert" +) + +func TestBootstrapConfig(t *testing.T) { + configYaml := ` +plugins: + directory: /tmp + path-lookup: true + +eventing: + sink-mappings: + - prefix: service + resource: services + group: core + version: v1 +` + + configFile, cleanup := setupConfig(t, configYaml) + defer cleanup() + + err := BootstrapConfig() + assert.NilError(t, err) + + assert.Equal(t, GlobalConfig.ConfigFile(), configFile) + assert.Equal(t, GlobalConfig.PluginsDir(), "/tmp") + assert.Equal(t, GlobalConfig.LookupPluginsInPath(), true) + assert.Equal(t, len(GlobalConfig.SinkMappings()), 1) + assert.DeepEqual(t, (GlobalConfig.SinkMappings())[0], SinkMapping{ + Prefix: "service", + Resource: "services", + Group: "core", + Version: "v1", + }) +} + +func TestBootstrapConfigWithoutConfigFile(t *testing.T) { + _, cleanup := setupConfig(t, "") + defer cleanup() + + err := BootstrapConfig() + assert.NilError(t, err) + + assert.Equal(t, GlobalConfig.ConfigFile(), bootstrapDefaults.configFile) + assert.Equal(t, GlobalConfig.PluginsDir(), bootstrapDefaults.pluginsDir) + assert.Equal(t, GlobalConfig.LookupPluginsInPath(), bootstrapDefaults.lookupPluginsInPath) + assert.Equal(t, len(GlobalConfig.SinkMappings()), 0) +} + +func TestBootstrapLegacyConfigFields(t *testing.T) { + configYaml := ` +plugins-dir: /legacy-plugin +lookup-plugins: true +sink: +- prefix: service + resource: services + group: core + version: v1 +` + + configFile, cleanup := setupConfig(t, configYaml) + defer cleanup() + + err := BootstrapConfig() + assert.NilError(t, err) + + assert.Equal(t, GlobalConfig.ConfigFile(), configFile) + assert.Equal(t, GlobalConfig.PluginsDir(), "/legacy-plugin") + assert.Equal(t, GlobalConfig.LookupPluginsInPath(), true) + assert.Equal(t, len(GlobalConfig.SinkMappings()), 1) + assert.DeepEqual(t, (GlobalConfig.SinkMappings())[0], SinkMapping{ + Prefix: "service", + Resource: "services", + Group: "core", + Version: "v1", + }) +} + +func setupConfig(t *testing.T, configContent string) (string, func()) { + tmpDir, err := ioutil.TempDir("", "configContent") + assert.NilError(t, err) + + // Avoid to be fooled by the things in the the real homedir + oldHome := os.Getenv("HOME") + os.Setenv("HOME", tmpDir) + + // Save old args + backupArgs := os.Args + + // Write out a temporary configContent file + var cfgFile string + if configContent != "" { + cfgFile = filepath.Join(tmpDir, "config.yaml") + os.Args = []string{"kn", "--config", cfgFile} + err = ioutil.WriteFile(cfgFile, []byte(configContent), 0644) + assert.NilError(t, err) + } + + // Reset various global state + oldHomeDirDisableCache := homedir.DisableCache + homedir.DisableCache = true + viper.Reset() + globalConfig = config{} + GlobalConfig = &globalConfig + bootstrapDefaults = initDefaults() + + return cfgFile, func() { + // Cleanup everything + os.RemoveAll(tmpDir) + os.Setenv("HOME", oldHome) + os.Args = backupArgs + bootstrapDefaults = initDefaults() + viper.Reset() + homedir.DisableCache = oldHomeDirDisableCache + globalConfig = config{} + GlobalConfig = &globalConfig + } +} diff --git a/pkg/kn/config/testing.go b/pkg/kn/config/testing.go new file mode 100644 index 0000000000..0ea521a77b --- /dev/null +++ b/pkg/kn/config/testing.go @@ -0,0 +1,33 @@ +// Copyright © 2020 The Knative 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 config + +// Implementation of Config useful for testing purposes +// Set an instance of this for config.GlobalConfig to mock +// your own configuration setup +type TestConfig struct { + TestPluginsDir string + TestConfigFile string + TestLookupPluginsInPath bool + TestSinkMappings []SinkMapping +} + +// Ensure that TestConfig implements the configuration interface +var _ Config = &TestConfig{} + +func (t TestConfig) PluginsDir() string { return t.TestPluginsDir } +func (t TestConfig) ConfigFile() string { return t.TestConfigFile } +func (t TestConfig) LookupPluginsInPath() bool { return t.TestLookupPluginsInPath } +func (t TestConfig) SinkMappings() []SinkMapping { return t.TestSinkMappings } diff --git a/pkg/kn/config/testing_test.go b/pkg/kn/config/testing_test.go new file mode 100644 index 0000000000..5acf22b149 --- /dev/null +++ b/pkg/kn/config/testing_test.go @@ -0,0 +1,37 @@ +// Copyright © 2020 The Knative 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 config + +import ( + "testing" + + "gotest.tools/assert" +) + +// Dummy test to keep code coverage quality gate happy. +// +func TestTestConfig(t *testing.T) { + cfg := TestConfig{ + TestPluginsDir: "pluginsDir", + TestConfigFile: "configFile", + TestLookupPluginsInPath: true, + TestSinkMappings: nil, + } + + assert.Equal(t, cfg.PluginsDir(), "pluginsDir") + assert.Equal(t, cfg.ConfigFile(), "configFile") + assert.Assert(t, cfg.LookupPluginsInPath()) + assert.Assert(t, cfg.SinkMappings() == nil) +} diff --git a/pkg/kn/config/types.go b/pkg/kn/config/types.go new file mode 100644 index 0000000000..1e97c8f296 --- /dev/null +++ b/pkg/kn/config/types.go @@ -0,0 +1,71 @@ +// Copyright © 2020 The Knative 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 config + +// Package for holding configuration types used in bootstrapping +// and for types in configuration foles + +type Config interface { + + // ConfigFile returns the location of the configuration file + ConfigFile() string + + // PluginsDir returns the path to the directory containing plugins + PluginsDir() string + + // LookupPluginsInPath returns true if plugins should be also looked up + // in the execution path + LookupPluginsInPath() bool + + // SinkMappings returns additional mappings for sink prefixes to resources + SinkMappings() []SinkMapping +} + +// SinkMappings is the struct of sink prefix config in kn config +type SinkMapping struct { + + // Prefix is the mapping prefix (like "svc") + Prefix string + + // Resource is the name for the mapped resource (like "services", mind the plural) + Resource string + + // Group is the api group for the mapped resource (like "core") + Group string + + // Version is the api version for the mapped resource (like "v1") + Version string +} + +// config Keys for looking up in viper +const ( + keyPluginsDirectory = "plugins.directory" + keyPluginsLookupInPath = "plugins.path-lookup" + keySinkMappings = "eventing.sink-mappings" +) + +// legacy config keys, deprecated +const ( + legacyKeyPluginsDirectory = "plugins-dir" + legacyKeyPluginsLookupInPath = "lookup-plugins" + legacyKeySinkMappings = "sink" +) + +// Global (hidden) flags +// TODO: Remove me if decided that they are not needed +const ( + flagPluginsDir = "plugins-dir" + flagPluginsLookupInPath = "lookup-plugins" +) diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go deleted file mode 100644 index 458075a599..0000000000 --- a/pkg/kn/core/root.go +++ /dev/null @@ -1,416 +0,0 @@ -// Copyright © 2018 The Knative 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 core - -import ( - "flag" - "fmt" - "io" - "os" - "path/filepath" - "runtime" - "strconv" - "strings" - - homedir "github.com/mitchellh/go-homedir" - "github.com/spf13/cobra" - "github.com/spf13/viper" - "golang.org/x/crypto/ssh/terminal" - _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" - _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" - "knative.dev/client/pkg/kn/commands" - "knative.dev/client/pkg/kn/commands/completion" - cmdflags "knative.dev/client/pkg/kn/commands/flags" - "knative.dev/client/pkg/kn/commands/plugin" - "knative.dev/client/pkg/kn/commands/revision" - "knative.dev/client/pkg/kn/commands/route" - "knative.dev/client/pkg/kn/commands/service" - "knative.dev/client/pkg/kn/commands/source" - "knative.dev/client/pkg/kn/commands/trigger" - "knative.dev/client/pkg/kn/commands/version" - "knative.dev/client/pkg/kn/flags" -) - -// NewDefaultKnCommand creates the default `kn` command with a default plugin handler -func NewDefaultKnCommand() (*cobra.Command, error) { - rootCmd := NewKnCommand() - - // Needed since otherwise --plugins-dir and --lookup-plugins - // will not be accounted for since the plugin is not a Cobra command - // and will not be parsed - pluginsDir, lookupPluginsInPath, err := extractKnPluginFlags(os.Args) - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - return &cobra.Command{}, fmt.Errorf("%v", err) - } - - pluginHandler := plugin.NewDefaultPluginHandler(plugin.ValidPluginFilenamePrefixes, - pluginsDir, lookupPluginsInPath) - - return NewDefaultKnCommandWithArgs(rootCmd, pluginHandler, - os.Args, os.Stdin, - os.Stdout, os.Stderr) -} - -// NewDefaultKnCommandWithArgs creates the `kn` command with arguments -func NewDefaultKnCommandWithArgs(rootCmd *cobra.Command, - pluginHandler plugin.PluginHandler, - args []string, - in io.Reader, - out, - errOut io.Writer) (*cobra.Command, error) { - if pluginHandler == nil { - return rootCmd, nil - } - - // process possible plugin call - if len(args) > 1 { - cmdPathPieces := args[1:] - cmdPathPieces = removeKnPluginFlags(cmdPathPieces) // Plugin does not need these flags - - // Return fast if -h or --help is in path pieces - if helpOptionsPresent(cmdPathPieces) { - return rootCmd, nil - } - - // only look for suitable extension executables if - // the specified command does not already exist - foundCmd, innerArgs, err := rootCmd.Find(cmdPathPieces) - if err != nil { - err := plugin.HandlePluginCommand(pluginHandler, cmdPathPieces) - if err != nil { - return &cobra.Command{}, fmt.Errorf("unknown command '%s' \nRun 'kn --help' for usage", args[1]) - } - } - - // when the call is on a leaf command, with sub commands - if foundCmd.HasSubCommands() { - // look for case of a plugin's command that shadows - // an existing command's subcommand - if len(innerArgs) > 0 { - cmdName := innerArgs[0] - for _, subcommand := range foundCmd.Commands() { - if subcommand.Name() == cmdName { - return &cobra.Command{}, fmt.Errorf("Error: sub-command '%s' for '%s' already exists.\nRun 'kn --help' for usage.\n", cmdName, foundCmd.Name()) - } - } - - // try to handle a plugin for a command extending a core comand group - err = plugin.HandlePluginCommand(pluginHandler, cmdPathPieces) - if err != nil { - return &cobra.Command{}, fmt.Errorf("Error: unknown sub-command '%s' for command '%s'\nRun 'kn --help' for usage.\n", cmdName, foundCmd.Name()) - } - } else { - _, _, err := rootCmd.Find(innerArgs) - if err != nil { - return &cobra.Command{}, fmt.Errorf(showSubcommands(foundCmd, cmdPathPieces, innerArgs[0])) - } - } - } - } - - return rootCmd, nil -} - -// NewKnCommand creates the rootCmd which is the base command when called without any subcommands -func NewKnCommand(params ...commands.KnParams) *cobra.Command { - var p *commands.KnParams - if len(params) == 0 { - p = &commands.KnParams{} - } else if len(params) == 1 { - p = ¶ms[0] - } else { - panic("Too many params objects to NewKnCommand") - } - p.Initialize() - - rootCmd := &cobra.Command{ - Use: "kn", - Short: "Knative client", - Long: `Manage your Knative building blocks: - -* Serving: Manage your services and release new software to them. -* Eventing: Manage event subscriptions and channels. Connect up event sources.`, - - // Disable docs header - DisableAutoGenTag: true, - - // Affects children as well - SilenceUsage: true, - - // Prevents Cobra from dealing with errors as we deal with them in main.go - SilenceErrors: true, - - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - err := initConfigFlags() - if err != nil { - return err - } - return flags.ReconcileBoolFlags(cmd.Flags()) - }, - } - if p.Output != nil { - rootCmd.SetOutput(p.Output) - } - - // Persistent flags - rootCmd.PersistentFlags().StringVar(&commands.CfgFile, "config", "", "kn config file (default is "+ - filepath.Join(commands.Cfg.DefaultConfigDir, "config.yaml")+")") - rootCmd.PersistentFlags().StringVar(&p.KubeCfgPath, "kubeconfig", "", "kubectl config file (default is ~/.kube/config)") - flags.AddBothBoolFlags(rootCmd.PersistentFlags(), &p.LogHTTP, "log-http", "", false, "log http traffic") - - plugin.AddPluginFlags(rootCmd) - plugin.BindPluginsFlagToViper(rootCmd) - - // root child commands - rootCmd.AddCommand(service.NewServiceCommand(p)) - rootCmd.AddCommand(revision.NewRevisionCommand(p)) - rootCmd.AddCommand(plugin.NewPluginCommand(p)) - rootCmd.AddCommand(route.NewRouteCommand(p)) - rootCmd.AddCommand(completion.NewCompletionCommand(p)) - rootCmd.AddCommand(version.NewVersionCommand(p)) - rootCmd.AddCommand(source.NewSourceCommand(p)) - rootCmd.AddCommand(trigger.NewTriggerCommand(p)) - - // Initialize default `help` cmd early to prevent unknown command errors - rootCmd.InitDefaultHelpCmd() - - // Deal with empty and unknown sub command groups - EmptyAndUnknownSubCommands(rootCmd) - - // Wrap usage. - w, err := width() - if err == nil { - newUsage := strings.ReplaceAll(rootCmd.UsageTemplate(), "FlagUsages ", - fmt.Sprintf("FlagUsagesWrapped %d ", w)) - rootCmd.SetUsageTemplate(newUsage) - } - - // For glog parse error. - flag.CommandLine.Parse([]string{}) - - // Set all current core commands to plugin.CoreCommandNames - for _, cmd := range rootCmd.Commands() { - plugin.CoreCommandNames = append(plugin.CoreCommandNames, cmd.Name()) - } - - return rootCmd -} - -// InitializeConfig initializes the kubeconfig used by all commands -func InitializeConfig() { - cobra.OnInitialize(initConfig) -} - -// EmptyAndUnknownSubCommands adds a RunE to all commands that are groups to -// deal with errors when called with empty or unknown sub command -func EmptyAndUnknownSubCommands(cmd *cobra.Command) { - for _, childCmd := range cmd.Commands() { - if childCmd.HasSubCommands() && childCmd.RunE == nil { - childCmd.RunE = func(aCmd *cobra.Command, args []string) error { - aCmd.Help() - if len(args) == 0 { - return fmt.Errorf("please provide a valid sub-command for \"kn %s\"", aCmd.Name()) - } - return fmt.Errorf("unknown sub-command \"%s\" for \"kn %s\"", args[0], aCmd.Name()) - } - } - - // recurse to deal with child commands that are themselves command groups - EmptyAndUnknownSubCommands(childCmd) - } -} - -// Private - -// initConfig reads in config file and ENV variables if set. -func initConfig() { - if commands.CfgFile != "" { - // Use config file from the flag. - viper.SetConfigFile(commands.CfgFile) - } else { - configDir, err := defaultConfigDir() - if err != nil { - // Deprecated path warning message & continue - fmt.Fprintf(os.Stderr, "\n%v\n\n", err) - } - // Search config in home directory with name ".kn" (without extension) - viper.AddConfigPath(configDir) - viper.SetConfigName("config") - } - - viper.AutomaticEnv() // read in environment variables that match - - // If a config file is found, read it in. - err := viper.ReadInConfig() - if err == nil { - fmt.Fprintln(os.Stderr, "Using kn config file:", viper.ConfigFileUsed()) - } -} - -func defaultConfigDir() (string, error) { - home, err := homedir.Dir() - if err != nil { - return "", fmt.Errorf("%v", err) - } - // Check the deprecated path first and fallback to it, add warning to error message - if configHome := filepath.Join(home, ".kn"); dirExists(configHome) { - migrationPath := filepath.Join(home, ".config", "kn") - if runtime.GOOS == "windows" { - migrationPath = filepath.Join(os.Getenv("APPDATA"), "kn") - } - return configHome, fmt.Errorf("WARNING: deprecated kn config directory detected. "+ - "Please move your configuration to: %s", migrationPath) - } - // Respect %APPDATA% on MS Windows - // C:\Documents and Settings\username\Application JsonData - if runtime.GOOS == "windows" { - return filepath.Join(os.Getenv("APPDATA"), "kn"), nil - } - // Respect XDG_CONFIG_HOME if set - if xdgHome := os.Getenv("XDG_CONFIG_HOME"); xdgHome != "" { - return filepath.Join(xdgHome, "kn"), nil - } - // Fallback to XDG default for both Linux and macOS - // ~/.config/kn - return filepath.Join(home, ".config", "kn"), nil -} - -func dirExists(path string) bool { - if _, err := os.Stat(path); !os.IsNotExist(err) { - return true - } - return false -} - -func initConfigFlags() error { - if viper.IsSet("plugins-dir") { - commands.Cfg.PluginsDir = viper.GetString("plugins-dir") - } - - // Always set the Cfg.LookupPlugins from viper value since default is false both ways - var aBool bool - aBool = viper.GetBool("lookup-plugins") - commands.Cfg.LookupPlugins = &aBool - - // set the Cfg.SinkPrefixes from viper if sink is configured - if viper.IsSet("sink") { - err := viper.UnmarshalKey("sink", &commands.Cfg.SinkPrefixes) - if err != nil { - return fmt.Errorf("unable to parse sink prefixes configuration in file %s because of %v", - viper.ConfigFileUsed(), err) - } - cmdflags.ConfigSinkPrefixes(commands.Cfg.SinkPrefixes) - } - - return nil -} - -func extractKnPluginFlags(args []string) (string, bool, error) { - // Deprecated default path, fallback to it when exist - home, _ := homedir.Dir() - pluginsDir := filepath.Join(home, ".kn", "plugins") - if !dirExists(pluginsDir) { - configDir, _ := defaultConfigDir() - pluginsDir = filepath.Join(configDir, "plugins") - } - - lookupPluginsInPath := false - - dirFlag := "--plugins-dir" - pathFlag := "--lookup-plugins" - var err error - - for _, arg := range args { - if arg == dirFlag { - // They forgot the =... - return "", false, fmt.Errorf("Missing %s flag value", dirFlag) - } else if strings.HasPrefix(arg, dirFlag+"=") { - // Starts with --plugins-dir= so we parse the value - pluginsDir = arg[len(dirFlag)+1:] - if pluginsDir == "" { - // They have a "=" but nothing afer it - return "", false, fmt.Errorf("Missing %s flag value", dirFlag) - } - } - - if arg == pathFlag { - // just --lookup-plugins no "=" - lookupPluginsInPath = true - } else if strings.HasPrefix(arg, pathFlag+"=") { - // Starts with --lookup-plugins= so we parse value - arg = arg[len(pathFlag)+1:] - if lookupPluginsInPath, err = strconv.ParseBool(arg); err != nil { - return "", false, fmt.Errorf("Invalid boolean value(%q) for %s flag", arg, dirFlag) - } - } - } - return pluginsDir, lookupPluginsInPath, nil -} - -func removeKnPluginFlags(args []string) []string { - var remainingArgs []string - - // Remove these two flags from the list of args. Even though some of - // of these cases should have resulted in an error, if for some reason - // we got here just remove them anyway. - for _, arg := range args { - if arg == "--plugins-dir" || - strings.HasPrefix(arg, "--plugins-dir=") || - arg == "--lookup-plugins" || - strings.HasPrefix(arg, "--lookup-plugins=") || - // remove -test.* args which are added when running go test - strings.HasPrefix(arg, "-test.") { - continue - } else { - remainingArgs = append(remainingArgs, arg) - } - } - - return remainingArgs -} - -func width() (int, error) { - width, _, err := terminal.GetSize(int(os.Stdout.Fd())) - return width, err -} - -func getCommands(args []string, innerArg string) string { - commands := []string{"kn"} - for _, arg := range args { - if arg == innerArg { - return strings.Join(commands, " ") - } - commands = append(commands, arg) - } - return "" -} - -func showSubcommands(cmd *cobra.Command, args []string, innerArg string) string { - var strs []string - for _, subcmd := range cmd.Commands() { - strs = append(strs, subcmd.Name()) - } - 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 { - for _, arg := range args { - if arg == "-h" || arg == "--help" { - return true - } - } - return false -} diff --git a/pkg/kn/core/root_test.go b/pkg/kn/core/root_test.go deleted file mode 100644 index 51eb29defc..0000000000 --- a/pkg/kn/core/root_test.go +++ /dev/null @@ -1,242 +0,0 @@ -// Copyright © 2019 The Knative 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 core - -import ( - "io/ioutil" - "os" - "strings" - "testing" - - "github.com/spf13/cobra" - "gotest.tools/assert" - "knative.dev/client/pkg/kn/commands" - "knative.dev/client/pkg/kn/commands/plugin" -) - -func TestNewDefaultKnCommand(t *testing.T) { - var rootCmd *cobra.Command - - setup := func(t *testing.T) { - rootCmd, _ = NewDefaultKnCommand() - } - - t.Run("returns a valid root command", func(t *testing.T) { - setup(t) - - checkRootCmd(t, rootCmd) - }) -} - -func TestNewDefaultKnCommandWithArgs(t *testing.T) { - var ( - rootCmd *cobra.Command - pluginHandler plugin.PluginHandler - args []string - ) - - setup := func(t *testing.T) { - rootCmd, _ = NewDefaultKnCommandWithArgs(NewKnCommand(), pluginHandler, args, os.Stdin, os.Stdout, os.Stderr) - } - - t.Run("when pluginHandler is nil", func(t *testing.T) { - args = []string{} - setup(t) - - t.Run("returns a valid root command", func(t *testing.T) { - checkRootCmd(t, rootCmd) - }) - }) - - t.Run("when pluginHandler is not nil", func(t *testing.T) { - t.Run("when args empty", func(t *testing.T) { - args = []string{} - setup(t) - - t.Run("returns a valid root command", func(t *testing.T) { - checkRootCmd(t, rootCmd) - }) - }) - - t.Run("when args not empty", func(t *testing.T) { - var ( - pluginName, pluginPath, tmpPathDir string - err error - ) - - pluginName = "fake-plugin-name" - - beforeEach := func(t *testing.T) { - tmpPathDir, err = ioutil.TempDir("", "plugin_list") - assert.Assert(t, err == nil) - - pluginPath = plugin.CreateTestPluginInPath(t, "kn-"+pluginName, plugin.KnTestPluginScript, plugin.FileModeExecutable, tmpPathDir) - } - - afterEach := func(t *testing.T) { - err = os.RemoveAll(tmpPathDir) - assert.Assert(t, err == nil) - } - - t.Run("when -h or --help option is present for plugin, return valid root command", func(t *testing.T) { - helpOptions := []string{"-h", "--help"} - for _, helpOption := range helpOptions { - beforeEach(t) - args = []string{pluginPath, pluginName, helpOption} - setup(t) - defer afterEach(t) - - checkRootCmd(t, rootCmd) - } - }) - - t.Run("when --help option is present for normal command, return valid root command", func(t *testing.T) { - helpOptions := []string{"-h", "--help"} - for _, helpOption := range helpOptions { - beforeEach(t) - args = []string{"service", helpOption} - setup(t) - defer afterEach(t) - - checkRootCmd(t, rootCmd) - } - }) - - t.Run("tries to handle args[1:] as plugin and return valid root command", func(t *testing.T) { - beforeEach(t) - args = []string{pluginPath, pluginName} - setup(t) - defer afterEach(t) - - checkRootCmd(t, rootCmd) - }) - - t.Run("when plugin extends an existing command group it return valid root command", func(t *testing.T) { - pluginName = "service-fakecmd" - beforeEach(t) - args = []string{pluginPath, pluginName} - setup(t) - defer afterEach(t) - - checkRootCmd(t, rootCmd) - }) - - t.Run("when plugin extends and shadows an existing command group it fails", func(t *testing.T) { - pluginName = "service-create" - beforeEach(t) - args = []string{pluginPath, pluginName, "test"} - setup(t) - defer afterEach(t) - - checkRootCmd(t, rootCmd) - }) - }) - }) -} - -func TestNewKnCommand(t *testing.T) { - var rootCmd *cobra.Command - - setup := func(t *testing.T) { - rootCmd = NewKnCommand(commands.KnParams{}) - } - - t.Run("returns a valid root command", func(t *testing.T) { - setup(t) - checkRootCmd(t, rootCmd) - }) - - t.Run("sets the output params", func(t *testing.T) { - setup(t) - assert.Assert(t, rootCmd.OutOrStdout() != nil) - }) - - t.Run("sets the config and kubeconfig global flags", func(t *testing.T) { - setup(t) - assert.Assert(t, rootCmd.PersistentFlags().Lookup("config") != nil) - assert.Assert(t, rootCmd.PersistentFlags().Lookup("kubeconfig") != nil) - }) - - t.Run("adds the top level commands: version and completion", func(t *testing.T) { - setup(t) - checkCommand(t, "version", rootCmd) - checkCommand(t, "completion", rootCmd) - }) - - t.Run("adds the top level group commands", func(t *testing.T) { - setup(t) - checkCommandGroup(t, "service", rootCmd) - checkCommandGroup(t, "revision", rootCmd) - }) -} - -func TestEmptyAndUnknownSubCommands(t *testing.T) { - var rootCmd, fakeCmd, fakeSubCmd *cobra.Command - - setup := func(t *testing.T) { - rootCmd = NewKnCommand(commands.KnParams{}) - fakeCmd = &cobra.Command{ - Use: "fake-cmd-name", - } - fakeSubCmd = &cobra.Command{ - Use: "fake-sub-cmd-name", - } - fakeCmd.AddCommand(fakeSubCmd) - rootCmd.AddCommand(fakeCmd) - - assert.Assert(t, fakeCmd.RunE == nil) - assert.Assert(t, fakeSubCmd.RunE == nil) - } - - t.Run("deals with empty and unknown sub-commands for all group commands", func(t *testing.T) { - setup(t) - EmptyAndUnknownSubCommands(rootCmd) - checkCommand(t, "fake-sub-cmd-name", fakeCmd) - checkCommandGroup(t, "fake-cmd-name", rootCmd) - }) -} - -// Private - -func checkRootCmd(t *testing.T, rootCmd *cobra.Command) { - assert.Assert(t, rootCmd != nil) - - assert.Equal(t, rootCmd.Name(), "kn") - assert.Equal(t, rootCmd.Short, "Knative client") - assert.Assert(t, strings.Contains(rootCmd.Long, "Manage your Knative building blocks:")) - - assert.Assert(t, rootCmd.DisableAutoGenTag) - assert.Assert(t, rootCmd.SilenceUsage) - assert.Assert(t, rootCmd.SilenceErrors) - - assert.Assert(t, rootCmd.Flags().Lookup("plugins-dir") != nil) - assert.Assert(t, rootCmd.Flags().Lookup("lookup-plugins") != nil) - - assert.Assert(t, rootCmd.RunE == nil) -} - -func checkCommand(t *testing.T, name string, rootCmd *cobra.Command) { - cmd, _, err := rootCmd.Find([]string{"version"}) - assert.Assert(t, err == nil) - assert.Assert(t, cmd != nil) -} - -func checkCommandGroup(t *testing.T, name string, rootCmd *cobra.Command) { - cmd, _, err := rootCmd.Find([]string{name}) - assert.Assert(t, err == nil) - assert.Assert(t, cmd != nil) - assert.Assert(t, cmd.RunE != nil) - assert.Assert(t, cmd.HasSubCommands()) -} diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go new file mode 100644 index 0000000000..0f3b49dcbe --- /dev/null +++ b/pkg/kn/plugin/manager.go @@ -0,0 +1,350 @@ +// Copyright © 2019 The Knative 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 plugin + +import ( + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "runtime" + "sort" + "strings" + + homedir "github.com/mitchellh/go-homedir" + "github.com/pkg/errors" +) + +// Interface describing a plugin +type Plugin interface { + // Get the name of the plugin (the file name without extensions) + Name() string + + // Execute the plugin with the given arguments + Execute(args []string) error + + // Return a description of the plugin (if support by the plugin binary) + Description() (string, error) + + // The command path leading to this plugin. + // Eg. for a plugin "kn source github" this will be [ "source", "github" ] + CommandParts() []string + + // Location of the plugin where it is stored in the filesystem + Path() string +} + +type Manager struct { + // Dedicated plugin directory as configured + pluginsDir string + + // Whether to check the OS path or not + lookupInPath bool +} + +type plugin struct { + // Path to the plugin to execute + path string + + // Name of the plugin + name string + + // Commands leading to the execution of this plugin (e.g. "service","log" for a plugin kn-service-log) + commandParts []string +} + +// All extensions that are supposed to be windows executable +var windowsExecExtensions = []string{".bat", ".cmd", ".com", ".exe", ".ps1"} + +// Used for sorting a list of plugins +type PluginList []Plugin + +func (p PluginList) Len() int { return len(p) } +func (p PluginList) Less(i, j int) bool { return p[i].Name() < p[j].Name() } +func (p PluginList) Swap(i, j int) { p[i], p[j] = p[j], p[i] } + +// === PluginManager ======================================================================= + +// NewManager creates a new manager for looking up plugins on the file system +func NewManager(pluginDir string, lookupInPath bool) *Manager { + return &Manager{ + pluginsDir: pluginDir, + lookupInPath: lookupInPath, + } +} + +// FindPlugin checks if a plugin for the given parts exist and return it. +// The args given must not contain any options and contain only +// the comands (like in [ "source", "github" ] for a plugin called 'kn-source-github' +// The plugin with the most specific (longest) name is returned or nil if non is found. +// An error is returned if the lookup fails for some reason like an io error +func (manager *Manager) FindPlugin(parts []string) (Plugin, error) { + if len(parts) == 0 { + // No command given + return nil, nil + } + + // Try to find plugin in pluginsDir + pluginDir, err := homedir.Expand(manager.pluginsDir) + if err != nil { + return nil, err + } + + return findMostSpecificPlugin(pluginDir, parts, manager.lookupInPath) +} + +// ListPlugins lists all plugins that can be found in the plugin directory or in the path (if configured) +func (manager *Manager) ListPlugins() (PluginList, error) { + var plugins []Plugin + + dirs, err := manager.pluginLookupDirectories() + if err != nil { + return nil, err + } + + // Examine all files in possible plugin directories + hasSeen := make(map[string]bool) + for _, dir := range dirs { + files, err := ioutil.ReadDir(dir) + + // Ignore non-existing directories + if os.IsNotExist(err) { + continue + } + + // Check for plugins within given directory + for _, f := range files { + name := f.Name() + if f.IsDir() { + continue + } + if !strings.HasPrefix(name, "kn-") { + continue + } + + // Ignore all plugins that are shadowed + if _, ok := hasSeen[name]; !ok { + plugins = append(plugins, &plugin{ + path: filepath.Join(dir, f.Name()), + name: stripWindowsExecExtensions(f.Name()), + commandParts: extractPluginCommandFromFileName(f.Name()), + }) + hasSeen[name] = true + } + } + } + // Sort according to name + sort.Sort(PluginList(plugins)) + return plugins, nil +} + +// PluginsDir returns the configured directory holding plugins +func (manager *Manager) PluginsDir() string { + return manager.pluginsDir +} + +// LookupInPath returns true if plugins should be also looked up within the path +func (manager *Manager) LookupInPath() bool { + return manager.lookupInPath +} + +// === Plugin ============================================================================== + +// Execute the plugin with the given arguments +func (plugin *plugin) Execute(args []string) error { + cmd := exec.Command(plugin.path, args...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Stdin = os.Stdin + cmd.Env = os.Environ() + return cmd.Run() +} + +// 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 +} + +// The the command path leading to this plugin. +// Eg. for a plugin "kn source github" this will be [ "source", "github" ] +func (plugin *plugin) CommandParts() []string { + return plugin.commandParts +} + +// Return the path to the plugin +func (plugin *plugin) Path() string { + return plugin.path +} + +// Name of the plugin +func (plugin *plugin) Name() string { + return plugin.name +} + +// ========================================================================================= + +// Find out all directories that might hold a plugin +func (manager *Manager) pluginLookupDirectories() ([]string, error) { + pluginPath, err := homedir.Expand(manager.pluginsDir) + if err != nil { + return nil, err + } + dirs := []string{pluginPath} + if manager.lookupInPath { + dirs = append(dirs, filepath.SplitList(os.Getenv("PATH"))...) + } + dirs = uniquePathsList(dirs) + return dirs, nil +} + +// uniquePathsList deduplicates a given slice of strings without +// sorting or otherwise altering its order in any way. +func uniquePathsList(paths []string) []string { + seen := map[string]bool{} + var newPaths []string + for _, p := range paths { + if seen[p] { + continue + } + seen[p] = true + newPaths = append(newPaths, p) + } + return newPaths +} + +// Split up a command name, discard the initial prefix ("kn-") and convert +// parts to command syntax (i.e. replace _ with -) +func extractPluginCommandFromFileName(name string) []string { + // Remove extension on windows + name = stripWindowsExecExtensions(name) + parts := strings.Split(name, "-") + if len(parts) < 1 { + return []string{} + } + var ret []string + for _, p := range parts[1:] { + ret = append(ret, convertUnderscoreToDash(p)) + } + return ret +} + +// Strip any extension that indicates an EXE on Windows +func stripWindowsExecExtensions(name string) string { + if runtime.GOOS == "windows" { + ext := filepath.Ext(name) + if len(ext) > 0 { + for _, e := range windowsExecExtensions { + if ext == e { + name = name[:len(name)-len(ext)] + break + } + } + } + } + return name +} + +// Return the path and the parts building the most specific plugin in the given directory +// If lookupInPath is true, then also the OS PATH is checked. +// An error returned if any IO operation fails +func findMostSpecificPlugin(dir string, parts []string, lookupInPath bool) (Plugin, error) { + for i := len(parts); i > 0; i-- { + + // Construct plugin name to lookup + var nameParts []string + var commandParts []string + for _, p := range parts[0:i] { + // Subcommands with "-" are translated to "_" + // (e.g. a command "kn log-all" is translated to a plugin "kn-log_all") + nameParts = append(nameParts, convertDashToUnderscore(p)) + commandParts = append(commandParts, p) + } + name := fmt.Sprintf("kn-%s", strings.Join(nameParts, "-")) + + // Check for the name in plugin directory and PATH (if requested) + path, err := findInDirOrPath(name, dir, lookupInPath) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("cannot lookup plugin %s in directory %s (lookup in path: %t)", name, dir, lookupInPath)) + } + + // Found, return it + if path != "" { + return &plugin{ + path: path, + commandParts: commandParts, + name: name, + }, nil + } + } + + // Nothing found + return nil, nil +} + +// convertDashToUnderscore converts from the command name to the file name +func convertDashToUnderscore(p string) string { + return strings.Replace(p, "-", "_", -1) +} + +// convertUnderscoreToDash converts from the filename to the command name +func convertUnderscoreToDash(p string) string { + return strings.Replace(p, "_", "-", -1) +} + +// Find a command with name in the given directory or on the execution PATH (if lookupInPath is true) +// On Windows, also check well known extensions for executables +// Return the full path found or "" if none has found +// Return an error on any IO error. +func findInDirOrPath(name string, dir string, lookupInPath bool) (string, error) { + + exts := []string{""} + if runtime.GOOS == "windows" { + // Add also well known extensions for windows + exts = append(exts, windowsExecExtensions...) + } + + for _, ext := range exts { + nameExt := name + ext + + // Check plugin dir first + path := filepath.Join(dir, nameExt) + _, err := os.Stat(path) + if err == nil { + // Found in dir + return path, nil + } + if !os.IsNotExist(err) { + return "", errors.Wrap(err, fmt.Sprintf("i/o error while reading %s", path)) + } + + // Check in PATH if requested + if lookupInPath { + path, err = exec.LookPath(name) + if err == nil { + // Found in path + return path, nil + } + if execErr, ok := err.(*exec.Error); !ok || execErr.Unwrap() != exec.ErrNotFound { + return "", errors.Wrap(err, fmt.Sprintf("error for looking up %s in path", name)) + } + } + } + + // Not found + return "", nil +} diff --git a/pkg/kn/plugin/manager_test.go b/pkg/kn/plugin/manager_test.go new file mode 100644 index 0000000000..16efba8f3b --- /dev/null +++ b/pkg/kn/plugin/manager_test.go @@ -0,0 +1,221 @@ +// Copyright © 2018 The Knative 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 plugin + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "runtime" + "testing" + + "gotest.tools/assert" +) + +var testPluginScriptUnix = `#!/bin/bash +echo "OK $*" +` +var testPluginScriptWindows = ` +print "OK" +` + +type testContext struct { + pluginsDir string + pluginManager *Manager +} + +func TestEmptyFind(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + + plugin, err := ctx.pluginManager.FindPlugin([]string{}) + assert.NilError(t, err) + assert.Equal(t, plugin, nil) + assert.Assert(t, ctx.pluginManager.PluginsDir() != "") + assert.Assert(t, !ctx.pluginManager.LookupInPath()) +} + +func TestLookupInPluginsDir(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + createTestPlugin(t, "kn-test", ctx) + + plugin, err := ctx.pluginManager.FindPlugin([]string{"test"}) + assert.NilError(t, err) + assert.Assert(t, plugin != nil) + assert.Assert(t, plugin.CommandParts()[0] == "test") + + out, err := executePlugin(plugin, []string{}) + assert.NilError(t, err) + assert.Equal(t, out, "OK \n") +} + +func TestLookupWithNotFoundResult(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + + plugin, err := ctx.pluginManager.FindPlugin([]string{"bogus", "plugin", "name"}) + assert.Assert(t, plugin == nil, "no plugin should be found") + assert.NilError(t, err, "no error expected") +} + +func TestPluginInPath(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + + // Prepare PATH + tmpPathDir, cleanupFunc := preparePathDirectory(t) + defer cleanupFunc() + + createTestPluginInDirectory(t, "kn-path-test", tmpPathDir) + pluginCommands := []string{"path", "test"} + + // Enable lookup --> find plugin + ctx.pluginManager.lookupInPath = true + plugin, err := ctx.pluginManager.FindPlugin(pluginCommands) + assert.NilError(t, err) + assert.Assert(t, plugin != nil) + assert.Equal(t, plugin.Path(), filepath.Join(tmpPathDir, "kn-path-test")) + assert.DeepEqual(t, plugin.CommandParts(), pluginCommands) + + // Disable lookup --> no plugin + ctx.pluginManager.lookupInPath = false + plugin, err = ctx.pluginManager.FindPlugin(pluginCommands) + assert.NilError(t, err) + assert.Assert(t, plugin == nil) +} + +func TestPluginExecute(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + createTestPlugin(t, "kn-test_with_dash-longer", ctx) + + plugin, err := ctx.pluginManager.FindPlugin([]string{"test-with-dash", "longer"}) + assert.NilError(t, err) + out, err := executePlugin(plugin, []string{"arg1", "arg2"}) + assert.NilError(t, err) + assert.Equal(t, out, "OK arg1 arg2\n") +} + +func TestPluginList(t *testing.T) { + ctx := setup(t) + defer cleanup(t, ctx) + + // Plugin in plugin's dr + createTestPlugin(t, "kn-zz-test_in_dir", ctx) + + // Plugin in Path + tmpPathDir, cleanupFunc := preparePathDirectory(t) + defer cleanupFunc() + createTestPluginInDirectory(t, "kn-aa-path-test", tmpPathDir) + + // Enable lookup --> Both plugins found + ctx.pluginManager.lookupInPath = true + pluginList, err := ctx.pluginManager.ListPlugins() + assert.NilError(t, err) + assert.Assert(t, pluginList != nil) + assert.Equal(t, len(pluginList), 2, "both plugins found (in dir + in path)") + assert.Equal(t, pluginList[0].Name(), "kn-aa-path-test", "first plugin is alphabetically smallest (list is sorted)") + assert.DeepEqual(t, pluginList[0].CommandParts(), []string{"aa", "path", "test"}) + assert.Equal(t, pluginList[1].Name(), "kn-zz-test_in_dir", "second plugin is alphabetically greater (list is sorted)") + assert.DeepEqual(t, pluginList[1].CommandParts(), []string{"zz", "test-in-dir"}) + + // Disable lookup --> Only one plugin found + ctx.pluginManager.lookupInPath = false + pluginList, err = ctx.pluginManager.ListPlugins() + assert.NilError(t, err) + assert.Assert(t, pluginList != nil) + assert.Equal(t, len(pluginList), 1, "1 plugin found (in dir)") + assert.Equal(t, pluginList[0].Name(), "kn-zz-test_in_dir", "second plugin is alphabetically greater (list is sorted)") + assert.DeepEqual(t, pluginList[0].CommandParts(), []string{"zz", "test-in-dir"}) +} + +// ==================================================================== +// Private + +func setup(t *testing.T) testContext { + return setupWithPathLookup(t, false) +} + +func setupWithPathLookup(t *testing.T, lookupInPath bool) testContext { + tmpPathDir, err := ioutil.TempDir("", "plugin_list") + assert.NilError(t, err) + return testContext{ + pluginsDir: tmpPathDir, + pluginManager: NewManager(tmpPathDir, lookupInPath), + } +} + +func cleanup(t *testing.T, ctx testContext) { + err := os.RemoveAll(ctx.pluginsDir) + assert.NilError(t, err) +} + +func executePlugin(plugin Plugin, args []string) (string, error) { + rescueStdout := os.Stdout + defer (func() { os.Stdout = rescueStdout })() + + r, w, _ := os.Pipe() + os.Stdout = w + + err := plugin.Execute(args) + w.Close() + if err != nil { + return "", err + } + out, _ := ioutil.ReadAll(r) + return string(out), nil +} + +// Prepare a directory and set the path to this directory +func preparePathDirectory(t *testing.T) (string, func()) { + tmpPathDir, err := ioutil.TempDir("", "plugin_path") + assert.NilError(t, err) + + oldPath := os.Getenv("PATH") + os.Setenv("PATH", fmt.Sprintf("%s%c%s", tmpPathDir, os.PathListSeparator, "fast-forward-this-year-plz")) + return tmpPathDir, func() { + os.RemoveAll(tmpPathDir) + os.Setenv("PATH", oldPath) + } +} + +// CreateTestPlugin with name, script, and fileMode and return the tmp random path +func createTestPlugin(t *testing.T, name string, ctx testContext) string { + return createTestPluginInDirectory(t, name, ctx.pluginsDir) +} + +// CreateTestPluginInPath with name, path, script, and fileMode and return the tmp random path +func createTestPluginInDirectory(t *testing.T, name string, dir string) string { + var nameExt, script string + if runtime.GOOS == "windows" { + nameExt = name + ".bat" + script = testPluginScriptWindows + } else { + nameExt = name + script = testPluginScriptUnix + } + fullPath := filepath.Join(dir, nameExt) + err := ioutil.WriteFile(fullPath, []byte(script), 0777) + assert.NilError(t, err) + // Some extra files to feed the tests + err = ioutil.WriteFile(filepath.Join(dir, "non-plugin-prefix-"+nameExt), []byte{}, 0555) + assert.NilError(t, err) + _, err = ioutil.TempDir(dir, "bogus-dir") + assert.NilError(t, err) + + return fullPath +} diff --git a/pkg/kn/commands/plugin/list_flags.go b/pkg/kn/plugin/stat.go similarity index 52% rename from pkg/kn/commands/plugin/list_flags.go rename to pkg/kn/plugin/stat.go index 8e219b1e04..8a329e7c81 100644 --- a/pkg/kn/commands/plugin/list_flags.go +++ b/pkg/kn/plugin/stat.go @@ -1,4 +1,4 @@ -// Copyright © 2018 The Knative Authors +// Copyright © 2019 The Knative Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,20 +12,24 @@ // See the License for the specific language governing permissions and // limitations under the License. +// +build !windows + package plugin +// This file doesn't compile for Windows platform, therefor a second stat_windows.go is +// added with a no-op + import ( - "github.com/spf13/cobra" + "fmt" + "os" + "syscall" ) -// pluginListFlags contains all plugin commands flags -type pluginListFlags struct { - nameOnly bool - verbose bool -} - -// AddPluginFlags adds the various flags to plugin command -func (p *pluginListFlags) AddPluginListFlags(command *cobra.Command) { - command.Flags().BoolVar(&p.nameOnly, "name-only", false, "If true, display only the binary name of each plugin, rather than its full path") - command.Flags().BoolVar(&p.verbose, "verbose", false, "verbose output") +func statFileOwner(fileInfo os.FileInfo) (uint32, uint32, error) { + var sys *syscall.Stat_t + var ok bool + if sys, ok = fileInfo.Sys().(*syscall.Stat_t); !ok { + return 0, 0, fmt.Errorf("cannot check owner/group of file %s", fileInfo.Name()) + } + return sys.Uid, sys.Gid, nil } diff --git a/pkg/kn/plugin/stat_windows.go b/pkg/kn/plugin/stat_windows.go new file mode 100644 index 0000000000..8a0af80f94 --- /dev/null +++ b/pkg/kn/plugin/stat_windows.go @@ -0,0 +1,24 @@ +// Copyright © 2019 The Knative 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 plugin + +import ( + "os" +) + +// statFileOwner is a no-op on windows +func statFileOwner(fileInfo os.FileInfo) (uint32, uint32, error) { + return 0, 0, nil +} diff --git a/pkg/kn/plugin/verify.go b/pkg/kn/plugin/verify.go new file mode 100644 index 0000000000..6237a495d5 --- /dev/null +++ b/pkg/kn/plugin/verify.go @@ -0,0 +1,259 @@ +// Copyright © 2019 The Knative 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 plugin + +import ( + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" + "runtime" + "strings" +) + +// Collection of errors and warning collected during verifications +type VerificationErrorsAndWarnings struct { + Errors []string + Warnings []string +} + +// permission bits for execute +const ( + UserExecute = 1 << 6 + GroupExecute = 1 << 3 + OtherExecute = 1 << 0 +) + +// Verification of a ll plugins. This method returns all errors and warnings +// for the verification. The following criteria are verified (for each plugin): +// * If the plugin is executable +// * If the plugin is overshadowed by a previous plugin +func (manager *Manager) Verify() VerificationErrorsAndWarnings { + eaw := VerificationErrorsAndWarnings{} + + dirs, err := manager.pluginLookupDirectories() + if err != nil { + return eaw.AddError("cannot lookup plugin directories: %v", err) + } + + // Examine all files in possible plugin directories + + seenPlugins := make(map[string]string) + for _, dir := range dirs { + files, err := ioutil.ReadDir(dir) + + // Ignore non-existing directories + if os.IsNotExist(err) { + continue + } + + if err != nil { + eaw.AddError("unable to read directory '%s' from your plugin path: %v", dir, err) + continue + } + + for _, f := range files { + if f.IsDir() { + continue + } + if !strings.HasPrefix(f.Name(), "kn-") { + continue + } + eaw = verifyPath(filepath.Join(dir, f.Name()), seenPlugins, eaw) + } + } + return eaw +} + +func verifyPath(path string, seenPlugins map[string]string, eaw VerificationErrorsAndWarnings) VerificationErrorsAndWarnings { + + // Verify that plugin actually exists + fileInfo, err := os.Stat(path) + if err != nil { + if err == os.ErrNotExist { + return eaw.AddError("cannot find plugin in %s", path) + } + return eaw.AddError("cannot stat %s: %v", path, err) + } + + eaw = addWarningIfNotExecutable(eaw, path, fileInfo) + eaw = addWarningIfAlreadySeen(eaw, seenPlugins, path) + + // Remember each verified plugin for duplicate check + seenPlugins[filepath.Base(path)] = path + + return eaw +} + +func addWarningIfNotExecutable(eaw VerificationErrorsAndWarnings, path string, fileInfo os.FileInfo) VerificationErrorsAndWarnings { + if runtime.GOOS == "windows" { + return checkForWindowsExecutable(eaw, fileInfo, path) + } + + mode := fileInfo.Mode() + if !mode.IsRegular() && !isSymlink(mode) { + return eaw.AddWarning("%s is not a file", path) + } + perms := uint32(mode.Perm()) + + uid, gid, err := statFileOwner(fileInfo) + if err != nil { + return eaw.AddWarning("%s", err.Error()) + } + isOwner := checkIfUserIsFileOwner(uid) + isInGroup, err := checkIfUserInGroup(gid) + if err != nil { + return eaw.AddError("cannot get group ids for checking executable status of file %s", path) + } + + // User is owner and owner can execute + if canOwnerExecute(perms, isOwner) { + return eaw + } + + // User is in group which can execute, but user is not file owner + if canGroupExecute(perms, isOwner, isInGroup) { + return eaw + } + + // All can execute, and the user is not file owner and not in the file's perm group + if canOtherExecute(perms, isOwner, isInGroup) { + return eaw + } + + return eaw.AddWarning("%s is not executable by current user", path) +} + +func addWarningIfAlreadySeen(eaw VerificationErrorsAndWarnings, seenPlugins map[string]string, path string) VerificationErrorsAndWarnings { + fileName := filepath.Base(path) + if existingPath, ok := seenPlugins[fileName]; ok { + return eaw.AddWarning("%s is ignored because it is shadowed by an equally named plugin: %s", path, existingPath) + } + return eaw +} + +func checkForWindowsExecutable(eaw VerificationErrorsAndWarnings, fileInfo os.FileInfo, path string) VerificationErrorsAndWarnings { + name := fileInfo.Name() + nameWithoutExecExtension := stripWindowsExecExtensions(name) + + if name == nameWithoutExecExtension { + return eaw.AddWarning("%s is not executable as it does not have a Windows exec extension (one of %s)", path, strings.Join(windowsExecExtensions, ", ")) + } + return eaw +} + +func checkIfUserInGroup(gid uint32) (bool, error) { + groups, err := os.Getgroups() + if err != nil { + return false, err + } + for _, g := range groups { + if int(gid) == g { + return true, nil + } + } + return false, nil +} + +func checkIfUserIsFileOwner(uid uint32) bool { + if int(uid) == os.Getuid() { + return true + } + return false +} + +// Check if all can execute, and the user is not file owner and not in the file's perm group +func canOtherExecute(perms uint32, isOwner bool, isInGroup bool) bool { + if perms&OtherExecute != 0 { + if os.Getuid() == 0 { + return true + } + if !isOwner && !isInGroup { + return true + } + } + return false +} + +// Check if user is owner and owner can execute +func canOwnerExecute(perms uint32, isOwner bool) bool { + if perms&UserExecute != 0 { + if os.Getuid() == 0 { + return true + } + if isOwner { + return true + } + } + return false +} + +// Check if user is in group which can execute, but user is not file owner +func canGroupExecute(perms uint32, isOwner bool, isInGroup bool) bool { + if perms&GroupExecute != 0 { + if os.Getuid() == 0 { + return true + } + if !isOwner && isInGroup { + return true + } + } + return false +} + +func (eaw *VerificationErrorsAndWarnings) AddError(format string, args ...interface{}) VerificationErrorsAndWarnings { + eaw.Errors = append(eaw.Errors, fmt.Sprintf(format, args...)) + return *eaw +} + +func (eaw *VerificationErrorsAndWarnings) AddWarning(format string, args ...interface{}) VerificationErrorsAndWarnings { + eaw.Warnings = append(eaw.Warnings, fmt.Sprintf(format, args...)) + return *eaw +} + +func (eaw *VerificationErrorsAndWarnings) PrintWarningsAndErrors(out io.Writer) { + printSection(out, "ERROR", eaw.Errors) + printSection(out, "WARNING", eaw.Warnings) +} + +func (eaw *VerificationErrorsAndWarnings) HasErrors() bool { + return len(eaw.Errors) > 0 +} + +func (eaw *VerificationErrorsAndWarnings) IsEmpty() bool { + return len(eaw.Errors)+len(eaw.Warnings) == 0 +} + +func printSection(out io.Writer, label string, values []string) { + if len(values) > 0 { + printLabelWithConditionalPluralS(out, label, len(values)) + for _, value := range values { + fmt.Fprintf(out, " - %s\n", value) + } + } +} + +func printLabelWithConditionalPluralS(out io.Writer, label string, nr int) { + if nr == 1 { + fmt.Fprintf(out, "%s:\n", label) + } else { + fmt.Fprintf(out, "%ss:\n", label) + } +} + +func isSymlink(mode os.FileMode) bool { + return mode&os.ModeSymlink != 0 +} diff --git a/pkg/kn/plugin/verify_test.go b/pkg/kn/plugin/verify_test.go new file mode 100644 index 0000000000..ca9b9dc742 --- /dev/null +++ b/pkg/kn/plugin/verify_test.go @@ -0,0 +1,187 @@ +// Copyright © 2018 The Knative 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 plugin + +import ( + "bytes" + "errors" + "os" + "os/exec" + "os/user" + "runtime" + "strconv" + "testing" + + "knative.dev/client/pkg/util" + + "gotest.tools/assert" +) + +func TestPluginIsExecutableUnix(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skip test for windows as the permission check are for Unix only") + return + } + + ctx := setup(t) + defer cleanup(t, ctx) + + pluginPath := createTestPlugin(t, "kn-test", ctx) + for _, uid := range getExecTestUids() { + for _, gid := range getExecTestGids() { + for _, userPerm := range []int{0, UserExecute} { + for _, groupPerm := range []int{0, GroupExecute} { + for _, otherPerm := range []int{0, OtherExecute} { + perm := os.FileMode(userPerm | groupPerm | otherPerm + 0444) + assert.NilError(t, prepareFile(pluginPath, uid, gid, perm), "prepare plugin file, uid: %d, gid: %d, perm: %03o", uid, gid, perm) + + eaw := ctx.pluginManager.Verify() + + if isExecutable(pluginPath) { + assert.Assert(t, len(eaw.Warnings) == 0, "executable: perm %03o | uid %d | gid %d | %v", perm, uid, gid, eaw.Warnings) + } else { + assert.Assert(t, len(eaw.Warnings) == 1, "not executable: perm %03o | uid %d | gid %d | %v", perm, uid, gid, eaw.Warnings) + assert.Assert(t, util.ContainsAll(eaw.Warnings[0], pluginPath)) + } + assert.Assert(t, len(eaw.Errors) == 0) + } + } + } + } + } +} + +func TestPluginIsExecutableWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("Skip test for non-windows OS as this test checks for Windows Extensions") + return + } + + ctx := setup(t) + defer cleanup(t, ctx) + + pluginPath := createTestPlugin(t, "kn-test.bat", ctx) + eaw := ctx.pluginManager.Verify() + assert.Equal(t, len(eaw.Warnings), 0) + assert.Equal(t, len(eaw.Errors), 0) + os.Remove(pluginPath) + + pluginPath = createTestPlugin(t, "kn-test", ctx) + eaw = ctx.pluginManager.Verify() + assert.Equal(t, len(eaw.Warnings), 1) + assert.Assert(t, util.ContainsAll(eaw.Warnings[0], pluginPath)) + assert.Equal(t, len(eaw.Errors), 0) +} + +func TestWarnIfPluginShadowsOtherPlugin(t *testing.T) { + ctx := setupWithPathLookup(t, true) + defer cleanup(t, ctx) + + pl1path := createTestPlugin(t, "kn-test", ctx) + pathDir, cleanupFunc := preparePathDirectory(t) + defer cleanupFunc() + pl2path := createTestPluginInDirectory(t, "kn-test", pathDir) + + eaw := ctx.pluginManager.Verify() + assert.Assert(t, !eaw.IsEmpty()) + assert.Equal(t, len(eaw.Errors), 0) + assert.Equal(t, len(eaw.Warnings), 1) + assert.Assert(t, util.ContainsAll(eaw.Warnings[0], "shadowed", "ignored", pl1path, pl2path)) + + var buf bytes.Buffer + eaw.PrintWarningsAndErrors(&buf) + assert.Assert(t, util.ContainsAll(buf.String(), "WARNING", "shadowed", "ignored")) +} + +func isExecutable(plugin string) bool { + _, err := exec.Command(plugin).Output() + return err == nil +} + +func getExecTestUids() []int { + currentUser := os.Getuid() + // Only root can switch ownership of a file + if currentUser == 0 { + foreignUser, err := lookupForeignUser() + if err == nil { + return []int{currentUser, foreignUser} + } + } + return []int{currentUser} +} + +func getExecTestGids() []int { + currentUser := os.Getuid() + currentGroup := os.Getgid() + // Only root can switch group of a file + if currentUser == 0 { + foreignGroup, err := lookupForeignGroup() + if err == nil { + return []int{currentGroup, foreignGroup} + } + } + return []int{currentGroup} +} + +func lookupForeignUser() (int, error) { + for _, probe := range []string{"daemon", "nobody", "_unknown"} { + u, err := user.Lookup(probe) + if err != nil { + continue + } + uid, err := strconv.Atoi(u.Uid) + if err != nil { + continue + } + if uid != os.Getuid() { + return uid, nil + } + } + return 0, errors.New("could not find foreign user") +} + +func lookupForeignGroup() (int, error) { + gids, err := os.Getgroups() + if err != nil { + return 0, err + } +OUTER: + for _, probe := range []string{"daemon", "wheel", "nobody", "nogroup", "admin"} { + group, err := user.LookupGroup(probe) + if err != nil { + continue + } + gid, err := strconv.Atoi(group.Gid) + if err != nil { + continue + } + + for _, g := range gids { + if gid == g { + continue OUTER + } + } + return gid, nil + } + return 0, errors.New("could not find a foreign group") +} + +func prepareFile(file string, uid int, gid int, perm os.FileMode) error { + err := os.Chown(file, uid, gid) + if err != nil { + return err + } + return os.Chmod(file, perm) +} diff --git a/pkg/kn/root/root.go b/pkg/kn/root/root.go new file mode 100644 index 0000000000..d402571372 --- /dev/null +++ b/pkg/kn/root/root.go @@ -0,0 +1,140 @@ +// Copyright © 2018 The Knative 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 root + +import ( + "flag" + "fmt" + "os" + "strings" + + "github.com/pkg/errors" + "github.com/spf13/cobra" + "golang.org/x/crypto/ssh/terminal" + _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" + _ "k8s.io/client-go/plugin/pkg/client/auth/oidc" + + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/commands/completion" + "knative.dev/client/pkg/kn/commands/plugin" + "knative.dev/client/pkg/kn/commands/revision" + "knative.dev/client/pkg/kn/commands/route" + "knative.dev/client/pkg/kn/commands/service" + "knative.dev/client/pkg/kn/commands/source" + "knative.dev/client/pkg/kn/commands/trigger" + "knative.dev/client/pkg/kn/commands/version" + "knative.dev/client/pkg/kn/config" + "knative.dev/client/pkg/kn/flags" +) + +// NewRootCommand creates the default `kn` command with a default plugin handler +func NewRootCommand() (*cobra.Command, error) { + p := &commands.KnParams{} + p.Initialize() + + rootCmd := &cobra.Command{ + Use: "kn", + Short: "Knative client", + Long: `Manage your Knative building blocks: + +* Serving: Manage your services and release new software to them. +* Eventing: Manage event subscriptions and channels. Connect up event sources.`, + + // Disable docs header + DisableAutoGenTag: true, + + // Affects children as well + SilenceUsage: true, + + // Prevents Cobra from dealing with errors as we deal with them in main.go + SilenceErrors: true, + + // Validate our boolean configs + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + return flags.ReconcileBoolFlags(cmd.Flags()) + }, + } + if p.Output != nil { + rootCmd.SetOut(p.Output) + } + + // Bootstrap flags (rebinding to avoid errors when parsing the full commands) + config.AddBootstrapFlags(rootCmd.PersistentFlags()) + + // Global flags + rootCmd.PersistentFlags().StringVar(&p.KubeCfgPath, "kubeconfig", "", "kubectl configuration file (default: ~/.kube/config)") + flags.AddBothBoolFlags(rootCmd.PersistentFlags(), &p.LogHTTP, "log-http", "", false, "log http traffic") + + // root child commands + rootCmd.AddCommand(service.NewServiceCommand(p)) + rootCmd.AddCommand(revision.NewRevisionCommand(p)) + rootCmd.AddCommand(plugin.NewPluginCommand(p)) + rootCmd.AddCommand(route.NewRouteCommand(p)) + rootCmd.AddCommand(completion.NewCompletionCommand(p)) + rootCmd.AddCommand(version.NewVersionCommand(p)) + rootCmd.AddCommand(source.NewSourceCommand(p)) + rootCmd.AddCommand(trigger.NewTriggerCommand(p)) + + // Initialize default `help` cmd early to prevent unknown command errors + rootCmd.InitDefaultHelpCmd() + + // Deal with empty and unknown sub command groups + err := addEmptyAndUnknownSubCommandsValidation(rootCmd) + if err != nil { + return nil, err + } + // Wrap usage. + fitUsageMessageToTerminalWidth(rootCmd) + + // For glog parse error. + flag.CommandLine.Parse([]string{}) + return rootCmd, nil +} + +// addEmptyAndUnknownSubCommandsValidation adds a RunE to all commands that are groups to +// deal with errors when called with empty or unknown sub command +func addEmptyAndUnknownSubCommandsValidation(cmd *cobra.Command) error { + for _, childCmd := range cmd.Commands() { + if childCmd.HasSubCommands() { + if childCmd.RunE != nil || childCmd.Run != nil { + return errors.Errorf("internal: command group '%s' must not enable any direct logic, only leaf commands are allowed to take actions", childCmd.Name()) + } + + childCmd.RunE = func(aCmd *cobra.Command, args []string) error { + aCmd.Help() + if len(args) == 0 { + return fmt.Errorf("please provide a valid sub-command for \"kn %s\"", aCmd.Name()) + } + return fmt.Errorf("unknown sub-command \"%s\" for \"kn %s\"", args[0], aCmd.Name()) + } + } + + // recurse to deal with child commands that are themselves command groups + err := addEmptyAndUnknownSubCommandsValidation(childCmd) + if err != nil { + return err + } + } + return nil +} + +func fitUsageMessageToTerminalWidth(rootCmd *cobra.Command) { + width, _, err := terminal.GetSize(int(os.Stdout.Fd())) + if err == nil { + newUsage := strings.ReplaceAll(rootCmd.UsageTemplate(), "FlagUsages ", + fmt.Sprintf("FlagUsagesWrapped %d ", width)) + rootCmd.SetUsageTemplate(newUsage) + } +} diff --git a/pkg/kn/root/root_test.go b/pkg/kn/root/root_test.go new file mode 100644 index 0000000000..c739f9a810 --- /dev/null +++ b/pkg/kn/root/root_test.go @@ -0,0 +1,132 @@ +// Copyright © 2019 The Knative 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 root + +import ( + "strings" + "testing" + + "github.com/spf13/cobra" + "gotest.tools/assert" + + "knative.dev/client/pkg/util" +) + +func TestNewRootCommand(t *testing.T) { + rootCmd, err := NewRootCommand() + assert.NilError(t, err) + assert.Assert(t, rootCmd != nil) + + assert.Equal(t, rootCmd.Name(), "kn") + assert.Equal(t, rootCmd.Short, "Knative client") + assert.Assert(t, util.ContainsAll(rootCmd.Long, "Knative", "Serving", "Eventing")) + + assert.Assert(t, rootCmd.DisableAutoGenTag) + assert.Assert(t, rootCmd.SilenceUsage) + assert.Assert(t, rootCmd.SilenceErrors) + + assert.Assert(t, rootCmd.OutOrStdout() != nil) + + assert.Assert(t, rootCmd.PersistentFlags().Lookup("config") != nil) + assert.Assert(t, rootCmd.PersistentFlags().Lookup("kubeconfig") != nil) + + assert.Assert(t, rootCmd.RunE == nil) +} + +func TestSubCommands(t *testing.T) { + rootCmd, err := NewRootCommand() + assert.NilError(t, err) + checkLeafCommand(t, "version", rootCmd) +} + +func TestCommandGroup(t *testing.T) { + rootCmd, err := NewRootCommand() + assert.NilError(t, err) + commandGroups := []string{ + "service", "revision", "plugin", "source", "source apiserver", + "source sinkbinding", "source ping", "trigger", + } + for _, group := range commandGroups { + cmds := strings.Split(group, " ") + checkCommandGroup(t, cmds, rootCmd) + } +} + +func TestEmptyAndUnknownSubCommands(t *testing.T) { + rootCmd := &cobra.Command{ + Use: "root", + } + fakeGroupCmd := &cobra.Command{ + Use: "fake-group", + } + fakeSubCmd := &cobra.Command{ + Use: "fake-subcommand", + } + fakeGroupCmd.AddCommand(fakeSubCmd) + rootCmd.AddCommand(fakeGroupCmd) + + err := addEmptyAndUnknownSubCommandsValidation(rootCmd) + assert.NilError(t, err) + checkLeafCommand(t, "fake-subcommand", fakeGroupCmd) + checkCommandGroup(t, []string{"fake-group"}, rootCmd) +} + +func TestCommandGroupWithRunMethod(t *testing.T) { + rootCmd := &cobra.Command{ + Use: "root", + } + fakeGroupCmd := &cobra.Command{ + Use: "fake-group", + Run: func(cmd *cobra.Command, args []string) { + + }, + } + fakeSubCmd := &cobra.Command{ + Use: "fake-subcommand", + } + fakeGroupCmd.AddCommand(fakeSubCmd) + rootCmd.AddCommand(fakeGroupCmd) + + err := addEmptyAndUnknownSubCommandsValidation(rootCmd) + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAll(err.Error(), fakeGroupCmd.Name(), "internal", "not enable")) +} + +// Private + +func checkLeafCommand(t *testing.T, name string, rootCmd *cobra.Command) { + cmd, _, err := rootCmd.Find([]string{name}) + assert.Assert(t, err == nil) + assert.Assert(t, cmd != nil) + assert.Assert(t, !cmd.HasSubCommands()) +} + +func checkCommandGroup(t *testing.T, commands []string, rootCmd *cobra.Command) { + cmd, _, err := rootCmd.Find(commands) + assert.Assert(t, err == nil) + assert.Assert(t, cmd != nil) + assert.Assert(t, cmd.RunE != nil) + assert.Assert(t, cmd.HasSubCommands()) + + cmd.SetHelpFunc(func(command *cobra.Command, i []string) {}) // Avoid output noise when running the test + err = cmd.RunE(cmd, []string{}) + + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAll(err.Error(), "provide", "valid", "sub-command", cmd.Name())) + + err = cmd.RunE(cmd, []string{"deeper"}) + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAll(err.Error(), "deeper", "unknown", "sub-command", cmd.Name())) +} diff --git a/test/e2e/plugins_test.go b/test/e2e/plugins_test.go index 396090e5e5..2a93fd56a5 100644 --- a/test/e2e/plugins_test.go +++ b/test/e2e/plugins_test.go @@ -22,6 +22,7 @@ import ( "fmt" "io/ioutil" "os" + "os/exec" "path/filepath" "testing" @@ -101,12 +102,25 @@ func TestPluginWithoutLookup(t *testing.T) { listPlugin(r, knFlags, []string{pc.knPluginPath}, []string{}) t.Log("execute plugin in --plugins-dir") - runPlugin(r, knFlags, "helloe2e", []string{"e2e", "test"}, []string{"Hello Knative, I'm a Kn plugin", "I received arguments: e2e"}) + runPlugin(r, knFlags, "helloe2e", []string{"e2e", "test"}, []string{"Hello Knative, I'm a Kn plugin", "I received arguments", "e2e"}) t.Log("does not list any other plugin in $PATH") 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 TestPluginWithLookup(t *testing.T) { it, err := test.NewKnTest() assert.NilError(t, err) @@ -184,8 +198,12 @@ func listPlugin(r *test.KnRunResultCollector, knFlags []string, expectedPlugins out := test.Kn{}.Run(knArgs...) r.AssertNoError(out) - assert.Check(r.T(), util.ContainsAll(out.Stdout, expectedPlugins...)) - assert.Check(r.T(), util.ContainsNone(out.Stdout, unexpectedPlugins...)) + for _, p := range expectedPlugins { + assert.Check(r.T(), util.ContainsAll(out.Stdout, filepath.Base(p))) + } + for _, p := range unexpectedPlugins { + assert.Check(r.T(), util.ContainsNone(out.Stdout, filepath.Base(p))) + } } func runPlugin(r *test.KnRunResultCollector, knFlags []string, pluginName string, args []string, expectedOutput []string) { From 02de66142d79e79ada42dfdc65ab8a3e1bbf399a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Wed, 10 Jun 2020 16:59:46 +0200 Subject: [PATCH 2/8] fix: invalid subcommands will lead to a proper error message --- cmd/kn/main.go | 23 ++++++++++++++++++- cmd/kn/main_test.go | 49 ++++++++++++++++++++++++++++++++++++++++ pkg/kn/root/root.go | 31 +++++++++++++++++-------- pkg/kn/root/root_test.go | 6 ++--- vendor/modules.txt | 1 + 5 files changed, 96 insertions(+), 14 deletions(-) diff --git a/cmd/kn/main.go b/cmd/kn/main.go index 4377742cd6..36883674d8 100644 --- a/cmd/kn/main.go +++ b/cmd/kn/main.go @@ -80,7 +80,12 @@ func run(args []string) error { return plugin.Execute(argsWithoutCommands(args, plugin.CommandParts())) } else { - // Execute kn root command + // Validate args for root command + err = validateRootCommand(rootCmd) + if err != nil { + return err + } + // Execute kn root command, args are taken from os.Args directly return rootCmd.Execute() } } @@ -158,3 +163,19 @@ func validatePlugin(root *cobra.Command, plugin plugin.Plugin) error { } return nil } + +// Check whether an unknown sub-command is addressed and return an error if this is the case +// Needs to be called after the plugin has been extracted (as a plugin name can also lead to +// an unknown sub command error otherwise) +func validateRootCommand(cmd *cobra.Command) error { + foundCmd, innerArgs, err := cmd.Find(os.Args[1:]) + if err == nil && foundCmd.HasSubCommands() && len(innerArgs) > 0 { + argsWithoutFlags, err := stripFlags(innerArgs) + if len(argsWithoutFlags) > 0 || err != nil { + return errors.Errorf("unknown sub-command '%s' for '%s'. Available sub-commands: %s", innerArgs[0], foundCmd.Name(), strings.Join(root.ExtractSubCommandNames(foundCmd.Commands()), ", ")) + } + // If no args where given (only flags), then fall through to execute the command itself, which leads to + // a more appropriate error message + } + return nil +} diff --git a/cmd/kn/main_test.go b/cmd/kn/main_test.go index 6051cc6b41..d7db1e6340 100644 --- a/cmd/kn/main_test.go +++ b/cmd/kn/main_test.go @@ -23,6 +23,7 @@ import ( "gotest.tools/assert" "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/kn/root" "knative.dev/client/pkg/util" ) @@ -126,6 +127,54 @@ func TestArgsWithoutCommands(t *testing.T) { } } +func TestUnknownCommands(t *testing.T) { + oldArgs := os.Args + defer (func() { + os.Args = oldArgs + })() + + data := []struct { + givenCmdArgs []string + commandPath []string + expectedError []string + }{ + { + []string{"service", "udpate", "test", "--min-scale=0"}, + []string{"service"}, + []string{"unknown sub-command", "udpate"}, + }, + { + []string{"service", "--foo=bar"}, + []string{"service"}, + []string{}, + }, + { + []string{"source", "ping", "blub", "--foo=bar"}, + []string{"source", "ping"}, + []string{"unknown sub-command", "blub"}, + }, + } + for _, d := range data { + args := append([]string{"kn"}, d.givenCmdArgs...) + rootCmd, err := root.NewRootCommand() + os.Args = args + assert.NilError(t, err) + err = validateRootCommand(rootCmd) + if len(d.expectedError) == 0 { + assert.NilError(t, err) + continue + } + assert.Assert(t, err != nil) + assert.Assert(t, util.ContainsAll(err.Error(), d.expectedError...)) + cmd, _, e := rootCmd.Find(d.commandPath) + assert.NilError(t, e) + for _, sub := range cmd.Commands() { + assert.ErrorContains(t, err, sub.Name()) + } + } + +} + func TestStripFlags(t *testing.T) { data := []struct { diff --git a/pkg/kn/root/root.go b/pkg/kn/root/root.go index d402571372..b434ffb418 100644 --- a/pkg/kn/root/root.go +++ b/pkg/kn/root/root.go @@ -90,39 +90,41 @@ func NewRootCommand() (*cobra.Command, error) { // Initialize default `help` cmd early to prevent unknown command errors rootCmd.InitDefaultHelpCmd() - // Deal with empty and unknown sub command groups - err := addEmptyAndUnknownSubCommandsValidation(rootCmd) + // Check that command groups can't execute and that leaf commands don't h + err := validateCommandStructure(rootCmd) if err != nil { return nil, err } + // Wrap usage. fitUsageMessageToTerminalWidth(rootCmd) - // For glog parse error. + // For glog parse error. TOO: Check why this is needed flag.CommandLine.Parse([]string{}) return rootCmd, nil } -// addEmptyAndUnknownSubCommandsValidation adds a RunE to all commands that are groups to -// deal with errors when called with empty or unknown sub command -func addEmptyAndUnknownSubCommandsValidation(cmd *cobra.Command) error { +// Verify that command groups are not executable and that leaf commands have a run function +func validateCommandStructure(cmd *cobra.Command) error { for _, childCmd := range cmd.Commands() { if childCmd.HasSubCommands() { if childCmd.RunE != nil || childCmd.Run != nil { return errors.Errorf("internal: command group '%s' must not enable any direct logic, only leaf commands are allowed to take actions", childCmd.Name()) } + subCommands := childCmd.Commands() + name := childCmd.Name() childCmd.RunE = func(aCmd *cobra.Command, args []string) error { - aCmd.Help() + subText := fmt.Sprintf("Available sub-commands: %s", strings.Join(ExtractSubCommandNames(subCommands), ", ")) if len(args) == 0 { - return fmt.Errorf("please provide a valid sub-command for \"kn %s\"", aCmd.Name()) + return fmt.Errorf("no sub-command given for 'kn %s'. %s", name, subText) } - return fmt.Errorf("unknown sub-command \"%s\" for \"kn %s\"", args[0], aCmd.Name()) + return fmt.Errorf("unknown sub-command '%s' for 'kn %s'. %s", args[0], aCmd.Name(), subText) } } // recurse to deal with child commands that are themselves command groups - err := addEmptyAndUnknownSubCommandsValidation(childCmd) + err := validateCommandStructure(childCmd) if err != nil { return err } @@ -138,3 +140,12 @@ func fitUsageMessageToTerminalWidth(rootCmd *cobra.Command) { rootCmd.SetUsageTemplate(newUsage) } } + +// ExtractSubCommandNames extracts the names of all sub commands of a given command +func ExtractSubCommandNames(cmds []*cobra.Command) []string { + var ret []string + for _, subCmd := range cmds { + ret = append(ret, subCmd.Name()) + } + return ret +} diff --git a/pkg/kn/root/root_test.go b/pkg/kn/root/root_test.go index c739f9a810..ed99dc5120 100644 --- a/pkg/kn/root/root_test.go +++ b/pkg/kn/root/root_test.go @@ -77,7 +77,7 @@ func TestEmptyAndUnknownSubCommands(t *testing.T) { fakeGroupCmd.AddCommand(fakeSubCmd) rootCmd.AddCommand(fakeGroupCmd) - err := addEmptyAndUnknownSubCommandsValidation(rootCmd) + err := validateCommandStructure(rootCmd) assert.NilError(t, err) checkLeafCommand(t, "fake-subcommand", fakeGroupCmd) checkCommandGroup(t, []string{"fake-group"}, rootCmd) @@ -99,7 +99,7 @@ func TestCommandGroupWithRunMethod(t *testing.T) { fakeGroupCmd.AddCommand(fakeSubCmd) rootCmd.AddCommand(fakeGroupCmd) - err := addEmptyAndUnknownSubCommandsValidation(rootCmd) + err := validateCommandStructure(rootCmd) assert.Assert(t, err != nil) assert.Assert(t, util.ContainsAll(err.Error(), fakeGroupCmd.Name(), "internal", "not enable")) } @@ -124,7 +124,7 @@ func checkCommandGroup(t *testing.T, commands []string, rootCmd *cobra.Command) err = cmd.RunE(cmd, []string{}) assert.Assert(t, err != nil) - assert.Assert(t, util.ContainsAll(err.Error(), "provide", "valid", "sub-command", cmd.Name())) + assert.Assert(t, util.ContainsAll(err.Error(), "no", "sub-command", cmd.Name())) err = cmd.RunE(cmd, []string{"deeper"}) assert.Assert(t, err != nil) diff --git a/vendor/modules.txt b/vendor/modules.txt index 24300b3155..e12cce8a7e 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -98,6 +98,7 @@ github.com/pelletier/go-toml # github.com/peterbourgon/diskv v2.0.1+incompatible github.com/peterbourgon/diskv # github.com/pkg/errors v0.9.1 +## explicit github.com/pkg/errors # github.com/robfig/cron/v3 v3.0.1 github.com/robfig/cron/v3 From 2d6bdbd9ce5860048b83369371be2c289c35a006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 15 Jun 2020 14:50:19 +0200 Subject: [PATCH 3/8] Update pkg/kn/config/types.go Co-authored-by: Navid Shaikh --- pkg/kn/config/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kn/config/types.go b/pkg/kn/config/types.go index 1e97c8f296..281e1f8f85 100644 --- a/pkg/kn/config/types.go +++ b/pkg/kn/config/types.go @@ -15,7 +15,7 @@ package config // Package for holding configuration types used in bootstrapping -// and for types in configuration foles +// and for types in configuration files type Config interface { From a7144b13f520b528e8153ca447af1194daf210c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 15 Jun 2020 14:50:28 +0200 Subject: [PATCH 4/8] Update pkg/kn/plugin/manager.go Co-authored-by: Navid Shaikh --- pkg/kn/plugin/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kn/plugin/manager.go b/pkg/kn/plugin/manager.go index 0f3b49dcbe..df20ba22b3 100644 --- a/pkg/kn/plugin/manager.go +++ b/pkg/kn/plugin/manager.go @@ -1,4 +1,4 @@ -// Copyright © 2019 The Knative Authors +// Copyright © 2020 The Knative Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 188b6c187eaafe3d75b9429c80f656620fa9390a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 15 Jun 2020 14:50:47 +0200 Subject: [PATCH 5/8] Update hack/generate-docs.go Co-authored-by: Navid Shaikh --- hack/generate-docs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/generate-docs.go b/hack/generate-docs.go index fe52b10e76..d8130aed64 100644 --- a/hack/generate-docs.go +++ b/hack/generate-docs.go @@ -40,7 +40,7 @@ func main() { if len(os.Args) > 2 { withFrontMatter, err = strconv.ParseBool(os.Args[2]) if err != nil { - log.Panicf("Invalid argument %s, has to be boolean to switch on/off generation of frontmatter (%v)", os.Args[2], err) + log.Panicf("invalid argument %s, has to be boolean to switch on/off generation of frontmatter (%v)", os.Args[2], err) } } prependFunc := emptyString From 0063a263d121702432c1ee71cef30df375a40e76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 15 Jun 2020 14:51:02 +0200 Subject: [PATCH 6/8] Update hack/generate-docs.go Co-authored-by: Navid Shaikh --- hack/generate-docs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/generate-docs.go b/hack/generate-docs.go index d8130aed64..f87b9a38fc 100644 --- a/hack/generate-docs.go +++ b/hack/generate-docs.go @@ -29,7 +29,7 @@ import ( func main() { rootCmd, err := root.NewRootCommand() if err != nil { - log.Panicf("Can not create root command: %v", err) + log.Panicf("can not create root command: %v", err) } dir := "." From 65a8f2ac3bc7bd729915283c21a0332717b3aa83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 15 Jun 2020 14:59:53 +0200 Subject: [PATCH 7/8] chore: Add missing links --- docs/dev/developer-guide.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/dev/developer-guide.md b/docs/dev/developer-guide.md index 19308d1b6c..00c6c156dc 100644 --- a/docs/dev/developer-guide.md +++ b/docs/dev/developer-guide.md @@ -8,7 +8,7 @@ _This guide is not complete and has still many gaps that we are going to fill ov ## Flow -The journey starts at [main.go]() which is the entry point when you call `kn`. +The journey starts at [main.go](https://github.com/knative/client/blob/master/cmd/kn/main.go) which is the entry point when you call `kn`. You find here the main control flow, which can be roughly divided into three phases: - _Bootstrap_ is about retrieving essential configuration parameters from command-line flags, and a configuration file. @@ -23,7 +23,7 @@ Let's talk now about the three phases separately. ### Bootstrap -The bootstrap performed by [config.BootstrapConfig()]() extracts all the options relevant for config file detection and plugin configuration. +The bootstrap performed by [config.BootstrapConfig()](https://github.com/knative/client/blob/master/pkg/kn/config/config.go#L94) extracts all the options relevant for config file detection and plugin configuration. The bootstrap process does not fully parse all arguments but only those that are relevant for starting up and for looking up any plugin. The configuration can be either provided via a `--config` flag or is picked up from a default location. The default configuration location conforms to the [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html) and is different for Unix systems and Windows systems. @@ -33,7 +33,7 @@ The default configuration location conforms to the [XDG Base Directory Specifica ### Plugin Lookup In the next step, a `PluginManager` checks whether the given command-line arguments are pointing to a plugin. -All non-flag arguments are extracted and then used to lookup via [plugin.PluginManager.FindPlugin()]() in the plugin directory (and the execution `$PATH` if configured) calculated in the _Bootstrap_ phase. +All non-flag arguments are extracted and then used to lookup via [plugin.PluginManager.FindPlugin()](https://github.com/knative/client/blob/master/pkg/kn/plugin/manager.go#L94) in the plugin directory (and the execution `$PATH` if configured) calculated in the _Bootstrap_ phase. ### Execution From 72408dccf0514ab7e983fdcc4ef23dfc1497432e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Mon, 15 Jun 2020 15:09:47 +0200 Subject: [PATCH 8/8] chore: recert to shas in links in developer guide for now. --- docs/dev/developer-guide.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/dev/developer-guide.md b/docs/dev/developer-guide.md index 00c6c156dc..b5c41befa5 100644 --- a/docs/dev/developer-guide.md +++ b/docs/dev/developer-guide.md @@ -23,7 +23,7 @@ Let's talk now about the three phases separately. ### Bootstrap -The bootstrap performed by [config.BootstrapConfig()](https://github.com/knative/client/blob/master/pkg/kn/config/config.go#L94) extracts all the options relevant for config file detection and plugin configuration. +The bootstrap performed by [config.BootstrapConfig()](https://github.com/knative/client/blob/0063a263d121702432c1ee71cef30df375a40e76/pkg/kn/config/config.go#L94) extracts all the options relevant for config file detection and plugin configuration. The bootstrap process does not fully parse all arguments but only those that are relevant for starting up and for looking up any plugin. The configuration can be either provided via a `--config` flag or is picked up from a default location. The default configuration location conforms to the [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html) and is different for Unix systems and Windows systems. @@ -33,7 +33,7 @@ The default configuration location conforms to the [XDG Base Directory Specifica ### Plugin Lookup In the next step, a `PluginManager` checks whether the given command-line arguments are pointing to a plugin. -All non-flag arguments are extracted and then used to lookup via [plugin.PluginManager.FindPlugin()](https://github.com/knative/client/blob/master/pkg/kn/plugin/manager.go#L94) in the plugin directory (and the execution `$PATH` if configured) calculated in the _Bootstrap_ phase. +All non-flag arguments are extracted and then used to lookup via [plugin.PluginManager.FindPlugin()](https://github.com/knative/client/blob/0063a263d121702432c1ee71cef30df375a40e76/pkg/kn/plugin/manager.go#L94) in the plugin directory (and the execution `$PATH` if configured) calculated in the _Bootstrap_ phase. ### Execution