Disable data collector for solo if token isn't configured

Signed-off-by: Daniel DeLeo <dan@chef.io>

2 files changed, 108 insertions, 13 deletions

diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb
index 5d947d0ee8..e377e4ae12 100644
--- a/lib/chef/data_collector.rb
+++ b/lib/chef/data_collector.rb
@@ -33,12 +33,37 @@ class Chef
   # Provides methods for determinine whether a reporter should be registered.
   class DataCollector
 
-    # TODO: this probably should be false if running chef-solo and the user has
-    # not set a server_url, but maybe it will behave ok with the default?
+    # Whether or not to enable data collection:
+    # * always disabled for why run mode
+    # * disabled when the user sets `Chef::Config[:data_collector][:mode]` to a
+    #   value that excludes the mode (client or solo) that we are running as
+    # * disabled in solo mode if the user did not configure the auth token
+    # * disabled if `Chef::Config[:data_collector][:server_url]` is set to a
+    #   falsey value
     def self.register_reporter?
-      Chef::Config[:data_collector][:server_url] &&
-        !Chef::Config[:why_run] &&
-        self.reporter_enabled_for_current_mode?
+      return false if why_run?
+      return false unless reporter_enabled_for_current_mode?
+      if solo?
+        data_collector_url_configured? && token_auth_configured?
+      else
+        data_collector_url_configured?
+      end
+    end
+
+    def self.data_collector_url_configured?
+      !!Chef::Config[:data_collector][:server_url]
+    end
+
+    def self.why_run?
+      !!Chef::Config[:why_run]
+    end
+
+    def self.token_auth_configured?
+      !!Chef::Config[:data_collector][:token]
+    end
+
+    def self.solo?
+      !!Chef::Config[:solo] || !!Chef::Config[:local_mode]
     end
 
     def self.reporter_enabled_for_current_mode?
diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb
index 3d79c386fe..b28c953e13 100644
--- a/spec/unit/data_collector_spec.rb
+++ b/spec/unit/data_collector_spec.rb
@@ -52,23 +52,93 @@ describe Chef::DataCollector do
       end
 
       context "when not operating in why_run mode" do
+
         before do
           Chef::Config[:why_run] = false
+          Chef::Config[:data_collector][:token] = token
         end
 
-        context "when report is enabled for current mode" do
-          it "returns true" do
-            allow(Chef::DataCollector).to receive(:reporter_enabled_for_current_mode?).and_return(true)
-            expect(Chef::DataCollector.register_reporter?).to be_truthy
+        context "when a token is configured" do
+
+          let(:token) { "supersecrettoken" }
+
+          context "when report is enabled for current mode" do
+            it "returns true" do
+              allow(Chef::DataCollector).to receive(:reporter_enabled_for_current_mode?).and_return(true)
+              expect(Chef::DataCollector.register_reporter?).to be_truthy
+            end
           end
+
+          context "when report is disabled for current mode" do
+            it "returns false" do
+              allow(Chef::DataCollector).to receive(:reporter_enabled_for_current_mode?).and_return(false)
+              expect(Chef::DataCollector.register_reporter?).to be_falsey
+            end
+          end
+
         end
 
-        context "when report is disabled for current mode" do
-          it "returns false" do
-            allow(Chef::DataCollector).to receive(:reporter_enabled_for_current_mode?).and_return(false)
-            expect(Chef::DataCollector.register_reporter?).to be_falsey
+        # `Chef::Config[:data_collector][:server_url]` defaults to a URL
+        # relative to the `chef_server_url`, so we use configuration of the
+        # token to infer whether a solo/local mode user intends for data
+        # collection to be enabled.
+        context "when a token is not configured" do
+
+          let(:token) { nil }
+
+          context "when report is enabled for current mode" do
+
+            before do
+              allow(Chef::DataCollector).to receive(:reporter_enabled_for_current_mode?).and_return(true)
+            end
+
+            context "when the current mode is solo" do
+
+              before do
+                Chef::Config[:solo] = true
+              end
+
+              it "returns false" do
+                expect(Chef::DataCollector.register_reporter?).to be_falsey
+              end
+
+            end
+
+            context "when the current mode is local mode" do
+
+              before do
+                Chef::Config[:local_mode] = true
+              end
+
+              it "returns false" do
+                expect(Chef::DataCollector.register_reporter?).to be_falsey
+              end
+            end
+
+            context "when the current mode is client mode" do
+
+              before do
+                Chef::Config[:local_mode] = false
+                Chef::Config[:solo] = false
+              end
+
+              it "returns true" do
+                expect(Chef::DataCollector.register_reporter?).to be_truthy
+              end
+
+            end
+
+          end
+
+          context "when report is disabled for current mode" do
+            it "returns false" do
+              allow(Chef::DataCollector).to receive(:reporter_enabled_for_current_mode?).and_return(false)
+              expect(Chef::DataCollector.register_reporter?).to be_falsey
+            end
           end
+
         end
+
       end
     end
   end