The patch for #6107 fd73874147a1aaa3a047f904a0bc1ae67780a2e4 introduced a bug when content was an invalid checksum. Rather than error the checksum was invalid, it would overwrite the file with empty string, essentially truncating it.
The problem with #6107 is that when I wrote it, I didn't realize that the content parameter was munged to be nil when it was a checksum, and then chunking method special cased nil content to mean you should check the filebucket. #6107 intended to fix the case where content REALLY WAS nil, and handle that by returning an empty string. This patch fixes it so that we check to see if we really passed in a checksum when chunking, and only then going to the filebucket. I've said it before, and sure I'll say it again, but long term the file provider really needs a refactor. I'll write some acceptance tests for file behavior right after committing this so that the refactoring will be easier. Reviewed-by: Daniel Pittman <[email protected]> Signed-off-by: Matt Robinson <[email protected]> --- lib/puppet/type/file/content.rb | 11 ++++++++--- spec/unit/type/file/content_spec.rb | 9 ++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb index 5223ee3..ae541bc 100755 --- a/lib/puppet/type/file/content.rb +++ b/lib/puppet/type/file/content.rb @@ -163,13 +163,14 @@ module Puppet Puppet.settings[:name] == "apply" end + # the content is munged so if it's a checksum source_or_content is nil here def each_chunk_from(source_or_content) if source_or_content.is_a?(String) yield source_or_content - elsif source_or_content.nil? && resource.parameter(:ensure) && [:present, :file].include?(resource.parameter(:ensure).value) - yield '' - elsif source_or_content.nil? + elsif content_is_really_a_checksum? yield read_file_from_filebucket + elsif source_or_content.nil? + yield '' elsif self.class.standalone? yield source_or_content.content elsif source_or_content.local? @@ -181,6 +182,10 @@ module Puppet private + def content_is_really_a_checksum? + checksum?(should) + end + def chunk_file_from_disk(source_or_content) File.open(source_or_content.full_path, "r") do |src| while chunk = src.read(8192) diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb index 7d23399..3853484 100755 --- a/spec/unit/type/file/content_spec.rb +++ b/spec/unit/type/file/content_spec.rb @@ -380,12 +380,19 @@ describe content do @content.each_chunk_from(nil) { |chunk| chunk.should == '' } end + # you might do this if you were just auditing it "when no content, source, but ensure file" do @resource[:ensure] = :file @content.each_chunk_from(nil) { |chunk| chunk.should == '' } end - it "when no content or source" do + it "when source_or_content is nil and content not a checksum" do + @content.each_chunk_from(nil) { |chunk| chunk.should == '' } + end + + # the content is munged so that if it's a checksum nil gets passed in + it "when content is a checksum it should try to read from filebucket" do + @content.should = "{md5}123abcd" @content.expects(:read_file_from_filebucket).once.returns('im_a_filebucket') @content.each_chunk_from(nil) { |chunk| chunk.should == 'im_a_filebucket' } end -- 1.7.3.1 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
