Deprecate `lookup-path` option and updated relevant documentation (#1422)

* Deprecate `lookup-path` option and updated relevant documentation

Lookup in path is now the default for all plugins.
This option will be removed eventually in a future version,
when path lookup is enabled uncoditinally and can't be turned of.

* Review fixes

* remove trailing whitespace

* readd config to make test coverage happy

CHANGELOG.adoc

@@ -17,20 +17,25 @@
 |===
 | | Description | PR
 
+| ✨
+| Deprecate `lookup-path` as path lookup will always be enabled in the future
+| https://github.com/knative/client/pull/1422[#1422]
+
+
+| ✨
+| Lookup plugins in `$PATH` by default
+| https://github.com/knative/client/pull/1412[#1412]
+
 | 🐛
 | Show server error messages without any taints
 | https://github.com/knative/client/pull/1406[#1406]
 
 | 🎁
-| Adding --class flag to broker create command
+| Adding `--class` flag to broker create command
 | https://github.com/knative/client/pull/1402[#1402]
 
 | 🎁
-| Add an `client.knative.dev/updateTimestamp` annotation to trigger a new revision when required
-| https://github.com/knative/client/pull/1364[#1364]
-
-| 🎁
-| Adding darwin/arm64 support to kn
+| Adding `darwin/arm64` support to kn
 | https://github.com/knative/client/pull/1401[#1401]
 
 | 🎁
@@ -45,6 +50,10 @@
 | make --cmd flag as an array instead of string
 | https://github.com/knative/client/pull/1380[#1380]
 
+| 🎁
+| Add an `client.knative.dev/updateTimestamp` annotation to trigger a new revision when required
+| https://github.com/knative/client/pull/1364[#1364]
+
 |===
 
 ## v0.24.0 (2021-06-29)

docs/README.md

@@ -77,7 +77,7 @@ The following example contains a fully commented `config.yaml` with all availabl
 ```yaml
 # Plugins related configuration
 plugins:
-  # Whether to lookup configuration in the execution path (default: false)
+  # Whether to lookup configuration in the execution path (default: true). This option is deprecated and will be removed in a future version where path lookup will be enabled unconditionally
   path-lookup: true
   # Directory from where plugins are looked up. (default: "$base_dir/plugins"
   # where "$base_dir" is the directory where this configuration file is stored)
@@ -118,7 +118,7 @@ You can specify the following options:
    plugins. It can be any directory that is visible to the user.
 
 * `path-lookup`, which is the same as the persistent flag
-   `--lookup-plugins-in-path` and specifies if `kn` should look for plugins anywhere in the specified `PATH` environment variable. This option is a boolean type, and the default value is `false`.
+   `--lookup-plugins-in-path` and specifies if `kn` should look for plugins anywhere in the specified `PATH` environment variable. This option is a boolean type, and the default value is `true`. This option is DEPRECATED and will be removed in a future version, when path lookup will be always enabled.
 
 #### Eventing configuration
 

docs/cmd/kn_plugin_list.md

@@ -10,7 +10,7 @@ Available plugins are those that are:
 - executable
 - begin with "kn-"
 - Kn's plugin directory
-- Anywhere in the execution $PATH (if plugins.path-lookup configuration variable is enabled)
+- Anywhere in the execution $PATH
 
 ```
 kn plugin list

pkg/kn/commands/plugin/list.go

@@ -48,7 +48,7 @@ Available plugins are those that are:
 - executable
 - begin with "kn-"
 - Kn's plugin directory
-- Anywhere in the execution $PATH (if plugins.path-lookup configuration variable is enabled)`,
+- Anywhere in the execution $PATH`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return listPlugins(cmd, plFlags)
 		},

pkg/kn/config/config.go

@@ -71,11 +71,8 @@ func (c *config) PluginsDir() string {
 
 // 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)
+	if viper.IsSet(deprecatedKeyPluginsLookupInPath) {
+		return viper.GetBool(deprecatedKeyPluginsLookupInPath)
 	} else {
 		// If legacy branch is removed, switch to setting the default to viper
 		// See TODO comment below.
@@ -117,7 +114,7 @@ func BootstrapConfig() error {
 	if err != nil {
 		return err
 	}
-	err = viper.BindPFlag(keyPluginsLookupInPath, bootstrapFlagSet.Lookup(flagPluginsLookupInPath))
+	err = viper.BindPFlag(deprecatedKeyPluginsLookupInPath, bootstrapFlagSet.Lookup(flagPluginsLookupInPath))
 	if err != nil {
 		return err
 	}
@@ -140,7 +137,7 @@ func BootstrapConfig() error {
 	// 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)
+	// viper.SetDefault(deprecatedKeyPluginsLookupInPath, bootstrapDefaults.lookupPluginsInPath)
 
 	// If a config file is found, read it in.
 	err = viper.ReadInConfig()

pkg/kn/config/config_test.go

@@ -30,7 +30,6 @@ func TestBootstrapConfig(t *testing.T) {
 plugins:
   directory: /tmp
   path-lookup: true
-
 eventing:
   sink-mappings:
   - prefix: service

pkg/kn/config/types.go

@@ -71,7 +71,7 @@ type ChannelTypeMapping struct {
 // config Keys for looking up in viper
 const (
 	keyPluginsDirectory              = "plugins.directory"
-	keyPluginsLookupInPath = "plugins.path-lookup"
+	deprecatedKeyPluginsLookupInPath = "plugins.path-lookup"
 	keySinkMappings                  = "eventing.sink-mappings"
 	keyChannelTypeMappings           = "eventing.channel-type-mappings"
 )
@@ -79,7 +79,6 @@ const (
 // legacy config keys, deprecated
 const (
 	legacyKeyPluginsDirectory = "plugins-dir"
-	legacyKeyPluginsLookupInPath = "lookup-plugins"
 	legacyKeySinkMappings     = "sink"
 )