(MODULES-5651) Do not append infinitely

https://tickets.puppetlabs.com/browse/MODULES-5003 gave rise to
#788 and
#794 which caused
different behavior based on whether the line value was matched by the
match regex or not.

The change in behavior was both breaking and broken, though that was
hard to tell because the behavior was ill-described in general.

[bugfix] This commit resolves the breaking behavior by reverting the
behavior of "replacing matches when a line matching `line` exists even
when `multiple` is set to `true`".

[feature] This commit adds a new parameter to make file_line replace all
matches universally with the `line` value, even when the line exists
elsewhere in the file. This feature only affects modifying multiple
lines in a file when the `line` value already exists.

[bugfix] This commit more strictly defines the various interactions of
`ensure`, `match`, `append_on_no_match`, `replace`, `multiple`, and
`replace_all_matches_not_matching_line`. It also more clearly documents
and tests these interactions.

Author: hunner

README.md

@@ -219,7 +219,7 @@ Values: String.
 
 ##### `match`
 
-Specifies a regular expression to compare against existing lines in the file; if a match is found, it is replaced rather than adding a new line. A regex comparison is performed against the line value, and if it does not match, an exception is raised.
+Specifies a regular expression to compare against existing lines in the file; if a match is found, it is replaced rather than adding a new line.
 
 Values: String containing a regex.
 
@@ -236,7 +236,7 @@ Default value: `false`.
 
 ##### `multiple`
 
-Specifies whether `match` and `after` can change multiple lines. If set to `false`, an exception is raised if more than one line matches.
+Specifies whether `match` and `after` can change multiple lines. If set to `false`, allows file\_line to replace only one line and raises an error if more than one will be replaced. If set to `true`, allows file\_line to replace one or more lines.
 
 Values: `true`, `false`.
 
@@ -261,12 +261,20 @@ Value: String specifying an absolute path to the file.
 
 ##### `replace`
 
-Specifies whether the resource overwrites an existing line that matches the `match` parameter. If set to `false` and a line is found matching the `match` parameter, the line is not placed in the file.
+Specifies whether the resource overwrites an existing line that matches the `match` parameter when `line` does not otherwise exist.
+
+If set to `false` and a line is found matching the `match` parameter, the line is not placed in the file.
 
 Boolean.
 
 Default value: `true`.
 
+##### `replace_all_matches_not_matching_line`
+
+Replace all lines matched by `match` parameter, even if `line` already exists in the file.
+
+Default value: `false`.
+
 ### Data types
 
 #### `Stdlib::Absolutepath`

lib/puppet/provider/file_line/ruby.rb

@@ -12,12 +12,50 @@ def exists?
       found = lines_count > 0
     else
       match_count = count_matches(new_match_regex)
-      if resource[:append_on_no_match].to_s == 'false'
-        found = true
-      elsif resource[:replace].to_s == 'true'
-        found = lines_count > 0 && lines_count == match_count
+      if resource[:ensure] == :present
+        if match_count == 0
+          if lines_count == 0
+            if resource[:append_on_no_match].to_s == 'false'
+              found = true # lies, but gets the job done
+            else
+              found = false
+            end
+          else
+            found = true
+          end
+        else
+          if resource[:replace_all_matches_not_matching_line].to_s == 'true'
+            found = false # maybe lies, but knows there's still work to do
+          else
+            if lines_count == 0
+              if resource[:replace].to_s == 'false'
+                found = true
+              else
+                found = false
+              end
+            else
+              found = true
+            end
+          end
+        end
       else
-        found = match_count > 0
+        if match_count == 0
+          if lines_count == 0
+            found = false
+          else
+            found = true
+          end
+        else
+          if lines_count == 0
+            if resource[:match_for_absence].to_s == 'true'
+              found = true # found matches, not lines
+            else
+              found = false
+            end
+          else
+            found = true
+          end
+        end
       end
     end
     found
@@ -68,7 +106,13 @@ def new_match_regex
   end
 
   def count_matches(regex)
-    lines.select{ |line| line.match(regex) }.size
+    lines.select do |line|
+      if resource[:replace_all_matches_not_matching_line].to_s == 'true'
+        line.match(regex) unless line.chomp == resource[:line]
+      else
+        line.match(regex)
+      end
+    end.size
   end
 
   def handle_create_with_match()
@@ -140,8 +184,9 @@ def handle_destroy_line
   end
 
   def handle_append_line
+    local_lines = lines
     File.open(resource[:path],'w') do |fh|
-      lines.each do |line|
+      local_lines.each do |line|
         fh.puts(line)
       end
       fh.puts(resource[:line])

lib/puppet/type/file_line.rb

@@ -151,6 +151,13 @@ def retrieve
     defaultto true
   end
 
+  newparam(:replace_all_matches_not_matching_line) do
+    desc 'Configures the behavior of replacing all lines in a file which match the `match` parameter regular expression, regardless of whether the specified line is already present in the file.'
+
+    newvalues(true, false)
+    defaultto false
+  end
+
   newparam(:encoding) do
     desc 'For files that are not UTF-8 encoded, specify encoding such as iso-8859-1'
     defaultto 'UTF-8'
@@ -168,6 +175,12 @@ def retrieve
   end
 
   validate do
+    if self[:replace_all_matches_not_matching_line].to_s == 'true' and self[:multiple].to_s == 'false'
+      raise(Puppet::Error, "multiple must be true when replace_all_matches_not_matching_line is true")
+    end
+    if self[:replace_all_matches_not_matching_line].to_s == 'true' and self[:replace].to_s == 'false'
+      raise(Puppet::Error, "replace must be true when replace_all_matches_not_matching_line is true")
+    end
     unless self[:line]
       unless (self[:ensure].to_s == 'absent') and (self[:match_for_absence].to_s == 'true') and self[:match]
         raise(Puppet::Error, "line is a required attribute")