diff options
author | Prajakta Purohit <prajakta@opscode.com> | 2016-07-14 18:19:56 -0700 |
---|---|---|
committer | Prajakta Purohit <prajakta@opscode.com> | 2016-07-22 12:28:14 -0700 |
commit | 65e21d5093a8ac5e97a2594bb172bf0c40cdce2e (patch) | |
tree | 4498821575987ed7883922218a0ba0504b9064a4 | |
parent | 1aadc96bcb855fe665cad4b4004ee657d241e5ec (diff) | |
download | chef-praj/FLOW-339/checksums.tar.gz |
Verify cookbook checksum before downloadpraj/FLOW-339/checksums
-rw-r--r-- | lib/chef/cookbook/synchronizer.rb | 17 | ||||
-rw-r--r-- | lib/chef/exceptions.rb | 6 | ||||
-rw-r--r-- | spec/unit/cookbook/synchronizer_spec.rb | 69 |
3 files changed, 86 insertions, 6 deletions
diff --git a/lib/chef/cookbook/synchronizer.rb b/lib/chef/cookbook/synchronizer.rb index 1ee30bacc7..64e461890c 100644 --- a/lib/chef/cookbook/synchronizer.rb +++ b/lib/chef/cookbook/synchronizer.rb @@ -252,7 +252,7 @@ class Chef # (remote, per manifest), do the update. This will also execute if there # is no current checksum. if !cached_copy_up_to_date?(cache_filename, file.manifest_record["checksum"]) - download_file(file.manifest_record["url"], cache_filename) + download_file(file, cache_filename) @events.updated_cookbook_file(file.cookbook.name, cache_filename) else Chef::Log.debug("Not storing #{cache_filename}, as the cache is up to date.") @@ -278,11 +278,16 @@ class Chef # Unconditionally download the file from the given URL. File will be # downloaded to the path +destination+ which is relative to the Chef file # cache root. - def download_file(url, destination) - raw_file = server_api.streaming_request(url) - - Chef::Log.info("Storing updated #{destination} in the cache.") - cache.move_to(raw_file.path, destination) + def download_file(file, destination) + raw_file = server_api.streaming_request(file.manifest_record["url"]) + raw_file_md5 = Chef::Digester.generate_md5_checksum_for_file(raw_file.path) + Chef::Log.debug("The md5 hash of the cookbook file to be served: #{raw_file_md5}") + if raw_file_md5 == file.manifest_record["checksum"] + Chef::Log.debug("Storing updated #{destination} in the cache.") + cache.move_to(raw_file.path, destination) + else + raise Chef::Exceptions::FileIntegrityCompromise.new(raw_file.path, file.manifest_record["checksum"], raw_file_md5) + end end # Marks the given file as valid (non-stale). diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index 43759568a7..d80c0964ec 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -520,5 +520,11 @@ This error is most often caused by network issues (proxies, etc) outside of chef super "Found multiple matching resources. #{matches_info.join("\n")}" end end + + class FileIntegrityCompromise < RuntimeError + def initialize(file, orig_chksum, current_chksum) + super "For the file: #{File.basename(file)} original checksum: (#{orig_chksum}) does not match existing content checksum: (#{current_chksum})" + end + end end end diff --git a/spec/unit/cookbook/synchronizer_spec.rb b/spec/unit/cookbook/synchronizer_spec.rb index 3f5624f3b0..e6012cf6c7 100644 --- a/spec/unit/cookbook/synchronizer_spec.rb +++ b/spec/unit/cookbook/synchronizer_spec.rb @@ -213,6 +213,36 @@ describe Chef::CookbookSynchronizer do :path => "/tmp/cookbook_a_template_default_tempfile") end + def setup_common_files_missing_expectations_for_downloads + # Files are not in the cache: + expect(file_cache).to receive(:has_key?). + with("cookbooks/cookbook_a/recipes/default.rb"). + and_return(false) + expect(file_cache).to receive(:has_key?). + with("cookbooks/cookbook_a/attributes/default.rb"). + and_return(false) + + # Fetch and copy default.rb recipe + expect(server_api).to receive(:streaming_request). + with("http://chef.example.com/abc123"). + and_return(cookbook_a_default_recipe_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_default_recipe_tempfile.path). + and_return("xyz123") + expect(file_cache).not_to receive(:move_to) + expect(file_cache).not_to receive(:load) + + # Fetch and copy default.rb attribute file + expect(server_api).to receive(:streaming_request). + with("http://chef.example.com/abc456"). + and_return(cookbook_a_default_attribute_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_default_attribute_tempfile.path). + and_return("xyz456") + expect(file_cache).not_to receive(:move_to) + expect(file_cache).not_to receive(:load) + end + def setup_common_files_missing_expectations # Files are not in the cache: expect(file_cache).to receive(:has_key?). @@ -226,6 +256,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/abc123"). and_return(cookbook_a_default_recipe_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_default_recipe_tempfile.path). + and_return("abc123") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_recipes_default_rb", "cookbooks/cookbook_a/recipes/default.rb") expect(file_cache).to receive(:load). @@ -236,6 +269,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/abc456"). and_return(cookbook_a_default_attribute_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_default_attribute_tempfile.path). + and_return("abc456") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_attributes_default_rb", "cookbooks/cookbook_a/attributes/default.rb") expect(file_cache).to receive(:load). @@ -254,6 +290,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/megaman.conf"). and_return(cookbook_a_file_default_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_file_default_tempfile.path). + and_return("abc124") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_file_default_tempfile", "cookbooks/cookbook_a/files/default/megaman.conf") expect(file_cache).to receive(:load). @@ -263,6 +302,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/ffffff"). and_return(cookbook_a_template_default_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_template_default_tempfile.path). + and_return("abc125") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_template_default_tempfile", "cookbooks/cookbook_a/templates/default/apache2.conf.erb") expect(file_cache).to receive(:load). @@ -283,6 +325,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/abc123"). and_return(cookbook_a_default_recipe_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_default_recipe_tempfile.path). + and_return("abc123") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_recipes_default_rb", "cookbooks/cookbook_a/recipes/default.rb") expect(file_cache).to receive(:load). @@ -299,6 +344,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/abc456"). and_return(cookbook_a_default_attribute_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_default_attribute_tempfile.path). + and_return("abc456") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_attributes_default_rb", "cookbooks/cookbook_a/attributes/default.rb") expect(file_cache).to receive(:load). @@ -325,6 +373,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/megaman.conf"). and_return(cookbook_a_file_default_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_file_default_tempfile.path). + and_return("abc124") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_file_default_tempfile", "cookbooks/cookbook_a/files/default/megaman.conf") expect(file_cache).to receive(:load). @@ -336,6 +387,9 @@ describe Chef::CookbookSynchronizer do expect(server_api).to receive(:streaming_request). with("http://chef.example.com/ffffff"). and_return(cookbook_a_template_default_tempfile) + expect(Chef::Digester).to receive(:generate_md5_checksum_for_file). + with(cookbook_a_template_default_tempfile.path). + and_return("abc125") expect(file_cache).to receive(:move_to). with("/tmp/cookbook_a_template_default_tempfile", "cookbooks/cookbook_a/templates/default/apache2.conf.erb") expect(file_cache).to receive(:load). @@ -427,6 +481,17 @@ describe Chef::CookbookSynchronizer do allow(synchronizer).to receive(:cache).and_return(file_cache) end + context "when the cookbook checksum does not match the original checksum" do + before do + setup_common_files_missing_expectations_for_downloads + end + let(:no_lazy_load) { false } + + it "does not download cookbook and raise error" do + expect { synchronizer.sync_cookbooks }.to raise_error(Chef::Exceptions::FileIntegrityCompromise) + end + end + context "when the cache does not contain the desired files" do before do setup_common_files_missing_expectations @@ -439,6 +504,10 @@ describe Chef::CookbookSynchronizer do synchronizer.sync_cookbooks end + it "downloads cookbooks if checksum matches" do + expect { synchronizer.sync_cookbooks }.not_to raise_error + end + it "does not fetch templates or cookbook files" do # Implicitly tested in previous test; this test is just for behavior specification. expect(server_api).not_to receive(:streaming_request). |