Details
-
New Feature
-
Status: Resolved
-
Normal
-
Resolution: Fixed
-
None
-
None
-
-
Coremunity
-
Platform Core KANBAN
-
Enhancement
-
The temporary location used when creating the new content for a File resource can now be customized using the `staging_location` parameter.
-
Needs Assessment
Description
When Puppet creates a new file it creates it in a temporary location. This location is currently the location the file will exist in plus a unique name, eg /etc/foo123fsd for /etc/foo . In cases where a user wants to run a validate_cmd against the file it should be possible to render this file in a different location, run the validation, and then if the validation passes move the file to it's desired location.
This avoids issues where, for example, an application reads all the files in a directory when performing a task (think sudo) or an application reloads configs automatically from a directory.
The changes to make this possible seems doable and I'd be happy to submit a PR but wanted to post a feature here first to ensure this is something that would be acceptable to the Puppet community.
The change would look something like the following, diffed against Puppet 4.5.2. The replace_file function is used in many other places so would require other modifications, just giving a brief idea of what this might look like (open to other param names)
diff --git a/vendor/puppet-4.5.2/lib/puppet/type/file.rb b/vendor/puppet-4.5.2/lib/puppet/type/file.rb
|
index 8a29286dce..131e6549b8 100644
|
--- a/vendor/puppet-4.5.2/lib/puppet/type/file.rb
|
+++ b/vendor/puppet-4.5.2/lib/puppet/type/file.rb
|
@@ -324,6 +324,29 @@ Puppet::Type.newtype(:file) do
|
defaultto '%'
|
end
|
|
|
+ newparam(:staging_location) do
|
+ desc "For use with validate_cmd parameter. When running validate_cmd first
|
+ render to this staging location. This prevents rendering a temporary file in a location
|
+ where it may break a configuration before running a test against the file to ensure that
|
+ it is valid"
|
+
|
+ validate do |value|
|
+ unless Puppet::Util.absolute_path?(value)
|
+ fail Puppet::Error, "File paths must be fully qualified, not '#{value}'"
|
+ end
|
+ end
|
+
|
+ munge do |value|
|
+ if value.start_with?('//') and ::File.basename(value) == "/"
|
+ # This is a UNC path pointing to a share, so don't add a trailing slash
|
+ ::File.expand_path(value)
|
+ else
|
+ ::File.join(::File.split(::File.expand_path(value)))
|
+ end
|
+ end
|
+
|
+ end
|
+
|
# Autorequire the nearest ancestor directory found in the catalog.
|
autorequire(:file) do
|
req = []
|
@@ -849,7 +872,7 @@ Puppet::Type.newtype(:file) do
|
mode_int = mode ? symbolic_mode_to_int(mode, Puppet::Util::DEFAULT_POSIX_MODE) : nil
|
|
|
|
|
if write_temporary_file?
|
- Puppet::Util.replace_file(self[:path], mode_int) do |file|
|
+ Puppet::Util.replace_file(self[:path], mode_int, self[:staging_location]) do |file|
|
diff --git a/vendor/puppet-4.5.2/lib/puppet/util.rb b/vendor/puppet-4.5.2/lib/puppet/util.rb
|
index 011f2108e8..e8818e6906 100644
|
--- a/vendor/puppet-4.5.2/lib/puppet/util.rb
|
+++ b/vendor/puppet-4.5.2/lib/puppet/util.rb
|
@@ -419,7 +419,7 @@ module Util
|
# preserved. This works hard to avoid loss of any metadata, but will result
|
# in an inode change for the file.
|
#
|
- # Arguments: `filename`, `default_mode`
|
+ # Arguments: `filename`, `default_mode`, `staging_location`
|
#
|
# The filename is the file we are going to replace.
|
#
|
@@ -431,7 +431,7 @@ module Util
|
DEFAULT_POSIX_MODE = 0644
|
DEFAULT_WINDOWS_MODE = nil
|
|
|
- def replace_file(file, default_mode, &block)
|
+ def replace_file(file, default_mode, staging_location, &block)
|
raise Puppet::DevError, "replace_file requires a block" unless block_given?
|
|
|
if default_mode
|
@@ -449,8 +449,12 @@ module Util
|
end
|
|
|
begin
|
- file = Puppet::FileSystem.pathname(file)
|
- tempfile = Puppet::FileSystem::Uniquefile.new(Puppet::FileSystem.basename_string(file), Puppet::FileSystem.dir_string(file))
|
+ file = Puppet::FileSystem.pathname(file)
|
+ if staging_location
|
+ tempfile = Puppet::FileSystem::Uniquefile.new(Puppet::FileSystem.basename_string(file), staging_location)
|
+ else
|
+ tempfile = Puppet::FileSystem::Uniquefile.new(Puppet::FileSystem.basename_string(file), Puppet::FileSystem.dir_string(file))
|
+ end
|
|
|
# Set properties of the temporary file before we write the content, because
|
# Tempfile doesn't promise to be safe from reading by other people, just
|
