Merge branch 'download-cookbooks-as-artifacts'

Closes #2897 w/ rebase

2 files changed, 144 insertions, 79 deletions

diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb
index ff8e067bf6..d368b055f7 100644
--- a/lib/chef/policy_builder/policyfile.rb
+++ b/lib/chef/policy_builder/policyfile.rb
@@ -335,13 +335,11 @@ class Chef
       # cookbooks are fetched by the "dotted_decimal_identifier": a
       # representation of a SHA1 in the traditional x.y.z version format.
       def manifest_for(cookbook_name, lock_data)
-        xyz_version = lock_data["dotted_decimal_identifier"]
-        http_api.get("cookbooks/#{cookbook_name}/#{xyz_version}")
-      rescue Exception => e
-        message = "Error loading cookbook #{cookbook_name} at version #{xyz_version}: #{e.class} - #{e.message}"
-        err = Chef::Exceptions::CookbookNotFound.new(message)
-        err.set_backtrace(e.backtrace)
-        raise err
+        if Chef::Config[:policy_document_native_api]
+          artifact_manifest_for(cookbook_name, lock_data)
+        else
+          compat_mode_manifest_for(cookbook_name, lock_data)
+        end
       end
 
       def cookbook_locks
@@ -356,6 +354,30 @@ class Chef
         Chef::Config
       end
 
+      private
+
+      def compat_mode_manifest_for(cookbook_name, lock_data)
+        xyz_version = lock_data["dotted_decimal_identifier"]
+        rel_url = "cookbooks/#{cookbook_name}/#{xyz_version}"
+        http_api.get(rel_url)
+      rescue Exception => e
+        message = "Error loading cookbook #{cookbook_name} at version #{xyz_version} from #{rel_url}: #{e.class} - #{e.message}"
+        err = Chef::Exceptions::CookbookNotFound.new(message)
+        err.set_backtrace(e.backtrace)
+        raise err
+      end
+
+      def artifact_manifest_for(cookbook_name, lock_data)
+        xyz_version = lock_data["dotted_decimal_identifier"]
+        rel_url = "cookbook_artifacts/#{cookbook_name}/#{xyz_version}"
+        http_api.get(rel_url)
+      rescue Exception => e
+        message = "Error loading cookbook #{cookbook_name} at version #{xyz_version} from #{rel_url}: #{e.class} - #{e.message}"
+        err = Chef::Exceptions::CookbookNotFound.new(message)
+        err.set_backtrace(e.backtrace)
+        raise err
+      end
+
     end
   end
 end
diff --git a/spec/unit/policy_builder/policyfile_spec.rb b/spec/unit/policy_builder/policyfile_spec.rb
index 92cdd7f57e..8b6e928a46 100644
--- a/spec/unit/policy_builder/policyfile_spec.rb
+++ b/spec/unit/policy_builder/policyfile_spec.rb
@@ -144,7 +144,7 @@ describe Chef::PolicyBuilder::Policyfile do
 
   end
 
-  describe "when using compatibility mode" do
+  describe "loading policy data" do
 
     let(:http_api) { double("Chef::REST") }
 
@@ -171,43 +171,64 @@ describe Chef::PolicyBuilder::Policyfile do
       allow(policy_builder).to receive(:http_api).and_return(http_api)
     end
 
-    context "when the deployment group cannot be loaded" do
-      let(:error404) { Net::HTTPServerException.new("404 message", :body) }
+    describe "when using compatibility mode (policy_document_native_api == false)" do
 
-      before do
-        expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
-        expect(http_api).to receive(:get).
-          with("data/policyfiles/example-policy-stage").
-          and_raise(error404)
-      end
+      context "when the deployment group cannot be loaded" do
+        let(:error404) { Net::HTTPServerException.new("404 message", :body) }
 
-      it "raises an error" do
-        expect { policy_builder.load_node }.to raise_error(err_namespace::ConfigurationError)
-      end
+        before do
+          expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
+          expect(http_api).to receive(:get).
+            with("data/policyfiles/example-policy-stage").
+            and_raise(error404)
+        end
 
-      it "sends error message to the event system" do
-        expect(events).to receive(:node_load_failed).with(node_name, an_instance_of(err_namespace::ConfigurationError), Chef::Config)
-        expect { policy_builder.load_node }.to raise_error(err_namespace::ConfigurationError)
-      end
+        it "raises an error" do
+          expect { policy_builder.load_node }.to raise_error(err_namespace::ConfigurationError)
+        end
 
-    end
+        it "sends error message to the event system" do
+          expect(events).to receive(:node_load_failed).with(node_name, an_instance_of(err_namespace::ConfigurationError), Chef::Config)
+          expect { policy_builder.load_node }.to raise_error(err_namespace::ConfigurationError)
+        end
 
-    describe "when the deployment_group is not configured" do
-      before do
-        Chef::Config[:deployment_group] = nil
-        expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
       end
 
-      it "errors while loading the node" do
-        expect { policy_builder.load_node }.to raise_error(err_namespace::ConfigurationError)
+      context "when the deployment_group is not configured" do
+        before do
+          Chef::Config[:deployment_group] = nil
+          expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
+        end
+
+        it "errors while loading the node" do
+          expect { policy_builder.load_node }.to raise_error(err_namespace::ConfigurationError)
+        end
+
+
+        it "passes error information to the event system" do
+          # TODO: also make sure something acceptable happens with the error formatters
+          err_class = err_namespace::ConfigurationError
+          expect(events).to receive(:node_load_failed).with(node_name, an_instance_of(err_class), Chef::Config)
+          expect { policy_builder.load_node }.to raise_error(err_class)
+        end
       end
 
+      context "when deployment_group is correctly configured" do
+
+        let(:policy_relative_url) { "data/policyfiles/example-policy-stage" }
+
+        before do
+          expect(http_api).to receive(:get).with(policy_relative_url).and_return(parsed_policyfile_json)
+        end
+
+        it "fetches the policy file from a data bag item" do
+          expect(policy_builder.policy).to eq(parsed_policyfile_json)
+        end
+
+        it "extracts the run_list from the policyfile" do
+          expect(policy_builder.run_list).to eq(policyfile_run_list)
+        end
 
-      it "passes error information to the event system" do
-        # TODO: also make sure something acceptable happens with the error formatters
-        err_class = err_namespace::ConfigurationError
-        expect(events).to receive(:node_load_failed).with(node_name, an_instance_of(err_class), Chef::Config)
-        expect { policy_builder.load_node }.to raise_error(err_class)
       end
     end
 
@@ -253,12 +274,10 @@ describe Chef::PolicyBuilder::Policyfile do
     end
 
 
-    context "and a deployment_group is configured" do
-
-      let(:policy_relative_url) { "data/policyfiles/example-policy-stage" }
+    describe "building policy from the policyfile" do
 
       before do
-        expect(http_api).to receive(:get).with(policy_relative_url).and_return(parsed_policyfile_json)
+        allow(policy_builder).to receive(:policy).and_return(parsed_policyfile_json)
       end
 
       it "fetches the policy file from a data bag item" do
@@ -379,67 +398,91 @@ describe Chef::PolicyBuilder::Policyfile do
 
         let(:cookbook_synchronizer) { double("Chef::CookbookSynchronizer") }
 
-        context "and a cookbook is missing" do
+        shared_examples_for "fetching cookbooks" do
+          context "and a cookbook is missing" do
 
-          let(:error404) { Net::HTTPServerException.new("404 message", :body) }
+            let(:error404) { Net::HTTPServerException.new("404 message", :body) }
 
-          before do
-            expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
+            before do
+              expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
 
-            # Remove references to example2 cookbook because we're iterating
-            # over a Hash data structure and on ruby 1.8.7 iteration order will
-            # not be stable.
-            parsed_policyfile_json["cookbook_locks"].delete("example2")
-            parsed_policyfile_json["run_list"].delete("recipe[example2::server]")
+              policy_builder.load_node
+              policy_builder.build_node
 
-            policy_builder.load_node
-            policy_builder.build_node
+              expect(http_api).to receive(:get).with(cookbook1_url).
+                and_raise(error404)
+            end
 
-            expect(http_api).to receive(:get).with("cookbooks/example1/#{example1_xyz_version}").
-              and_raise(error404)
-          end
+            it "raises an error indicating which cookbook is missing" do
+              expect { policy_builder.cookbooks_to_sync }.to raise_error(Chef::Exceptions::CookbookNotFound)
+            end
 
-          it "raises an error indicating which cookbook is missing" do
-            expect { policy_builder.cookbooks_to_sync }.to raise_error(Chef::Exceptions::CookbookNotFound)
           end
 
-        end
+          context "and the cookbooks can be fetched" do
+            before do
+              expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
 
-        context "and the cookbooks can be fetched" do
-          before do
-            expect(Chef::Node).to receive(:find_or_create).with(node_name).and_return(node)
+              policy_builder.load_node
+              policy_builder.build_node
 
-            policy_builder.load_node
-            policy_builder.build_node
+              expect(http_api).to receive(:get).with(cookbook1_url).
+                and_return(example1_cookbook_object)
+              expect(http_api).to receive(:get).with(cookbook2_url).
+                and_return(example2_cookbook_object)
 
-            expect(http_api).to receive(:get).with("cookbooks/example1/#{example1_xyz_version}").
-              and_return(example1_cookbook_object)
-            expect(http_api).to receive(:get).with("cookbooks/example2/#{example2_xyz_version}").
-              and_return(example2_cookbook_object)
+              allow(Chef::CookbookSynchronizer).to receive(:new).
+                with(expected_cookbook_hash, events).
+                and_return(cookbook_synchronizer)
+            end
+
+            it "builds a Hash of the form 'cookbook_name' => Chef::CookbookVersion" do
+              expect(policy_builder.cookbooks_to_sync).to eq(expected_cookbook_hash)
+            end
+
+            it "syncs the desired cookbooks via CookbookSynchronizer" do
+              expect(cookbook_synchronizer).to receive(:sync_cookbooks)
+              policy_builder.sync_cookbooks
+            end
+
+            it "builds a run context" do
+              expect(cookbook_synchronizer).to receive(:sync_cookbooks)
+              expect_any_instance_of(Chef::RunContext).to receive(:load).with(policy_builder.run_list_expansion_ish)
+              run_context = policy_builder.setup_run_context
+              expect(run_context.node).to eq(node)
+              expect(run_context.cookbook_collection.keys).to match_array(["example1", "example2"])
+            end
 
-            allow(Chef::CookbookSynchronizer).to receive(:new).
-              with(expected_cookbook_hash, events).
-              and_return(cookbook_synchronizer)
           end
+        end # shared_examples_for "fetching cookbooks"
+
+        context "when using compatibility mode (policy_document_native_api == false)" do
+          include_examples "fetching cookbooks" do
+
+            let(:cookbook1_url) { "cookbooks/example1/#{example1_xyz_version}" }
+            let(:cookbook2_url) { "cookbooks/example2/#{example2_xyz_version}" }
 
-          it "builds a Hash of the form 'cookbook_name' => Chef::CookbookVersion" do
-            expect(policy_builder.cookbooks_to_sync).to eq(expected_cookbook_hash)
           end
 
-          it "syncs the desired cookbooks via CookbookSynchronizer" do
-            expect(cookbook_synchronizer).to receive(:sync_cookbooks)
-            policy_builder.sync_cookbooks
+        end
+
+        context "when using native API mode (policy_document_native_api == true)" do
+
+          before do
+            Chef::Config[:policy_document_native_api] = true
+            Chef::Config[:policy_group] = "policy-stage"
+            Chef::Config[:policy_name] = "example"
           end
 
-          it "builds a run context" do
-            expect(cookbook_synchronizer).to receive(:sync_cookbooks)
-            expect_any_instance_of(Chef::RunContext).to receive(:load).with(policy_builder.run_list_expansion_ish)
-            run_context = policy_builder.setup_run_context
-            expect(run_context.node).to eq(node)
-            expect(run_context.cookbook_collection.keys).to match_array(["example1", "example2"])
+          include_examples "fetching cookbooks" do
+
+            let(:cookbook1_url) { "cookbook_artifacts/example1/#{example1_xyz_version}" }
+            let(:cookbook2_url) { "cookbook_artifacts/example2/#{example2_xyz_version}" }
+
           end
 
         end
+
       end
     end