This patch doesn't work for this type of manifest:
file { "/tmp/mydir" :
source => '/tmp/sourcedir',
recurse => true,
}
I've got another patch that I'll get reviewed tomorrow that includes
tests, but the change from I've already sent is really just:
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -163,11 +163,12 @@ module Puppet
Puppet.settings[:name] == "apply"
end
- # the content is munged so if it's a checksum source_or_content is nil here
+ # the content is munged so if it's a checksum source_or_content is nil
+ # unless the checksum indirectly comes from source
def each_chunk_from(source_or_content)
if source_or_content.is_a?(String)
yield source_or_content
- elsif content_is_really_a_checksum?
+ elsif content_is_really_a_checksum? && source_or_content.nil?
yield read_file_from_filebucket
elsif source_or_content.nil?
yield ''
Full patch is on my branch:
https://github.com/mmrobins/puppet/tree/ticket/2.6.next/6541-md5_in_content_truncates
On Tue, Mar 1, 2011 at 5:41 PM, Matt Robinson <[email protected]> wrote:
> 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.
>
>
--
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.
