Change 'prerelease' to 'channel'.

This adds support for deprecating a boolean flag
into a non-boolean value, and uses it to make `--prerelease`
deprecated in favor of `--channel current`.  By default,
`channel` is `stable`.

Separately, all deprecated options are now configured
to display at the end of the options list, instead of mixed into
the non-deprecated list.

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>

2 files changed, 58 insertions, 41 deletions

diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index 701c207503..92f6fbcec3 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -86,7 +86,8 @@ class Chef
         short: "-w AUTH-METHOD",
         long: "--winrm-auth-method AUTH-METHOD",
         description: "The WinRM authentication method to use. Valid choices are #{friendly_opt_list(WINRM_AUTH_PROTOCOL_LIST)}.",
-        proc: Proc.new { |protocol| Chef::Config[:knife][:winrm_auth_method] = protocol }
+        proc: Proc.new { |protocol| Chef::Config[:knife][:winrm_auth_method] = protocol },
+        in: WINRM_AUTH_PROTOCOL_LIST
 
       option :winrm_basic_auth_only,
         long: "--winrm-basic-auth-only",
@@ -156,9 +157,11 @@ class Chef
         description: "The version of #{Chef::Dist::PRODUCT} to install.",
         proc: lambda { |v| Chef::Config[:knife][:bootstrap_version] = v }
 
-      option :prerelease,
-        long: "--prerelease",
-        description: "Install from 'current' channel"
+      option :channel,
+        long: "--channel CHANNEL",
+        description: "Install from the given channel. Valid values are 'current' and 'stable'. Default is 'stable'",
+        default: "stable",
+        in: %w{stable current}
 
       # client.rb content via chef-full/bootstrap_context
       option :bootstrap_proxy,
@@ -351,42 +354,45 @@ class Chef
 
       DEPRECATED_FLAGS = {
         # deprecated_key: [new_key, deprecated_long]
-        auth_timeout: [:max_wait, "--max-wait SECONDS"],
-        host_key_verify: [:ssh_verify_host_key,
-                          "--[no-]host-key-verify",
-                          ],
-        ssh_user: [:connection_user,
-                   "--ssh-user USER",
-                   ],
-        ssh_password: [:connection_password,
-                       "--ssh-password PASSWORD",
-                       ],
-        ssh_port: [:connection_port,
-                   "-ssh-port",
-                   ],
-        ssl_peer_fingerprint: [:winrm_ssl_peer_fingerprint,
-                               "--ssl-peer-fingerprint FINGERPRINT",
-                               ],
-        winrm_user: [:connection_user,
-                     "--winrm-user USER",
-                     ],
-        winrm_password: [:connection_password,
-                         "--winrm-password",
-                         ],
-        winrm_port: [:connection_port,
-                     "--winrm-port",
-                     ],
-        winrm_authentication_protocol: [:winrm_auth_method,
-                                        "--winrm-authentication-protocol PROTOCOL",
-                                        ],
+        # optional third element: replacement_value - if converting from bool
+        #   (--bool-option) to valued flag (--new-option VALUE)
+        #   this will be the value that is assigned the new flag when the old flag is used.
+        auth_timeout: [:max_wait, "--max-wait SECONDS" ],
+        host_key_verify:
+          [:ssh_verify_host_key, "--[no-]host-key-verify"],
+        prerelease:
+          [:channel, "--prerelease", "current"],
+        ssh_user:
+          [:connection_user, "--ssh-user USER"],
+        ssh_password:
+          [:connection_password, "--ssh-password PASSWORD"],
+        ssh_port:
+          [:connection_port, "-ssh-port" ],
+        ssl_peer_fingerprint:
+          [:winrm_ssl_peer_fingerprint, "--ssl-peer-fingerprint FINGERPRINT"],
+        winrm_user:
+          [:connection_user, "--winrm-user USER"],
+        winrm_password:
+          [:connection_password, "--winrm-password"],
+        winrm_port:
+          [:connection_port, "--winrm-port"],
+        winrm_authentication_protocol:
+          [:winrm_auth_method, "--winrm-authentication-protocol PROTOCOL"],
       }.freeze
 
       DEPRECATED_FLAGS.each do |deprecated_key, deprecation_entry|
-        new_key, deprecated_long = deprecation_entry
+        new_key, deprecated_long, replacement_value = deprecation_entry
         new_long = options[new_key][:long]
+        new_long_desc = if replacement_value.nil?
+                          new_long
+                        else
+                          "#{new_long.split(" ").first} #{replacement_value}"
+                        end
         option(deprecated_key, long: deprecated_long,
-                               description: "#{deprecated_long} is deprecated. Use #{new_long} instead.",
-                               boolean: options[new_key][:boolean])
+                               description: "This flag is deprecated. Please use '#{new_long_desc}' instead.",
+                               boolean: options[new_key][:boolean] || !replacement_value.nil?,
+                               # Put deprecated options at the end of the options list
+                               on: :tail)
       end
 
       attr_accessor :client_builder
@@ -642,12 +648,13 @@ class Chef
       end
 
       # If any deprecated flags are used, let the user know and
-      # update config[new-key] to the value given to the deprecated flag.
+      # update config[new-key] to the value given to the deprecated flag,
+      # or to the mapped value in case of changing flag type.
       # If a deprecated flag and its corresponding replacement
-      # are both used, raise an error.
+      # are both used, exit
       def verify_deprecated_flags!
         DEPRECATED_FLAGS.each do |deprecated_key, deprecation_entry|
-          new_key, deprecated_long = deprecation_entry
+          new_key, deprecated_long, replacement_value = deprecation_entry
           if config.key?(deprecated_key) && config_source(deprecated_key) == :cli
             if config.key?(new_key) && config_source(new_key) == :cli
               new_long = options[new_key][:long].split(" ").first
@@ -660,9 +667,9 @@ class Chef
               EOM
               exit 1
             else
-              config[new_key] = config[deprecated_key]
+              config[new_key] = replacement_value || config[deprecated_key]
               unless Chef::Config[:silence_deprecation_warnings] == true
-                ui.warn options[deprecated_key][:description]
+                ui.warn "You provided #{deprecated_long.split(" ").first}. #{options[deprecated_key][:description]}"
               end
             end
           end
diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb
index 995a2ef4c9..d82dc7614e 100644
--- a/spec/unit/knife/bootstrap_spec.rb
+++ b/spec/unit/knife/bootstrap_spec.rb
@@ -1608,13 +1608,14 @@ describe Chef::Knife::Bootstrap do
         knife.merge_configs
       end
       it "maps the key value to the new key and points the human to the new flag" do
-        expect(knife.ui).to receive(:warn).with(/--ssh-user USER is deprecated. Use --connection-user USERNAME instead./)
+        expect(knife.ui).to receive(:warn).with(/You provided --ssh-user. This flag is deprecated. Please use '--connection-user USERNAME' instead./)
         knife.verify_deprecated_flags!
         expect(knife.config[:connection_user]).to eq "sshuser"
       end
     end
 
     context "when a deprecated CLI flag is given on the CLI, along with its replacement" do
+      let(:bootstrap_cli_options) { %w{--connection-user a --ssh-user b} }
       before do
         knife.config[:ssh_user] = "sshuser"
         knife.config[:connection_user] = "real-user"
@@ -1626,6 +1627,15 @@ describe Chef::Knife::Bootstrap do
         expect { knife.verify_deprecated_flags! }.to raise_error SystemExit
       end
     end
+
+    context "when a deprecated boolean CLI flag is given on the CLI, and its non-boolean replacement is used" do
+      let(:bootstrap_cli_options) { %w{--prerelease} }
+      it "correctly maps the old boolean value to the new value" do
+        expect(knife.ui).to receive(:warn)
+        knife.verify_deprecated_flags!
+        expect(knife.config[:channel]).to eq "current"
+      end
+    end
   end
 
   describe "#register_client" do