(MODULES-5651) Do not append infinitely

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

This resolves the behavior by adding a new parameter to control whether
multiple matches should universally be replaced by the line value, or
whether matches should be left alone when the line exists elsewhere in
the file.

This problem only affects modifying multiple lines when the line already
exists.

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

@@ -1,26 +1,58 @@
 Puppet::Type.type(:file_line).provide(:ruby) do
   def exists?
-    found = false
+    leave_it = false
     lines_count = 0
     lines.each do |line|
-      found = line.chomp == resource[:line]
-      if found
+      leave_it = line.chomp == resource[:line]
+      if leave_it
         lines_count += 1
       end
     end
     if resource[:match] == nil
-      found = lines_count > 0
+      leave_it = 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
+      # conditions: match, line, append, replace, replace_all_matches_not_matching_line
+      # values      int    int   t/f     t/f      t/f
+      # cases:
+      # - false replace,  true replace_all_matches_not_matching_line => error (handled in type)
+      # - false multiple, true replace_all_matches_not_matching_line => error (handled in type)
+      #
+      #
+      #
+      # - zero match matches, zero line matches, and false append                           => leave it
+      # - zero match matches, zero line matches, and true  append                           => append
+      # - zero match matches, any  line matches                                             => leave it
+      # - any  match matches, zero line matches, and false replace                          => leave it
+      # - any  match matches, zero line matches, and true  replace                          => replace
+      # - any  match matches,                   true  replace_all_matches_not_matching_line => replace
+      # - any  match matches,                   false replace_all_matches_not_matching_line => go with old behavior
+      if match_count == 0
+        if lines_count == 0
+          if resource[:append_on_no_match].to_s == 'false'
+            leave_it = true
+          else
+            leave_it = false
+          end
+        else
+          leave_it = true
+        end
       else
-        found = match_count > 0
+        if resource[:replace_all_matches_not_matching_line].to_s == 'true'
+          leave_it = false
+        else
+          if lines_count == 0
+            if resource[:replace].to_s == 'false'
+              leave_it = true
+            else
+              leave_it = false
+            end
+          else
+            leave_it = true
+          end
+        end
       end
     end
-    found
+    leave_it
   end
 
   def create

lib/puppet/type/file_line.rb

@@ -168,6 +168,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")

spec/unit/puppet/provider/file_line/ruby_spec.rb

@@ -195,25 +195,51 @@
         expect(File.read(@tmpfile).chomp).to eql("foo1\nfoo = bar\nfoo2")
       end
 
-      it 'should not add line after no matches found' do
-        @resource = Puppet::Type::File_line.new(
-          {
-            :name               => 'foo',
-            :path               => @tmpfile,
-            :line               => 'inserted = line',
-            :match              => '^foo3$',
-            :append_on_no_match => false,
-          }
-        )
-        @provider = provider_class.new(@resource)
-        File.open(@tmpfile, 'w') do |fh|
-          fh.write("foo1\nfoo = blah\nfoo2\nfoo = baz")
+    end
+
+    describe 'using match+append_on_no_match' do
+      context 'when there is a match' do
+        it 'should replace line' do
+          @resource = Puppet::Type::File_line.new(
+            {
+              :name               => 'foo',
+              :path               => @tmpfile,
+              :line               => 'inserted = line',
+              :match              => '^foo3$',
+              :append_on_no_match => false,
+            }
+          )
+          @provider = provider_class.new(@resource)
+          File.open(@tmpfile, 'w') do |fh|
+            fh.write("foo1\nfoo = blah\nfoo2\nfoo = baz")
+          end
+          expect(@provider.exists?).to be true
+          expect(File.read(@tmpfile).chomp).to eql("foo1\nfoo = blah\nfoo2\nfoo = baz")
+        end
+      end
+      context 'when there is no match' do
+        it 'should not add line after no matches found' do
+          @resource = Puppet::Type::File_line.new(
+            {
+              :name               => 'foo',
+              :path               => @tmpfile,
+              :line               => 'inserted = line',
+              :match              => '^foo3$',
+              :append_on_no_match => false,
+            }
+          )
+          @provider = provider_class.new(@resource)
+          File.open(@tmpfile, 'w') do |fh|
+            fh.write("foo1\nfoo = blah\nfoo2\nfoo = baz")
+          end
+          expect(@provider.exists?).to be true
+          expect(File.read(@tmpfile).chomp).to eql("foo1\nfoo = blah\nfoo2\nfoo = baz")
         end
-        expect(@provider.exists?).to be true
-        expect(File.read(@tmpfile).chomp).to eql("foo1\nfoo = blah\nfoo2\nfoo = baz")
       end
     end
 
+    describe 'using match+replace+append_on_no_match'
+
     describe 'using after' do
       let :resource do
         Puppet::Type::File_line.new(