By ensuring that we sync our properties before renaming the file over the original we avoid a (brief) race which could result in incorrect permissions, ownership, or SELinux settings causing trouble for applications.
Signed-off-by: Daniel Pittman <[email protected]> --- lib/puppet/type/file.rb | 16 ++++++++++++---- spec/unit/type/file_spec.rb | 16 ++++++++++++++-- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 88a0ccb..9a9e208 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -711,11 +711,10 @@ Puppet::Type.newtype(:file) do path = self[:path] name = File.basename(path) - dir = File.dirname(path) - - mode = self.should(:mode) # might be nil - umask = mode ? 000 : 022 + dir = File.dirname(path) or Dir::tmpdir + # Base umask on our desire to set an appropriate mode... + umask = self.should(:mode) ? 000 : 022 Puppet::Util.withumask(umask) { # Write the content to a temporary file. begin @@ -723,6 +722,15 @@ Puppet::Type.newtype(:file) do begin checksum = write_content(f) + # Update attributes to match the target, before we commit the + # whole thing through the rename. This is a bit ugly... + begin + self[:path] = f.path + property_fix + ensure + self[:path] = path + end + # Ensure we have pushed that content safely to disk. f.flush; f.fsync diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb index b2d9ef1..00d3d34 100755 --- a/spec/unit/type/file_spec.rb +++ b/spec/unit/type/file_spec.rb @@ -53,7 +53,7 @@ describe Puppet::Type.type(:file) do end it "should delegate writing to the content property" do - filehandle = stub_everything 'fh' + filehandle = stub_everything 'fh', { :path => "/my/file.tmp" } Tempfile.stubs(:open).yields(filehandle) File.stubs(:rename) property = stub('content_property', :actual_content => "something", :length => "something".length) @@ -66,6 +66,19 @@ describe Puppet::Type.type(:file) do file.write(:content) end + it "should update file properties on the temporary file" do + filehandle = stub_everything 'fh', { :path => "/my/file.tmp" } + file = Puppet::Type::File.new(:name => "/my/file", :mode => '0644') + + Tempfile.stubs(:open).yields(filehandle) + File.stubs(:rename) + + File.expects(:chmod).with(0644, "/my/file.tmp") # temp file is fixed + File.expects(:chmod).with(0644, "/my/file") # and so is real file + + file.write(:content) + end + describe "when validating the checksum" do before { @file.stubs(:validate_checksum?).returns(true) } @@ -92,7 +105,6 @@ describe Puppet::Type.type(:file) do lambda { @file.write :NOTUSED }.should_not raise_error(Puppet::Error) end - end end -- 1.7.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.
