Check for regexp match when using insertbefore or insertafter (#36474) (#36552)

Add tests to cover this scenario
Add changelog fragment
Fixes #36156

(cherry picked from commit 723daf3e3a)

changelogs/fragments/lineinfile_insertbefore_after_regexp_fix.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- lineinfile - fixed regexp used with insert(before|after) inserting duplicate lines (https://github.com/ansible/ansible/pull/36156)

lib/ansible/modules/files/lineinfile.py

@@ -42,10 +42,10 @@ options:
   regexp:
     description:
       - The regular expression to look for in every line of the file. For
-        C(state=present), the pattern to replace if found; only the last line
+        C(state=present), the pattern to replace if found. Only the last line
         found will be replaced. For C(state=absent), the pattern of the line(s)
-        to remove.  Uses Python regular expressions; see
-        U(http://docs.python.org/2/library/re.html).
+        to remove. Uses Python regular expressions.
+        See U(http://docs.python.org/2/library/re.html).
     version_added: '1.7'
   state:
     description:
@@ -282,7 +282,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
                 if firstmatch:
                     break
             if insertbefore:
-                # + 1 for the previous line
+                # index[1] for the previous line
                 index[1] = lineno
                 if firstmatch:
                     break
@@ -301,7 +301,11 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
         if not b_new_line.endswith(b_linesep):
             b_new_line += b_linesep
 
-        # Add lines when the regexp match already exists somewhere else in the file
+        # If a regexp is specified and a match is found anywhere in the file, do
+        # not insert the line before or after.
+        if regexp is None and m:
+
+            # Insert lines
             if insertafter and insertafter != 'EOF':
 
                 # Ensure there is a line separator after the found string
@@ -339,6 +343,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
             b_lines[index[0]] = b_new_line
             msg = 'line replaced'
             changed = True
+
     elif backrefs:
         # Do absolutely nothing, since it's not safe generating the line
         # without the regexp matching to populate the backrefs.

test/integration/targets/lineinfile/files/test.conf

@@ -0,0 +1,5 @@
+[section_one]
+
+[section_two]
+
+[section_three]

test/integration/targets/lineinfile/tasks/main.yml

@@ -544,3 +544,167 @@
   assert:
     that:
       - result.stat.checksum == 'a8452bb3643be8d18ba3fc212632b1633bd9f885'
+
+###################################################################
+# Issue 36156
+# Test insertbefore and insertafter with regexp
+
+- name: Deploy the test.conf file
+  copy:
+    src: test.conf
+    dest: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the test.conf file was deployed
+  assert:
+    that:
+      - result is changed
+      - result.checksum == '6037f13e419b132eb3fd20a89e60c6c87a6add38'
+      - result.state == 'file'
+
+# Test instertafter
+- name: Insert lines after with regexp
+  lineinfile:
+    path: "{{ output_dir }}/test.conf"
+    regexp: "{{ item.regexp }}"
+    line: "{{ item.line }}"
+    insertafter: "{{ item.after }}"
+  with_items: "{{ test_befaf_regexp }}"
+  register: _multitest_5
+
+- name: Do the same thing again and check for changes
+  lineinfile:
+    path: "{{ output_dir }}/test.conf"
+    regexp: "{{ item.regexp }}"
+    line: "{{ item.line }}"
+    insertafter: "{{ item.after }}"
+  with_items: "{{ test_befaf_regexp }}"
+  register: _multitest_6
+
+- name: Assert that the file was changed the first time but not the second time
+  assert:
+    that:
+      - item.0 is changed
+      - item.1 is not changed
+  with_together:
+    - "{{ _multitest_5.results }}"
+    - "{{ _multitest_6.results }}"
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the file contents match what is expected
+  assert:
+    that:
+      - result.stat.checksum == '06e2c456e5028dd7bcd0b117b5927a1139458c82'
+
+- name: Do the same thing a third time without regexp and check for changes
+  lineinfile:
+    path: "{{ output_dir }}/test.conf"
+    line: "{{ item.line }}"
+    insertafter: "{{ item.after }}"
+  with_items: "{{ test_befaf_regexp }}"
+  register: _multitest_7
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the file was changed when no regexp was provided
+  assert:
+    that:
+      - item is changed
+  with_items: "{{ _multitest_7.results }}"
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the file contents match what is expected
+  assert:
+    that:
+      - result.stat.checksum == '5bf50f3d74afd20de4010ca5c04bc7037b062d30'
+
+# Test insertbefore
+- name: Deploy the test.conf file
+  copy:
+    src: test.conf
+    dest: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the test.conf file was deployed
+  assert:
+    that:
+      - result is changed
+      - result.checksum == '6037f13e419b132eb3fd20a89e60c6c87a6add38'
+      - result.state == 'file'
+
+- name: Insert lines before with regexp
+  lineinfile:
+    path: "{{ output_dir }}/test.conf"
+    regexp: "{{ item.regexp }}"
+    line: "{{ item.line }}"
+    insertbefore: "{{ item.before }}"
+  with_items: "{{ test_befaf_regexp }}"
+  register: _multitest_8
+
+- name: Do the same thing again and check for changes
+  lineinfile:
+    path: "{{ output_dir }}/test.conf"
+    regexp: "{{ item.regexp }}"
+    line: "{{ item.line }}"
+    insertbefore: "{{ item.before }}"
+  with_items: "{{ test_befaf_regexp }}"
+  register: _multitest_9
+
+- name: Assert that the file was changed the first time but not the second time
+  assert:
+    that:
+      - item.0 is changed
+      - item.1 is not changed
+  with_together:
+    - "{{ _multitest_8.results }}"
+    - "{{ _multitest_9.results }}"
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the file contents match what is expected
+  assert:
+    that:
+      - result.stat.checksum == 'c3be9438a07c44d4c256cebfcdbca15a15b1db91'
+
+- name: Do the same thing a third time without regexp and check for changes
+  lineinfile:
+    path: "{{ output_dir }}/test.conf"
+    line: "{{ item.line }}"
+    insertbefore: "{{ item.before }}"
+  with_items: "{{ test_befaf_regexp }}"
+  register: _multitest_10
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the file was changed when no regexp was provided
+  assert:
+    that:
+      - item is changed
+  with_items: "{{ _multitest_10.results }}"
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test.conf"
+  register: result
+
+- name: Assert that the file contents match what is expected
+  assert:
+    that:
+      - result.stat.checksum == 'eca8d8ea089d4ea57a3b87d4091599ca8b60dfd2'

test/integration/targets/lineinfile/vars/main.yml

@@ -10,3 +10,20 @@ test_regexp:
 
   - regex: '4'
     replace: 'bar'
+
+
+test_befaf_regexp:
+  - before: section_three
+    after: section_one
+    regexp: option_one=
+    line: option_one=1
+
+  - before: section_three
+    after: section_one
+    regexp: option_two=
+    line: option_two=2
+
+  - before: section_three
+    after: section_one
+    regexp: option_three=
+    line: option_three=3