diff options
author | Thom May <thom@may.lt> | 2015-11-12 10:10:04 +0000 |
---|---|---|
committer | Thom May <thom@may.lt> | 2015-11-12 10:10:04 +0000 |
commit | 880f744ce9f6cef12b7bdbd746641b87a27f5809 (patch) | |
tree | 5c89d827589f27d8bc5173ac161fb001f4081f6c | |
parent | 1486782645f7e8eb2fcd10116b2eacd2af594c87 (diff) | |
parent | 7b9726f50abaf05ddb2c4ab215b9e265138ca063 (diff) | |
download | chef-880f744ce9f6cef12b7bdbd746641b87a27f5809.tar.gz |
Merge pull request #3991 from chef/jdm/remote-file-sha2
Modify remote_file cache_control_data to use sha256 for its name
-rw-r--r-- | lib/chef/provider/remote_file/cache_control_data.rb | 41 | ||||
-rw-r--r-- | spec/unit/provider/remote_file/cache_control_data_spec.rb | 98 |
2 files changed, 99 insertions, 40 deletions
diff --git a/lib/chef/provider/remote_file/cache_control_data.rb b/lib/chef/provider/remote_file/cache_control_data.rb index f9b729362c..3f39dac625 100644 --- a/lib/chef/provider/remote_file/cache_control_data.rb +++ b/lib/chef/provider/remote_file/cache_control_data.rb @@ -145,18 +145,51 @@ class Chef end def load_json_data - Chef::FileCache.load("remote_file/#{sanitized_cache_file_basename}") + path = sanitized_cache_file_path(sanitized_cache_file_basename) + if Chef::FileCache.has_key?(path) + Chef::FileCache.load(path) + else + old_path = sanitized_cache_file_path(sanitized_cache_file_basename_md5) + if Chef::FileCache.has_key?(old_path) + # We found an old cache control data file. We started using sha256 instead of md5 + # to name these. Upgrade the file to the new name. + Chef::Log.debug("Found old cache control data file at #{old_path}. Moving to #{path}.") + Chef::FileCache.load(old_path).tap do |data| + Chef::FileCache.store(path, data) + Chef::FileCache.delete(old_path) + end + else + raise Chef::Exceptions::FileNotFound + end + end end - def sanitized_cache_file_basename + def sanitized_cache_file_path(basename) + "remote_file/#{basename}" + end + + def scrubbed_uri # Scrub and truncate in accordance with the goals of keeping the name # human-readable but within the bounds of local file system # path length limits - scrubbed_uri = uri.gsub(/\W/, '_')[0..63] + uri.gsub(/\W/, '_')[0..63] + end + + def sanitized_cache_file_basename + uri_sha2 = Chef::Digester.instance.generate_checksum(StringIO.new(uri)) + cache_file_basename(uri_sha2[0,32]) + end + + + def sanitized_cache_file_basename_md5 + # Old way of creating the file basename uri_md5 = Chef::Digester.instance.generate_md5_checksum(StringIO.new(uri)) - "#{scrubbed_uri}-#{uri_md5}.json" + cache_file_basename(uri_md5) end + def cache_file_basename(checksum) + "#{scrubbed_uri}-#{checksum}.json" + end end end end diff --git a/spec/unit/provider/remote_file/cache_control_data_spec.rb b/spec/unit/provider/remote_file/cache_control_data_spec.rb index 11f2af3edc..c154c4228e 100644 --- a/spec/unit/provider/remote_file/cache_control_data_spec.rb +++ b/spec/unit/provider/remote_file/cache_control_data_spec.rb @@ -20,12 +20,12 @@ require 'spec_helper' require 'uri' CACHE_FILE_TRUNCATED_FRIENDLY_FILE_NAME_LENGTH = 64 -CACHE_FILE_MD5_HEX_LENGTH = 32 +CACHE_FILE_CHECKSUM_HEX_LENGTH = 32 CACHE_FILE_JSON_FILE_EXTENSION_LENGTH = 5 CACHE_FILE_PATH_LIMIT = CACHE_FILE_TRUNCATED_FRIENDLY_FILE_NAME_LENGTH + 1 + - CACHE_FILE_MD5_HEX_LENGTH + + CACHE_FILE_CHECKSUM_HEX_LENGTH + CACHE_FILE_JSON_FILE_EXTENSION_LENGTH # {friendly}-{md5hex}.json == 102 describe Chef::Provider::RemoteFile::CacheControlData do @@ -36,7 +36,8 @@ describe Chef::Provider::RemoteFile::CacheControlData do Chef::Provider::RemoteFile::CacheControlData.load_and_validate(uri, current_file_checksum) end - let(:cache_path) { "remote_file/http___www_google_com_robots_txt-9839677abeeadf0691026e0cabca2339.json" } + let(:cache_path) { "remote_file/http___www_google_com_robots_txt-6dc1b24315d0cff764d30344199c6f7b.json" } + let(:old_cache_path) { "remote_file/http___www_google_com_robots_txt-9839677abeeadf0691026e0cabca2339.json" } # the checksum of the file we have on disk already let(:current_file_checksum) { "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" } @@ -44,7 +45,8 @@ describe Chef::Provider::RemoteFile::CacheControlData do context "when loading data for an unknown URI" do before do - expect(Chef::FileCache).to receive(:load).with(cache_path).and_raise(Chef::Exceptions::FileNotFound, "nope") + expect(Chef::FileCache).to receive(:has_key?).with(cache_path).and_return(false) + expect(Chef::FileCache).to receive(:has_key?).with(old_cache_path).and_return(false) end context "and there is no current copy of the file" do @@ -64,7 +66,8 @@ describe Chef::Provider::RemoteFile::CacheControlData do context "and the URI contains a password" do let(:uri) { URI.parse("http://bob:password@example.org/") } - let(:cache_path) { "remote_file/http___bob_XXXX_example_org_-f121caacb74c05a35bcefdf578ed5fc9.json" } + let(:cache_path) { "remote_file/http___bob_XXXX_example_org_-44be109aa176a165ef599c12d97af792.json" } + let(:old_cache_path) { "remote_file/http___bob_XXXX_example_org_-f121caacb74c05a35bcefdf578ed5fc9.json" } it "loads the cache data from a path based on a sanitized URI" do Chef::Provider::RemoteFile::CacheControlData.load_and_validate(uri, current_file_checksum) @@ -88,51 +91,73 @@ describe Chef::Provider::RemoteFile::CacheControlData do Chef::JSONCompat.to_json(cache) end - before do - expect(Chef::FileCache).to receive(:load).with(cache_path).and_return(cache_json_data) - end - - context "and there is no on-disk copy of the file" do - let(:current_file_checksum) { nil } - - it "returns empty cache control data" do - expect(cache_control_data.etag).to be_nil - expect(cache_control_data.mtime).to be_nil + context "when the cache control data uses sha256 for its name" do + before do + expect(Chef::FileCache).to receive(:has_key?).with(cache_path).and_return(true) + expect(Chef::FileCache).to receive(:load).with(cache_path).and_return(cache_json_data) end - end - context "and the cached checksum does not match the on-disk copy" do - let(:current_file_checksum) { "e2a8938cc31754f6c067b35aab1d0d4864272e9bf8504536ef3e79ebf8432305" } + context "and there is no on-disk copy of the file" do + let(:current_file_checksum) { nil } - it "returns empty cache control data" do - expect(cache_control_data.etag).to be_nil - expect(cache_control_data.mtime).to be_nil + it "returns empty cache control data" do + expect(cache_control_data.etag).to be_nil + expect(cache_control_data.mtime).to be_nil + end end - end - context "and the cached checksum matches the on-disk copy" do + context "and the cached checksum does not match the on-disk copy" do + let(:current_file_checksum) { "e2a8938cc31754f6c067b35aab1d0d4864272e9bf8504536ef3e79ebf8432305" } - it "populates the cache control data" do - expect(cache_control_data.etag).to eq(etag) - expect(cache_control_data.mtime).to eq(mtime) + it "returns empty cache control data" do + expect(cache_control_data.etag).to be_nil + expect(cache_control_data.mtime).to be_nil + end end - end - - context "and the cached checksum data is corrupted" do - let(:cache_json_data) { '{"foo",,"bar" []}' } - it "returns empty cache control data" do - expect(cache_control_data.etag).to be_nil - expect(cache_control_data.mtime).to be_nil + context "and the cached checksum matches the on-disk copy" do + context "when the filename uses sha256" do + before do + expect(Chef::FileCache).not_to receive(:has_key?).with(old_cache_path) + end + it "populates the cache control data" do + expect(cache_control_data.etag).to eq(etag) + expect(cache_control_data.mtime).to eq(mtime) + end + end end - context "and it still is valid JSON" do - let(:cache_json_data) { '' } + context "and the cached checksum data is corrupted" do + let(:cache_json_data) { '{"foo",,"bar" []}' } it "returns empty cache control data" do expect(cache_control_data.etag).to be_nil expect(cache_control_data.mtime).to be_nil end + + context "and it still is valid JSON" do + let(:cache_json_data) { '' } + + it "returns empty cache control data" do + expect(cache_control_data.etag).to be_nil + expect(cache_control_data.mtime).to be_nil + end + end + end + end + + context "when the filename uses md5" do + before do + expect(Chef::FileCache).to receive(:has_key?).with(cache_path).and_return(false) + expect(Chef::FileCache).to receive(:has_key?).with(old_cache_path).and_return(true) + expect(Chef::FileCache).to receive(:load).with(old_cache_path).and_return(cache_json_data) + end + + it "populates the cache control data and creates the cache control data file with the correct path" do + expect(Chef::FileCache).to receive(:store).with(cache_path, cache_json_data) + expect(Chef::FileCache).to receive(:delete).with(old_cache_path) + expect(cache_control_data.etag).to eq(etag) + expect(cache_control_data.mtime).to eq(mtime) end end end @@ -174,7 +199,8 @@ describe Chef::Provider::RemoteFile::CacheControlData do context "and the URI contains a password" do let(:uri) { URI.parse("http://bob:password@example.org/") } - let(:cache_path) { "remote_file/http___bob_XXXX_example_org_-f121caacb74c05a35bcefdf578ed5fc9.json" } + let(:cache_path) { "remote_file/http___bob_XXXX_example_org_-44be109aa176a165ef599c12d97af792.json" } + let(:old_cache_path) { "remote_file/http___bob_XXXX_example_org_-f121caacb74c05a35bcefdf578ed5fc9.json" } it "writes the data to the cache with a sanitized path name" do json_data = cache_control_data.json_data |