[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 2.6.5-303-gfcfa26a
Nick Lewis
nick at puppetlabs.com
Thu Mar 17 10:48:27 UTC 2011
The following commit has been merged in the upstream branch:
commit 743e03930758d17ed35fc6b73f7c2c68d8212137
Author: Nick Lewis <nick at puppetlabs.com>
Date: Mon Feb 28 13:40:18 2011 -0800
(#4922) Don't truncate remotely-sourced files on 404
We were 'handling' 404's on remote file content retrieval by returning nil
rather than raising an exception. This caused no content to be written to the
temporary file, but still appeared successful, so the destination file was
overwritten, instead of preserved. Now we just handle 404 like any other
error.
Note that the root cause of these 404s seems to have been #4319, which has been
fixed. However, in the event we do happen to get a 404 here, it's better not to
have code to specifically handle it incorrectly.
Paired-With: Max Martin
Reviewed-By: Matt Robinson
diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb
index 63c0aaf..1b36acb 100755
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -194,7 +194,6 @@ module Puppet
connection = Puppet::Network::HttpPool.http_instance(source_or_content.server, source_or_content.port)
connection.request_get(indirection2uri(request), add_accept_encoding({"Accept" => "raw"})) do |response|
case response.code
- when "404"; nil
when /^2/; uncompress(response) { |uncompressor| response.read_body { |chunk| yield uncompressor.uncompress(chunk) } }
else
# Raise the http error if we didn't get a 'success' of some kind.
diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb
index 9178c94..7d23399 100755
--- a/spec/unit/type/file/content_spec.rb
+++ b/spec/unit/type/file/content_spec.rb
@@ -4,15 +4,14 @@ Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f
content = Puppet::Type.type(:file).attrclass(:content)
describe content do
+ include PuppetSpec::Files
before do
- @resource = Puppet::Type.type(:file).new :path => "/foo/bar"
+ @filename = tmpfile('testfile')
+ @resource = Puppet::Type.type(:file).new :path => @filename
+ File.open(@filename, 'w') {|f| f.write "initial file content"}
content.stubs(:standalone?).returns(false)
end
- it "should be a subclass of Property" do
- content.superclass.must == Puppet::Property
- end
-
describe "when determining the checksum type" do
it "should use the type specified in the source checksum if a source is set" do
@resource[:source] = "/foo"
@@ -249,10 +248,10 @@ describe content do
describe "when writing" do
before do
@content = content.new(:resource => @resource)
- @fh = stub_everything
end
it "should attempt to read from the filebucket if no actual content nor source exists" do
+ @fh = File.open(@filename, 'w')
@content.should = "{md5}foo"
@content.resource.bucket.class.any_instance.stubs(:getfile).returns "foo"
@content.write(@fh)
@@ -302,166 +301,68 @@ describe content do
describe "from local source" do
before(:each) do
- @content.stubs(:actual_content).returns(nil)
- @source = stub_everything 'source', :local? => true, :full_path => "/path/to/source"
- @resource.stubs(:parameter).with(:source).returns @source
-
- @sum = stub_everything 'sum'
- @resource.stubs(:parameter).with(:checksum).returns(@sum)
-
- @digest = stub_everything 'digest'
- @sum.stubs(:sum_stream).yields(@digest)
-
- @file = stub_everything 'file'
- File.stubs(:open).yields(@file)
- @file.stubs(:read).with(8192).returns("chunk1").then.returns("chunk2").then.returns(nil)
- end
-
- it "should open the local file" do
- File.expects(:open).with("/path/to/source", "r")
- @content.write(@fh)
- end
+ @resource = Puppet::Type.type(:file).new :path => @filename, :backup => false
+ @sourcename = tmpfile('source')
+ @source_content = "source file content"*10000
+ @sourcefile = File.open(@sourcename, 'w') {|f| f.write @source_content}
- it "should read the local file by chunks" do
- @file.expects(:read).with(8192).returns("chunk1").then.returns(nil)
- @content.write(@fh)
+ @content = @resource.newattr(:content)
+ @source = @resource.newattr(:source)
+ @source.stubs(:metadata).returns stub_everything('metadata', :source => @sourcename, :ftype => 'file')
end
- it "should write each chunk to the file" do
- @fh.expects(:print).with("chunk1").then.with("chunk2")
- @content.write(@fh)
- end
-
- it "should pass each chunk to the current sum stream" do
- @digest.expects(:<<).with("chunk1").then.with("chunk2")
- @content.write(@fh)
+ it "should copy content from the source to the file" do
+ @resource.write(@source)
+ File.read(@filename).should == @source_content
end
it "should return the checksum computed" do
- @sum.stubs(:sum_stream).yields(@digest).returns("checksum")
- @content.write(@fh).should == "checksum"
+ File.open(@filename, 'w') do |file|
+ @content.write(file).should == "{md5}#{Digest::MD5.hexdigest(@source_content)}"
+ end
end
end
describe "from remote source" do
before(:each) do
- @response = stub_everything 'mock response', :code => "404"
+ @resource = Puppet::Type.type(:file).new :path => @filename, :backup => false
+ @response = stub_everything 'response', :code => "200"
+ @source_content = "source file content"*10000
+ @response.stubs(:read_body).multiple_yields(*(["source file content"]*10000))
+
@conn = stub_everything 'connection'
@conn.stubs(:request_get).yields(@response)
Puppet::Network::HttpPool.stubs(:http_instance).returns @conn
- @content.stubs(:actual_content).returns(nil)
- @source = stub_everything 'source', :local? => false, :full_path => "/path/to/source", :server => "server", :port => 1234
- @resource.stubs(:parameter).with(:source).returns @source
-
- @sum = stub_everything 'sum'
- @resource.stubs(:parameter).with(:checksum).returns(@sum)
-
- @digest = stub_everything 'digest'
- @sum.stubs(:sum_stream).yields(@digest)
- end
-
- it "should open a network connection to source server and port" do
- Puppet::Network::HttpPool.expects(:http_instance).with("server", 1234).returns @conn
- @content.write(@fh)
- end
-
- it "should send the correct indirection uri" do
- @conn.expects(:request_get).with { |uri,headers| uri == "/production/file_content/path/to/source" }.yields(@response)
- @content.write(@fh)
+ @content = @resource.newattr(:content)
+ @sourcename = "puppet:///test/foo"
+ @source = @resource.newattr(:source)
+ @source.stubs(:metadata).returns stub_everything('metadata', :source => @sourcename, :ftype => 'file')
end
- it "should return nil if source is not found" do
- @response.expects(:code).returns("404")
- @content.write(@fh).should == nil
+ it "should write the contents to the file" do
+ @resource.write(@source)
+ File.read(@filename).should == @source_content
end
it "should not write anything if source is not found" do
- @response.expects(:code).returns("404")
- @fh.expects(:print).never
- @content.write(@fh).should == nil
+ @response.stubs(:code).returns("404")
+ lambda {@resource.write(@source)}.should raise_error(Net::HTTPError) { |e| e.message =~ /404/ }
+ File.read(@filename).should == "initial file content"
end
it "should raise an HTTP error in case of server error" do
- @response.expects(:code).returns("500")
- lambda { @content.write(@fh) }.should raise_error
- end
-
- it "should write content by chunks" do
- @response.expects(:code).returns("200")
- @response.expects(:read_body).multiple_yields("chunk1","chunk2")
- @fh.expects(:print).with("chunk1").then.with("chunk2")
- @content.write(@fh)
- end
-
- it "should pass each chunk to the current sum stream" do
- @response.expects(:code).returns("200")
- @response.expects(:read_body).multiple_yields("chunk1","chunk2")
- @digest.expects(:<<).with("chunk1").then.with("chunk2")
- @content.write(@fh)
+ @response.stubs(:code).returns("500")
+ lambda { @content.write(@fh) }.should raise_error { |e| e.message.include? @source_content }
end
it "should return the checksum computed" do
- @response.expects(:code).returns("200")
- @response.expects(:read_body).multiple_yields("chunk1","chunk2")
- @sum.expects(:sum_stream).yields(@digest).returns("checksum")
- @content.write(@fh).should == "checksum"
- end
-
- it "should get the current accept encoding header value" do
- @content.expects(:add_accept_encoding)
- @content.write(@fh)
- end
-
- it "should uncompress body on error" do
- @response.expects(:code).returns("500")
- @response.expects(:body).returns("compressed body")
- @content.expects(:uncompress_body).with(@response).returns("uncompressed")
- lambda { @content.write(@fh) }.should raise_error { |e| e.message =~ /uncompressed/ }
- end
-
- it "should uncompress chunk by chunk" do
- uncompressor = stub_everything 'uncompressor'
- @content.expects(:uncompress).with(@response).yields(uncompressor)
- @response.expects(:code).returns("200")
- @response.expects(:read_body).multiple_yields("chunk1","chunk2")
-
- uncompressor.expects(:uncompress).with("chunk1").then.with("chunk2")
- @content.write(@fh)
- end
-
- it "should write uncompressed chunks to the file" do
- uncompressor = stub_everything 'uncompressor'
- @content.expects(:uncompress).with(@response).yields(uncompressor)
- @response.expects(:code).returns("200")
- @response.expects(:read_body).multiple_yields("chunk1","chunk2")
-
- uncompressor.expects(:uncompress).with("chunk1").returns("uncompressed1")
- uncompressor.expects(:uncompress).with("chunk2").returns("uncompressed2")
-
- @fh.expects(:print).with("uncompressed1")
- @fh.expects(:print).with("uncompressed2")
-
- @content.write(@fh)
- end
-
- it "should pass each uncompressed chunk to the current sum stream" do
- uncompressor = stub_everything 'uncompressor'
- @content.expects(:uncompress).with(@response).yields(uncompressor)
- @response.expects(:code).returns("200")
- @response.expects(:read_body).multiple_yields("chunk1","chunk2")
-
- uncompressor.expects(:uncompress).with("chunk1").returns("uncompressed1")
- uncompressor.expects(:uncompress).with("chunk2").returns("uncompressed2")
-
- @digest.expects(:<<).with("uncompressed1").then.with("uncompressed2")
- @content.write(@fh)
+ File.open(@filename, 'w') do |file|
+ @content.write(file).should == "{md5}#{Digest::MD5.hexdigest(@source_content)}"
+ end
end
end
- describe "from a filebucket" do
- end
-
# These are testing the implementation rather than the desired behaviour; while that bites, there are a whole
# pile of other methods in the File type that depend on intimate details of this implementation and vice-versa.
# If these blow up, you are gonna have to review the callers to make sure they don't explode! --daniel 2011-02-01
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list