Jira (PUP-9389) Add new parameter to File for tempfile staging location

[Brett Kochendorfer (JIRA)]

Title: Message Title

Brett Kochendorfer created an issue   Puppet / PUP-9389 Add new parameter to File for tempfile staging location Issue Type: New Feature Assignee: Unassigned Created: 2018/12/21 11:13 AM Priority: Normal Reporter: Brett Kochendorfer 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   Add Comment   This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93) -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To unsubscribe from this group and stop receiving emails from it, send an email to puppet-bugs+unsubscr...@googlegroups.com. To post to this group, send email to puppet-bugs@googlegroups.com. Visit this group at https://groups.google.com/group/puppet-bugs. For more options, visit https://groups.google.com/d/optout.