use mixlib-cli's deprecation mechanismupdate-bootstrap-deprecations

Deprecation logic is now available in `mixlib-cli`
via `deprecated_option`.  Let's use that instead
of keeping our custom deprecation behavior.

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

4 files changed, 84 insertions, 221 deletions

diff --git a/Gemfile.lock b/Gemfile.lock
index 75ebc48476..a8e5daaf16 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,17 +1,17 @@
 GIT
   remote: https://github.com/chef/chefstyle.git
-  revision: c9f82052de99484c69fdc368a0c239d03eb4e6d2
+  revision: 80dc0f94064ece591fcb8c437aa1d60e2881ba10
   branch: master
   specs:
-    chefstyle (0.12.1)
+    chefstyle (0.12.3)
       rubocop (= 0.62.0)
 
 GIT
   remote: https://github.com/chef/ohai.git
-  revision: 3734d441498e51dd44cefc21d842232cd0b5a47d
+  revision: f04de72cd0a1b8636ca60c238930c4ed60187516
   branch: master
   specs:
-    ohai (15.0.36)
+    ohai (15.0.39)
       chef-config (>= 12.8, < 16)
       ffi (~> 1.9)
       ffi-yajl (~> 2.2)
@@ -44,7 +44,7 @@ PATH
       license-acceptance (~> 1.0, >= 1.0.5)
       mixlib-archive (>= 0.4, < 2.0)
       mixlib-authentication (~> 2.1)
-      mixlib-cli (>= 1.7, < 3.0)
+      mixlib-cli (>= 2.1.1, < 3.0)
       mixlib-log (>= 2.0.3, < 4.0)
       mixlib-shellout (>= 2.4, < 4.0)
       net-sftp (~> 2.1, >= 2.1.2)
@@ -75,7 +75,7 @@ PATH
       license-acceptance (~> 1.0, >= 1.0.5)
       mixlib-archive (>= 0.4, < 2.0)
       mixlib-authentication (~> 2.1)
-      mixlib-cli (>= 1.7, < 3.0)
+      mixlib-cli (>= 2.1.1, < 3.0)
       mixlib-log (>= 2.0.3, < 4.0)
       mixlib-shellout (>= 2.4, < 4.0)
       net-sftp (~> 2.1, >= 2.1.2)
@@ -169,7 +169,7 @@ GEM
       ffi (>= 1.0.1)
     gyoku (1.3.1)
       builder (>= 2.1.2)
-    hashdiff (0.3.9)
+    hashdiff (0.4.0)
     hashie (3.6.0)
     highline (1.7.10)
     htmlentities (4.3.4)
@@ -220,12 +220,12 @@ GEM
     mixlib-archive (1.0.1-universal-mingw32)
       mixlib-log
     mixlib-authentication (2.1.1)
-    mixlib-cli (2.0.6)
+    mixlib-cli (2.1.1)
     mixlib-config (3.0.1)
       tomlrb
     mixlib-log (3.0.1)
-    mixlib-shellout (2.4.4)
-    mixlib-shellout (2.4.4-universal-mingw32)
+    mixlib-shellout (3.0.4)
+    mixlib-shellout (3.0.4-universal-mingw32)
       win32-process (~> 0.8.2)
       wmi-lite (~> 1.0)
     multi_json (1.13.1)
@@ -264,7 +264,7 @@ GEM
     pry-stack_explorer (0.4.9.3)
       binding_of_caller (>= 0.7)
       pry (>= 0.9.11)
-    public_suffix (3.0.3)
+    public_suffix (3.1.0)
     rack (2.0.7)
     rainbow (3.0.0)
     rake (12.3.2)
@@ -296,8 +296,9 @@ GEM
       rainbow (>= 2.2.2, < 4.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (~> 1.4.0)
-    ruby-prof (0.17.0)
-    ruby-progressbar (1.10.0)
+    ruby-prof (0.18.0)
+    ruby-prof (0.18.0-x64-mingw32)
+    ruby-progressbar (1.10.1)
     ruby-shadow (2.5.0)
     rubyntlm (0.6.2)
     rubyzip (1.2.3)
@@ -322,7 +323,7 @@ GEM
       tins (~> 1.0)
     thor (0.20.3)
     timers (4.3.0)
-    tins (1.20.2)
+    tins (1.20.3)
     tomlrb (1.2.8)
     train-core (2.1.7)
       json (>= 1.8, < 3.0)
@@ -357,10 +358,10 @@ GEM
     unicode-display_width (1.4.1)
     unicode_utils (1.4.0)
     uuidtools (2.1.5)
-    webmock (3.5.1)
+    webmock (3.6.0)
       addressable (>= 2.3.6)
       crack (>= 0.3.2)
-      hashdiff
+      hashdiff (>= 0.4.0, < 2.0.0)
     win32-api (1.5.3-universal-mingw32)
     win32-certstore (0.3.0)
       ffi
@@ -437,4 +438,4 @@ DEPENDENCIES
   yard
 
 BUNDLED WITH
-   1.17.3
+   1.17.2
diff --git a/chef.gemspec b/chef.gemspec
index 4524da6d9a..a89cf4ceb2 100644
--- a/chef.gemspec
+++ b/chef.gemspec
@@ -19,7 +19,7 @@ Gem::Specification.new do |s|
   s.add_dependency "train-core", "~> 2.0", ">= 2.0.12"
 
   s.add_dependency "license-acceptance", "~> 1.0", ">= 1.0.5"
-  s.add_dependency "mixlib-cli", ">= 1.7", "< 3.0"
+  s.add_dependency "mixlib-cli", ">= 2.1.1", "< 3.0"
   s.add_dependency "mixlib-log", ">= 2.0.3", "< 4.0"
   s.add_dependency "mixlib-authentication", "~> 2.1"
   s.add_dependency "mixlib-shellout", ">= 2.4", "< 4.0"
diff --git a/lib/chef/knife/bootstrap.rb b/lib/chef/knife/bootstrap.rb
index f60928375e..19384ba768 100644
--- a/lib/chef/knife/bootstrap.rb
+++ b/lib/chef/knife/bootstrap.rb
@@ -347,67 +347,70 @@ class Chef
           Chef::Config[:knife][:bootstrap_vault_item]
         }
 
-      DEPRECATED_FLAGS = {
-        # deprecated_key: [new_key, deprecated_long, replacement_value, deprecated_short]
-        # 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.
-        # optional fourth element: deprecated_short - if the short form of flag has also been deprecated.
-        auth_timeout: [:max_wait, "--max-wait SECONDS" ],
-        forward_agent:
-          [:ssh_forward_agent, "--forward-agent"],
-        host_key_verify:
-          [:ssh_verify_host_key, "--[no-]host-key-verify"],
-        prerelease:
-          [:channel, "--prerelease", "current"],
-        ssh_user:
-          [:connection_user, "--ssh-user USERNAME"],
-        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 USERNAME", nil, "-x USERNAME"],
-        winrm_password:
-          [:connection_password, "--winrm-password PASSWORD"],
-        winrm_port:
-          [:connection_port, "--winrm-port"],
-        winrm_authentication_protocol:
-          [:winrm_auth_method, "--winrm-authentication-protocol PROTOCOL"],
-        winrm_session_timeout:
-          [:session_timeout, "--winrm-session-timeout MINUTES"],
-        winrm_ssl_verify_mode:
-          [:winrm_no_verify_cert, "--winrm-ssl-verify-mode MODE"],
-        winrm_transport:
-          [:winrm_ssl, "--winrm-transport TRANSPORT"],
-      }.freeze
-
-      DEPRECATED_FLAGS.each do |deprecated_key, deprecation_entry|
-        new_key, deprecated_long, replacement_value, deprecated_short = deprecation_entry
-        new_long = options[new_key][:long]
-        new_long_key = new_long.split(" ").first
-        if new_short = options[new_key][:short]
-          new_long += "(or #{new_short})"
-          new_long_key += "(or #{new_short.split(" ").first})"
-        end
-        new_long_desc = if replacement_value.nil?
-                          new_long
-                        else
-                          "#{new_long_key} #{replacement_value}"
-                        end
-
-        opt = { long: deprecated_long,
-                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 }
-
-        opt[:short] = deprecated_short if deprecated_short
-
-        option(deprecated_key, opt)
-      end
+      # Deprecated options. These must be declared after
+      # regular options because they refer to the replacement
+      # option definitions implicitly.
+      deprecated_option :auth_timeout,
+        replacement: :max_wait,
+        long: "--max-wait SECONDS"
+
+      deprecated_option :forward_agent,
+        replacement: :ssh_forward_agent,
+        boolean: true, long: "--forward-agent"
+
+      deprecated_option :host_key_verify,
+        replacement: :ssh_verify_host_key,
+        boolean: true, long: "--[no-]host-key-verify",
+        value_mapper: Proc.new { |verify| verify ? "always" : "never" }
+
+      deprecated_option :prerelease,
+        replacement: :channel,
+        long: "--prerelease",
+        boolean: true, value_mapper: Proc.new { "current" }
+
+      deprecated_option :ssh_user,
+        replacement: :connection_user,
+        long: "--ssh-user USERNAME"
+
+      deprecated_option :ssh_password,
+        replacement: :connection_password,
+        long: "--ssh-password PASSWORD"
+
+      deprecated_option :ssh_port,
+        replacement: :connection_port,
+        long: "--ssh-port PASSWORD"
+
+      deprecated_option :ssl_peer_fingerprint,
+        replacement: :winrm_ssl_peer_fingerprint,
+        long: "--ssl-peer-fingerprint FINGERPRINT"
+
+      deprecated_option :winrm_user,
+        replacement: :connection_user,
+        long: "--winrm-user USERNAME", short: "-x USERNAME"
+
+      deprecated_option :winrm_password,
+        replacement: :connection_password,
+        long: "--winrm-password PASSWORD"
+
+      deprecated_option :winrm_port,
+        replacement: :connection_port,
+        long: "--winrm-port PORT"
+
+      deprecated_option :winrm_authentication_protocol,
+        replacement: :winrm_auth_method,
+        long: "--winrm-authentication-protocol PROTOCOL"
+
+      deprecated_option :winrm_session_timeout,
+        replacement: :session_timeout,
+        long: "--winrm-session-timeout MINUTES"
+
+      deprecated_option :winrm_ssl_verify_mode,
+        replacement: :winrm_no_verify_cert,
+        long: "--winrm-ssl-verify-mode MODE"
+
+      deprecated_option :winrm_transport, replacement: :winrm_ssl,
+        long: "--winrm-transport TRANSPORT",
+        value_mapper: Proc.new { |value| value == "ssl" }
 
       attr_reader :connection
 
@@ -548,7 +551,6 @@ class Chef
 
       def run
         check_license
-        verify_deprecated_flags!
 
         plugin_setup!
         validate_name_args!
@@ -693,7 +695,7 @@ class Chef
               Validatorless bootstrap over unsecure winrm channels could expose your
               key to network sniffing.
                Please use a 'winrm_auth_method' other than 'plaintext',
-              or enable ssl on #{server_name} then use the --ssl flag
+              or enable ssl on #{server_name} then use the ---winrm-ssl flag
               to connect.
             EOM
 
@@ -703,45 +705,6 @@ class Chef
         true
       end
 
-      # If any deprecated flags are used, let the user know and
-      # 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, exit
-      def verify_deprecated_flags!
-        DEPRECATED_FLAGS.each do |deprecated_key, deprecation_entry|
-          new_key, deprecated_long, replacement_value, deprecated_short = deprecation_entry
-
-          dep_long_key = deprecated_long.split(" ").first
-          if deprecated_short
-            dep_long_key += "(or #{deprecated_short.split(" ").first})"
-          end
-
-          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]
-              new_long_key = new_long.split(" ").first
-              if new_short = options[new_key][:short]
-                new_long += "(or #{new_short})"
-                new_long_key += "(or #{new_short.split(" ").first})"
-              end
-              ui.error <<~EOM
-                You provided both #{new_long_key} and #{dep_long_key}.
-
-                Please use one or the other, but note that
-                #{deprecated_long} is deprecated.
-              EOM
-              exit 1
-            else
-              config[new_key] = replacement_value || config[deprecated_key]
-              unless Chef::Config[:silence_deprecation_warnings] == true
-                ui.warn "You provided #{dep_long_key}. #{options[deprecated_key][:description]}"
-              end
-            end
-          end
-        end
-      end
-
       # fail if the server_name is nil
       def validate_name_args!
         if server_name.nil?
diff --git a/spec/unit/knife/bootstrap_spec.rb b/spec/unit/knife/bootstrap_spec.rb
index b04269e3df..bf2c7c4902 100644
--- a/spec/unit/knife/bootstrap_spec.rb
+++ b/spec/unit/knife/bootstrap_spec.rb
@@ -1657,7 +1657,6 @@ describe Chef::Knife::Bootstrap do
   describe "#run" do
     it "performs the steps we expect to run a bootstrap" do
       expect(knife).to receive(:check_license)
-      expect(knife).to receive(:verify_deprecated_flags!).ordered
       expect(knife).to receive(:validate_name_args!).ordered
       expect(knife).to receive(:validate_protocol!).ordered
       expect(knife).to receive(:validate_first_boot_attributes!).ordered
@@ -1679,106 +1678,6 @@ describe Chef::Knife::Bootstrap do
     end
   end
 
-  describe "#verify_deprecated_flags!" do
-    before do
-      Chef::Config[:silence_deprecation_warnings] = false
-    end
-
-    context "when a deprecated CLI flag is given on the CLI" do
-      context "with flag containing only long argument" do
-        context "when given with no other options" do
-          let(:bootstrap_cli_options) { %w{--winrm-transport XXX} }
-          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("You provided --winrm-transport. This flag is deprecated. Please use '--winrm-ssl' instead.")
-            knife.verify_deprecated_flags!
-            expect(knife.config[:winrm_ssl]).to eq "XXX"
-          end
-        end
-        context "when given along with valid flag" do
-          let(:bootstrap_cli_options) { %w{--winrm-transport XXX --connection-user user-a} }
-          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("You provided --winrm-transport. This flag is deprecated. Please use '--winrm-ssl' instead.")
-            knife.verify_deprecated_flags!
-            expect(knife.config[:winrm_ssl]).to eq "XXX"
-            expect(knife.config[:connection_user]).to eq "user-a"
-          end
-        end
-        context "when given along with its replacement" do
-          let(:bootstrap_cli_options) { %w{--winrm-transport XXX --winrm-ssl YYY} }
-          it "informs the human that both are provided and exits" do
-            expect(knife.ui).to receive(:error).with(/You provided both --winrm-ssl and --winrm-transport.*Please use.*/m)
-            expect { knife.verify_deprecated_flags! }.to raise_error SystemExit
-          end
-        end
-      end
-
-      context "with flag also contains short argument" do
-        context "when given as short arg" do
-          context "with no other options" do
-            let(:bootstrap_cli_options) { %w{-x user-a} }
-            it "maps the key value to the new key and display information with both long and short arguments" do
-              expect(knife.ui).to receive(:warn).with("You provided --winrm-user(or -x). This flag is deprecated. Please use '--connection-user USERNAME(or -U USERNAME)' instead.")
-              knife.verify_deprecated_flags!
-              expect(knife.config[:connection_user]).to eq "user-a"
-            end
-          end
-          context "along with valid flag" do
-            let(:bootstrap_cli_options) { %w{-x user-a --connection-password XXX} }
-            it "maps the key value to the new key and display information with both long and short arguments" do
-              expect(knife.ui).to receive(:warn).with("You provided --winrm-user(or -x). This flag is deprecated. Please use '--connection-user USERNAME(or -U USERNAME)' instead.")
-              knife.verify_deprecated_flags!
-              expect(knife.config[:connection_user]).to eq "user-a"
-              expect(knife.config[:connection_password]).to eq "XXX"
-            end
-          end
-          context "along with its replacement" do
-            let(:bootstrap_cli_options) { %w{-x user-a --connection-user user-b} }
-            it "informs the human that both are provided and exits" do
-              expect(knife.ui).to receive(:error).with("You provided both --connection-user(or -U) and --winrm-user(or -x).\n\nPlease use one or the other, but note that\n--winrm-user USERNAME is deprecated.\n")
-              expect { knife.verify_deprecated_flags! }.to raise_error SystemExit
-            end
-          end
-        end
-
-        context "when given as long arg" do
-          context "with no other options" do
-            let(:bootstrap_cli_options) { %w{--winrm-user user-a} }
-            it "maps the key value to the new key and display information with both long and short arguments" do
-              expect(knife.ui).to receive(:warn).with("You provided --winrm-user(or -x). This flag is deprecated. Please use '--connection-user USERNAME(or -U USERNAME)' instead.")
-              knife.verify_deprecated_flags!
-              expect(knife.config[:connection_user]).to eq "user-a"
-            end
-          end
-          context "along with valid flag" do
-            let(:bootstrap_cli_options) { %w{--winrm-user user-a --connection-password XXX} }
-            it "maps the key value to the new key and display information with both long and short arguments" do
-              expect(knife.ui).to receive(:warn).with("You provided --winrm-user(or -x). This flag is deprecated. Please use '--connection-user USERNAME(or -U USERNAME)' instead.")
-              knife.verify_deprecated_flags!
-              expect(knife.config[:connection_user]).to eq "user-a"
-              expect(knife.config[:connection_password]).to eq "XXX"
-            end
-          end
-          context "along with its replacement" do
-            let(:bootstrap_cli_options) { %w{--winrm-user user-a --connection-user user-b} }
-            it "informs the human that both are provided and exits" do
-              expect(knife.ui).to receive(:error).with("You provided both --connection-user(or -U) and --winrm-user(or -x).\n\nPlease use one or the other, but note that\n--winrm-user USERNAME is deprecated.\n")
-              expect { knife.verify_deprecated_flags! }.to raise_error SystemExit
-            end
-          end
-        end
-      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
     let(:vault_handler_mock) { double("ChefVaultHandler") }
     let(:client_builder_mock) { double("ClientBuilder") }