Move the data collector should_be_enabled? checklcg/move-dc-check

This works better as part of the module that handles all the config
wrangling.  It isn't central to the data_collector operation itself.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

3 files changed, 61 insertions, 60 deletions

diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb
index 677dc99faf..b976f9d058 100644
--- a/lib/chef/data_collector.rb
+++ b/lib/chef/data_collector.rb
@@ -75,7 +75,7 @@ class Chef
       # (see EventDispatch::Base#run_start)
       #
       def run_start(chef_version, run_status)
-        events.unregister(self) unless should_be_enabled?
+        events.unregister(self) unless Chef::DataCollector::ConfigValidation.should_be_enabled?
         @run_status = run_status
       end
 
@@ -277,48 +277,6 @@ class Chef
 
         headers
       end
-
-      # Main logic controlling the data collector being enabled or disabled:
-      #
-      # * disabled in why-run mode
-      # * disabled when `Chef::Config[:data_collector][:mode]` excludes the solo-vs-client mode
-      # * disabled if there is no server_url or no output_locations to log to
-      # * enabled if there is a configured output_location even without a token
-      # * disabled in solo mode if the user did not configure the auth token
-      #
-      # @return [Boolean] true if the data collector should be enabled
-      #
-      def should_be_enabled?
-        running_mode = ( Chef::Config[:solo_legacy_mode] || Chef::Config[:local_mode] ) ? :solo : :client
-        want_mode = Chef::Config[:data_collector][:mode]
-
-        case
-        when Chef::Config[:why_run]
-          Chef::Log.trace("data collector is disabled for why run mode")
-          return false
-        when (want_mode != :both) && running_mode != want_mode
-          Chef::Log.trace("data collector is configured to only run in #{Chef::Config[:data_collector][:mode]} modes, disabling it")
-          return false
-        when !(Chef::Config[:data_collector][:server_url] || Chef::Config[:data_collector][:output_locations])
-          Chef::Log.trace("Neither data collector URL or output locations have been configured, disabling data collector")
-          return false
-        when running_mode == :client && Chef::Config[:data_collector][:token]
-          Chef::Log.warn("Data collector token authentication is not recommended for client-server mode. " \
-                         "Please upgrade #{Chef::Dist::SERVER_PRODUCT} to 12.11 or later and remove the token from your config file " \
-                         "to use key based authentication instead")
-          return true
-        when Chef::Config[:data_collector][:output_locations] && Chef::Config[:data_collector][:output_locations][:files] && !Chef::Config[:data_collector][:output_locations][:files].empty?
-          # we can run fine to a file without a token, even in solo mode.
-          return true
-        when running_mode == :solo && !Chef::Config[:data_collector][:token]
-          # we are in solo mode and are not logging to a file, so must have a token
-          Chef::Log.trace("Data collector token must be configured to use #{Chef::Dist::AUTOMATE} data collector with #{Chef::Dist::SOLO}")
-          return false
-        else
-          return true
-        end
-      end
-
     end
   end
 end
diff --git a/lib/chef/data_collector/config_validation.rb b/lib/chef/data_collector/config_validation.rb
index 72cc121df4..8bc409f03c 100644
--- a/lib/chef/data_collector/config_validation.rb
+++ b/lib/chef/data_collector/config_validation.rb
@@ -19,6 +19,8 @@ require "uri" unless defined?(URI)
 
 class Chef
   class DataCollector
+
+    # @api private
     module ConfigValidation
       class << self
         def validate_server_url!
@@ -58,6 +60,47 @@ class Chef
           end
         end
 
+        # Main logic controlling the data collector being enabled or disabled:
+        #
+        # * disabled in why-run mode
+        # * disabled when `Chef::Config[:data_collector][:mode]` excludes the solo-vs-client mode
+        # * disabled if there is no server_url or no output_locations to log to
+        # * enabled if there is a configured output_location even without a token
+        # * disabled in solo mode if the user did not configure the auth token
+        #
+        # @return [Boolean] true if the data collector should be enabled
+        #
+        def should_be_enabled?
+          running_mode = ( Chef::Config[:solo_legacy_mode] || Chef::Config[:local_mode] ) ? :solo : :client
+          want_mode = Chef::Config[:data_collector][:mode]
+
+          case
+          when Chef::Config[:why_run]
+            Chef::Log.trace("data collector is disabled for why run mode")
+            return false
+          when (want_mode != :both) && running_mode != want_mode
+            Chef::Log.trace("data collector is configured to only run in #{Chef::Config[:data_collector][:mode]} modes, disabling it")
+            return false
+          when !(Chef::Config[:data_collector][:server_url] || Chef::Config[:data_collector][:output_locations])
+            Chef::Log.trace("Neither data collector URL or output locations have been configured, disabling data collector")
+            return false
+          when running_mode == :client && Chef::Config[:data_collector][:token]
+            Chef::Log.warn("Data collector token authentication is not recommended for client-server mode. " \
+                           "Please upgrade #{Chef::Dist::SERVER_PRODUCT} to 12.11 or later and remove the token from your config file " \
+                           "to use key based authentication instead")
+            return true
+          when Chef::Config[:data_collector][:output_locations] && Chef::Config[:data_collector][:output_locations][:files] && !Chef::Config[:data_collector][:output_locations][:files].empty?
+            # we can run fine to a file without a token, even in solo mode.
+            return true
+          when running_mode == :solo && !Chef::Config[:data_collector][:token]
+            # we are in solo mode and are not logging to a file, so must have a token
+            Chef::Log.trace("Data collector token must be configured to use #{Chef::Dist::AUTOMATE} data collector with #{Chef::Dist::SOLO}")
+            return false
+          else
+            return true
+          end
+        end
+
         private
 
         # validate an output_location file
diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb
index fed0e54b45..d7b18b3c28 100644
--- a/spec/unit/data_collector_spec.rb
+++ b/spec/unit/data_collector_spec.rb
@@ -265,77 +265,77 @@ describe Chef::DataCollector do
   describe "#should_be_enabled?" do
     shared_examples_for "a solo-like run" do
       it "is disabled in solo-legacy without a data_collector url and token" do
-        expect(data_collector.send(:should_be_enabled?)).to be false
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
       end
 
       it "is disabled in solo-legacy with only a url" do
         Chef::Config[:data_collector][:server_url] = "https://www.esa.local/ariane5"
-        expect(data_collector.send(:should_be_enabled?)).to be false
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
       end
 
       it "is disabled in solo-legacy with only a token" do
         Chef::Config[:data_collector][:token] = "admit_one"
-        expect(data_collector.send(:should_be_enabled?)).to be false
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
       end
 
       it "is enabled in solo-legacy with both a token and url" do
         Chef::Config[:data_collector][:server_url] = "https://www.esa.local/ariane5"
         Chef::Config[:data_collector][:token] = "no_cash_value"
-        expect(data_collector.send(:should_be_enabled?)).to be true
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
       end
 
       it "is enabled in solo-legacy with only an output location to a file" do
         Chef::Config[:data_collector][:output_locations] = { files: [ "/always/be/counting/down" ] }
-        expect(data_collector.send(:should_be_enabled?)).to be true
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
       end
 
       it "is disabled in solo-legacy with only an output location to a uri" do
         Chef::Config[:data_collector][:output_locations] = { urls: [ "https://esa.local/ariane5" ] }
-        expect(data_collector.send(:should_be_enabled?)).to be false
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
       end
 
       it "is enabled in solo-legacy with only an output location to a uri with a token" do
         Chef::Config[:data_collector][:output_locations] = { urls: [ "https://esa.local/ariane5" ] }
         Chef::Config[:data_collector][:token] = "good_for_one_fare"
-        expect(data_collector.send(:should_be_enabled?)).to be true
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
       end
 
       it "is enabled in solo-legacy when the mode is :solo" do
         Chef::Config[:data_collector][:server_url] = "https://www.esa.local/ariane5"
         Chef::Config[:data_collector][:token] = "non_redeemable"
         Chef::Config[:data_collector][:mode] = :solo
-        expect(data_collector.send(:should_be_enabled?)).to be true
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
       end
 
       it "is enabled in solo-legacy when the mode is :both" do
         Chef::Config[:data_collector][:server_url] = "https://www.esa.local/ariane5"
         Chef::Config[:data_collector][:token] = "non_negotiable"
         Chef::Config[:data_collector][:mode] = :both
-        expect(data_collector.send(:should_be_enabled?)).to be true
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
       end
 
       it "is disabled in solo-legacy when the mode is :client" do
         Chef::Config[:data_collector][:server_url] = "https://www.esa.local/ariane5"
         Chef::Config[:data_collector][:token] = "NYCTA"
         Chef::Config[:data_collector][:mode] = :client
-        expect(data_collector.send(:should_be_enabled?)).to be false
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
       end
 
       it "is disabled in solo-legacy mode when the mode is :nonsense" do
         Chef::Config[:data_collector][:server_url] = "https://www.esa.local/ariane5"
         Chef::Config[:data_collector][:token] = "MTA"
         Chef::Config[:data_collector][:mode] = :nonsense
-        expect(data_collector.send(:should_be_enabled?)).to be false
+        expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
       end
     end
 
     it "by default it is enabled" do
-      expect(data_collector.send(:should_be_enabled?)).to be true
+      expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
     end
 
     it "is disabled in why-run" do
       Chef::Config[:why_run] = true
-      expect(data_collector.send(:should_be_enabled?)).to be false
+      expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
     end
 
     describe "a solo legacy run" do
@@ -356,22 +356,22 @@ describe Chef::DataCollector do
 
     it "is enabled in client mode when the mode is :both" do
       Chef::Config[:data_collector][:mode] = :both
-      expect(data_collector.send(:should_be_enabled?)).to be true
+      expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
     end
 
     it "is disabled in client mode when the mode is :solo" do
       Chef::Config[:data_collector][:mode] = :solo
-      expect(data_collector.send(:should_be_enabled?)).to be false
+      expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
     end
 
     it "is disabled in client mode when the mode is :nonsense" do
       Chef::Config[:data_collector][:mode] = :nonsense
-      expect(data_collector.send(:should_be_enabled?)).to be false
+      expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be false
     end
 
     it "is still enabled if you set a token in client mode" do
       Chef::Config[:data_collector][:token] =  "good_for_one_ride"
-      expect(data_collector.send(:should_be_enabled?)).to be true
+      expect(Chef::DataCollector::ConfigValidation.should_be_enabled?).to be true
     end
   end