From 481eba08b04f6fb661b9074268ec732c9f38e35e Mon Sep 17 00:00:00 2001 From: danielsdeleo Date: Wed, 11 Feb 2015 14:33:26 -0800 Subject: Reorganize contexts to better test compat vs. native mode behaviors --- spec/unit/policy_builder/policyfile_spec.rb | 175 ++++++++++++++++------------ 1 file changed, 100 insertions(+), 75 deletions(-) diff --git a/spec/unit/policy_builder/policyfile_spec.rb b/spec/unit/policy_builder/policyfile_spec.rb index 92cdd7f57e..ce0a24859a 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,66 +398,72 @@ 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]") + # 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("cookbooks/example1/#{example1_xyz_version}"). - 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("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) - 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 - 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 "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 "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 - 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 + end # shared_examples_for "fetching cookbooks" + context "when using compatibility mode (policy_document_native_api == false)" do + include_examples "fetching cookbooks" end end end -- cgit v1.2.1 From 3cf27d446c8f0777de4920fb065f7a8e6ce70261 Mon Sep 17 00:00:00 2001 From: danielsdeleo Date: Wed, 11 Feb 2015 14:49:40 -0800 Subject: Fetch cookbooks from cookbook_artifacts/ in native api mode --- lib/chef/policy_builder/policyfile.rb | 34 +++++++++++++++++++++++------ spec/unit/policy_builder/policyfile_spec.rb | 32 +++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb index ff8e067bf6..26c4f0c6c8 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,28 @@ class Chef Chef::Config end + private + + def compat_mode_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 + end + + def artifact_manifest_for(cookbook_name, lock_data) + xyz_version = lock_data["dotted_decimal_identifier"] + http_api.get("cookbook_artifacts/#{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 + end + end end end diff --git a/spec/unit/policy_builder/policyfile_spec.rb b/spec/unit/policy_builder/policyfile_spec.rb index ce0a24859a..8460fc593d 100644 --- a/spec/unit/policy_builder/policyfile_spec.rb +++ b/spec/unit/policy_builder/policyfile_spec.rb @@ -415,7 +415,7 @@ describe Chef::PolicyBuilder::Policyfile do policy_builder.load_node policy_builder.build_node - expect(http_api).to receive(:get).with("cookbooks/example1/#{example1_xyz_version}"). + expect(http_api).to receive(:get).with(cookbook1_url). and_raise(error404) end @@ -432,9 +432,9 @@ describe Chef::PolicyBuilder::Policyfile do policy_builder.load_node policy_builder.build_node - expect(http_api).to receive(:get).with("cookbooks/example1/#{example1_xyz_version}"). + expect(http_api).to receive(:get).with(cookbook1_url). and_return(example1_cookbook_object) - expect(http_api).to receive(:get).with("cookbooks/example2/#{example2_xyz_version}"). + expect(http_api).to receive(:get).with(cookbook2_url). and_return(example2_cookbook_object) allow(Chef::CookbookSynchronizer).to receive(:new). @@ -463,8 +463,32 @@ describe Chef::PolicyBuilder::Policyfile do end # shared_examples_for "fetching cookbooks" context "when using compatibility mode (policy_document_native_api == false)" do - include_examples "fetching cookbooks" + include_examples "fetching cookbooks" do + + let(:cookbook1_url) { "cookbooks/example1/#{example1_xyz_version}" } + let(:cookbook2_url) { "cookbooks/example2/#{example2_xyz_version}" } + + end + + 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 + + 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 -- cgit v1.2.1 From 728ce8d7ab8b83174b7d5ebabdd5aefb5600cdde Mon Sep 17 00:00:00 2001 From: danielsdeleo Date: Wed, 11 Feb 2015 15:21:06 -0800 Subject: Include relative URLs in error messages --- lib/chef/policy_builder/policyfile.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb index 26c4f0c6c8..d368b055f7 100644 --- a/lib/chef/policy_builder/policyfile.rb +++ b/lib/chef/policy_builder/policyfile.rb @@ -358,9 +358,10 @@ class Chef def compat_mode_manifest_for(cookbook_name, lock_data) xyz_version = lock_data["dotted_decimal_identifier"] - http_api.get("cookbooks/#{cookbook_name}/#{xyz_version}") + 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}: #{e.class} - #{e.message}" + 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 @@ -368,9 +369,10 @@ class Chef def artifact_manifest_for(cookbook_name, lock_data) xyz_version = lock_data["dotted_decimal_identifier"] - http_api.get("cookbook_artifacts/#{cookbook_name}/#{xyz_version}") + 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}: #{e.class} - #{e.message}" + 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 -- cgit v1.2.1 From 227859989606371d03aa991b46b73dbbca461bf5 Mon Sep 17 00:00:00 2001 From: danielsdeleo Date: Wed, 11 Feb 2015 15:22:30 -0800 Subject: Remove ruby 1.8-specific code from tests --- spec/unit/policy_builder/policyfile_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/unit/policy_builder/policyfile_spec.rb b/spec/unit/policy_builder/policyfile_spec.rb index 8460fc593d..8b6e928a46 100644 --- a/spec/unit/policy_builder/policyfile_spec.rb +++ b/spec/unit/policy_builder/policyfile_spec.rb @@ -406,12 +406,6 @@ describe Chef::PolicyBuilder::Policyfile do 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 -- cgit v1.2.1