Allow optional signed header auth for data collector

* When the data collector is enabled but the token is nil, data
  collector will use signed header auth.
* Switch data collector to an HTTP client that includes the JSON content
  middlewares so both auth methods talk to the `http` object the same
  way.

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

2 files changed, 57 insertions, 3 deletions

diff --git a/lib/chef/data_collector.rb b/lib/chef/data_collector.rb
index 2aad0d74b0..f07f81c846 100644
--- a/lib/chef/data_collector.rb
+++ b/lib/chef/data_collector.rb
@@ -19,6 +19,8 @@
 #
 
 require "uri"
+require "chef/server_api"
+require "chef/http/simple_json"
 require "chef/event_dispatch/base"
 require "chef/data_collector/messages"
 require "chef/data_collector/resource_report"
@@ -65,8 +67,14 @@ class Chef
         @error_descriptions      = {}
         @expanded_run_list       = {}
         @deprecations            = Set.new
-        @http                    = Chef::HTTP.new(data_collector_server_url)
         @enabled                 = true
+
+        @http =
+          if data_collector_token.nil?
+            Chef::ServerAPI.new(data_collector_server_url)
+          else
+            Chef::HTTP::SimpleJSON.new(data_collector_server_url)
+          end
       end
 
       # see EventDispatch::Base#run_started
@@ -81,7 +89,7 @@ class Chef
 
         disable_reporter_on_error do
           send_to_data_collector(
-            Chef::DataCollector::Messages.run_start_message(current_run_status).to_json
+            Chef::DataCollector::Messages.run_start_message(current_run_status)
           )
         end
       end
diff --git a/spec/unit/data_collector_spec.rb b/spec/unit/data_collector_spec.rb
index 37df758ff2..25de91864a 100644
--- a/spec/unit/data_collector_spec.rb
+++ b/spec/unit/data_collector_spec.rb
@@ -23,6 +23,14 @@ require "chef/data_collector"
 require "chef/resource_builder"
 
 describe Chef::DataCollector do
+
+  # TODO:
+  # * auto-configure a URL that is relative to Chef Server URL,
+  #   like https://chef.example/organizations/:orgname/data_collector
+  # * Use an HTTP client that does signed header auth if no token is configured
+  # * register_reporter should be `true` for the auto-configure case
+  # * when talking to a server without automate/data collector, disabling the collector should not be noisy
+
   describe ".register_reporter?" do
     context "when no data collector URL is configured" do
       it "returns false" do
@@ -150,14 +158,52 @@ describe Chef::DataCollector do
       end
     end
   end
+
 end
 
 describe Chef::DataCollector::Reporter do
   let(:reporter) { described_class.new }
   let(:run_status) { Chef::RunStatus.new(Chef::Node.new, Chef::EventDispatch::Dispatcher.new) }
 
+  let(:token) { "supersecrettoken" }
+
   before do
     Chef::Config[:data_collector][:server_url] = "http://my-data-collector-server.mycompany.com"
+    Chef::Config[:data_collector][:token] = token
+  end
+
+  describe "selecting token or signed header authentication" do
+
+    context "when the token is set in the config" do
+
+      before do
+        Chef::Config[:client_key] = "/no/key/should/exist/at/this/path.pem"
+      end
+
+      it "configures an HTTP client that doesn't do signed header auth" do
+        # Initializing with the wrong kind of HTTP class should cause Chef::Exceptions::PrivateKeyMissing
+        expect { reporter.http }.to_not raise_error
+      end
+
+    end
+
+    context "when no token is set in the config" do
+
+      let(:token) { nil }
+
+      let(:client_key) { File.join(CHEF_SPEC_DATA, "ssl", "private_key.pem") }
+
+      before do
+        Chef::Config[:client_key] = client_key
+      end
+
+      it "configures an HTTP client that does signed header auth" do
+        expect { reporter.http }.to_not raise_error
+        expect(reporter.http.options).to have_key(:signing_key_filename)
+        expect(reporter.http.options[:signing_key_filename]).to eq(client_key)
+      end
+    end
+
   end
 
   describe "#run_started" do
@@ -177,7 +223,7 @@ describe Chef::DataCollector::Reporter do
         .to receive(:run_start_message)
         .with(run_status)
         .and_return(key: "value")
-      expect(reporter).to receive(:send_to_data_collector).with('{"key":"value"}')
+      expect(reporter).to receive(:send_to_data_collector).with({ key: "value" })
       reporter.run_started(run_status)
     end
   end