There is some complex logic around replacing an existing file safely, and it
is much better that we provide a central, and easy to use, method for
achieving that in a consistent way.
Also, added a tiny stub test to verify that the method does the bare minimum
of what it says on the box.
---
lib/puppet/util.rb | 46 ++++++++++++++++++++++++++++++++++++++++++++++
spec/unit/util.rb | 26 ++++++++++++++++++++++++++
2 files changed, 72 insertions(+), 0 deletions(-)
create mode 100755 spec/unit/util.rb
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 503c02b..45f4284 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -2,6 +2,7 @@
require 'puppet/util/monkey_patches'
require 'sync'
+require 'tempfile'
require 'puppet/external/lock'
module Puppet
@@ -444,6 +445,51 @@ module Util
end
end
module_function :secure_open
+
+ def replace_file (file, mode = 0600, &block)
+ raise Puppet::DevError,"replace_file requires a block" unless
block_given?
+ # Theoretically we should have an absolute path anyway, but let us be
+ # forgiving to the developers, shall we?
+ dir = File.dirname(File.expand_path(file))
+ tempfile = Tempfile.new(file, dir)
+
+ # OK, now allow the caller to write the content of the file...
+ yield tempfile
+
+ # Adjust the mode of the file on disk before we replace the old file,
+ # to ensure that processes which respond immediately don't fail.
+ tempfile.chmod(mode)
+
+ # Now, make sure the data (which includes the mode) is safe on disk.
+ #
+ # Note: fsync may not be implemented by Ruby on all platforms, but
+ # there is absolutely no recovery path if we detect that. So, we just
+ # ignore the return code.
+ #
+ # However, don't be fooled: that is accepting that we are running in
+ # an unsafe fashion. If you are porting to a new platform don't stub
+ # that out.
+ tempfile.flush
+ tempfile.fsync
+
+ # ...and then actually replace the original file. this *must* occur
+ # before tempfile is closed, or the class will delete the data on disk.
+ File.rename(tempfile.path, file)
+
+ # Ideally, we would now fsync the directory as well, but Ruby doesn't
+ # have support for that, and it doesn't matter /that/ much...
+
+ # Close the file handle to stop anyone doing anything *too* silly
+ # happening, such as further writes to the file object we yielded if
+ # it got stashed away outside the scope or something.
+ #
+ # By "silly" I mean "not actually atomic", and "able to lose data"
+ tempfile.close
+
+ # Return something true, and possibly useful.
+ return file
+ end
+ module_function :replace_file
end
end
diff --git a/spec/unit/util.rb b/spec/unit/util.rb
new file mode 100755
index 0000000..1b662e8
--- /dev/null
+++ b/spec/unit/util.rb
@@ -0,0 +1,26 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../spec_helper'
+
+require 'tempfile'
+
+describe Puppet::Util do
+ it "should have a 'replace_file' method" do
+ Puppet::Util.should respond_to(:specificity)
+ end
+
+ it "should replace a file when replace_file is used" do
+ # Create a target file to replace.
+ target = Tempfile.new("replace-me")
+ target.print "replace-me\n"
+ target.fsync # flush that to disk...
+
+ # Now, replace it...
+ replace_file(target.path, 0600) { |f|
+ f.print "replacement\n"
+ }
+
+ # Finally, check that actually worked.
+ File.new(target.path).readline.must == "replacement\n"
+ end
+end
--
1.7.0.4
--
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.
