lineinfile - fix bug with insertbefore/insertafter and firstmatch (#63194)

(cherry picked from commit 3b18337cac)

changelogs/fragments/63194-lineinfile_insertafter_duplicate.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- lineinfile - fix bug that caused multiple line insertions (https://github.com/ansible/ansible/issues/58923).

lib/ansible/modules/files/lineinfile.py

@@ -208,7 +208,6 @@ import tempfile
 
 # import module snippets
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six import b
 from ansible.module_utils._text import to_bytes, to_native
 
 
@@ -272,7 +271,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
             b_lines = f.readlines()
 
     if module._diff:
-        diff['before'] = to_native(b('').join(b_lines))
+        diff['before'] = to_native(b''.join(b_lines))
 
     if regexp is not None:
         bre_m = re.compile(to_bytes(regexp, errors='surrogate_or_strict'))
@@ -289,26 +288,47 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
     index = [-1, -1]
     m = None
     b_line = to_bytes(line, errors='surrogate_or_strict')
-    for lineno, b_cur_line in enumerate(b_lines):
-        if regexp is not None:
+
+    # The module's doc says
+    # "If regular expressions are passed to both regexp and
+    # insertafter, insertafter is only honored if no match for regexp is found."
+    # Therefore:
+    # 1. regexp was found -> ignore insertafter, replace the founded line
+    # 2. regexp was not found -> insert the line after 'insertafter' or 'insertbefore' line
+
+    # Given the above:
+    # 1. First check that there is no match for regexp:
+    if regexp is not None:
+        for lineno, b_cur_line in enumerate(b_lines):
             match_found = bre_m.search(b_cur_line)
-        else:
-            match_found = b_line == b_cur_line.rstrip(b('\r\n'))
-        if match_found:
-            index[0] = lineno
-            m = match_found
-        elif bre_ins is not None and bre_ins.search(b_cur_line):
-            if insertafter:
-                # + 1 for the next line
-                index[1] = lineno + 1
-                if firstmatch:
-                    break
-            if insertbefore:
-                # index[1] for the previous line
-                index[1] = lineno
+            if match_found:
+                index[0] = lineno
+                m = match_found
                 if firstmatch:
                     break
 
+    # 2. When no match found on the previous step,
+    # parse for searching insertafter/insertbefore:
+    if not m:
+        for lineno, b_cur_line in enumerate(b_lines):
+            match_found = b_line == b_cur_line.rstrip(b'\r\n')
+            if match_found:
+                index[0] = lineno
+                m = match_found
+
+            elif bre_ins is not None and bre_ins.search(b_cur_line):
+                if insertafter:
+                    # + 1 for the next line
+                    index[1] = lineno + 1
+                    if firstmatch:
+                        break
+
+                if insertbefore:
+                    # index[1] for the previous line
+                    index[1] = lineno
+                    if firstmatch:
+                        break
+
     msg = ''
     changed = False
     b_linesep = to_bytes(os.linesep, errors='surrogate_or_strict')
@@ -331,17 +351,17 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
             if insertafter and insertafter != 'EOF':
                 # Ensure there is a line separator after the found string
                 # at the end of the file.
-                if b_lines and not b_lines[-1][-1:] in (b('\n'), b('\r')):
+                if b_lines and not b_lines[-1][-1:] in (b'\n', b'\r'):
                     b_lines[-1] = b_lines[-1] + b_linesep
 
                 # If the line to insert after is at the end of the file
                 # use the appropriate index value.
                 if len(b_lines) == index[1]:
-                    if b_lines[index[1] - 1].rstrip(b('\r\n')) != b_line:
+                    if b_lines[index[1] - 1].rstrip(b'\r\n') != b_line:
                         b_lines.append(b_line + b_linesep)
                         msg = 'line added'
                         changed = True
-                elif b_lines[index[1]].rstrip(b('\r\n')) != b_line:
+                elif b_lines[index[1]].rstrip(b'\r\n') != b_line:
                     b_lines.insert(index[1], b_line + b_linesep)
                     msg = 'line added'
                     changed = True
@@ -350,12 +370,12 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
                 # If the line to insert before is at the beginning of the file
                 # use the appropriate index value.
                 if index[1] <= 0:
-                    if b_lines[index[1]].rstrip(b('\r\n')) != b_line:
+                    if b_lines[index[1]].rstrip(b'\r\n') != b_line:
                         b_lines.insert(index[1], b_line + b_linesep)
                         msg = 'line added'
                         changed = True
 
-                elif b_lines[index[1] - 1].rstrip(b('\r\n')) != b_line:
+                elif b_lines[index[1] - 1].rstrip(b'\r\n') != b_line:
                     b_lines.insert(index[1], b_line + b_linesep)
                     msg = 'line added'
                     changed = True
@@ -380,12 +400,21 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
     elif insertafter == 'EOF' or index[1] == -1:
 
         # If the file is not empty then ensure there's a newline before the added line
-        if b_lines and not b_lines[-1][-1:] in (b('\n'), b('\r')):
+        if b_lines and not b_lines[-1][-1:] in (b'\n', b'\r'):
             b_lines.append(b_linesep)
 
         b_lines.append(b_line + b_linesep)
         msg = 'line added'
         changed = True
+
+    elif insertafter and index[1] != -1:
+
+        # Don't insert the line if it already matches at the index
+        if b_line != b_lines[index[1]].rstrip(b'\n\r'):
+            b_lines.insert(index[1], b_line + b_linesep)
+            msg = 'line added'
+            changed = True
+
     # insert matched, but not the regexp
     else:
         b_lines.insert(index[1], b_line + b_linesep)
@@ -393,7 +422,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
         changed = True
 
     if module._diff:
-        diff['after'] = to_native(b('').join(b_lines))
+        diff['after'] = to_native(b''.join(b_lines))
 
     backupdest = ""
     if changed and not module.check_mode:
@@ -430,7 +459,7 @@ def absent(module, dest, regexp, line, backup):
         b_lines = f.readlines()
 
     if module._diff:
-        diff['before'] = to_native(b('').join(b_lines))
+        diff['before'] = to_native(b''.join(b_lines))
 
     if regexp is not None:
         bre_c = re.compile(to_bytes(regexp, errors='surrogate_or_strict'))
@@ -442,7 +471,7 @@ def absent(module, dest, regexp, line, backup):
         if regexp is not None:
             match_found = bre_c.search(b_cur_line)
         else:
-            match_found = b_line == b_cur_line.rstrip(b('\r\n'))
+            match_found = b_line == b_cur_line.rstrip(b'\r\n')
         if match_found:
             found.append(b_cur_line)
         return not match_found
@@ -451,7 +480,7 @@ def absent(module, dest, regexp, line, backup):
     changed = len(found) > 0
 
     if module._diff:
-        diff['after'] = to_native(b('').join(b_lines))
+        diff['after'] = to_native(b''.join(b_lines))
 
     backupdest = ""
     if changed and not module.check_mode:

test/integration/targets/lineinfile/files/firstmatch.txt

@@ -0,0 +1,5 @@
+line1
+line1
+line1
+line2
+line3

test/integration/targets/lineinfile/files/test_58923.txt

@@ -0,0 +1,4 @@
+#!/bin/sh
+
+case "`uname`" in
+  Darwin*) if [ -z "$JAVA_HOME" ] ; then

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

@@ -818,3 +818,281 @@
       The regular expression is an empty string, which will match every line in the file.
       This may have unintended consequences, such as replacing the last line in the file rather than appending.
       If this is desired, use '^' to match every line in the file and avoid this warning.
+
+###################################################################
+## Issue #58923
+## Using firstmatch with insertafter and ensure multiple lines are not inserted
+
+- name: Deploy the firstmatch test file
+  copy:
+    src: firstmatch.txt
+    dest: "{{ output_dir }}/firstmatch.txt"
+  register: result
+
+- name: Assert that the test file was deployed
+  assert:
+    that:
+      - result is changed
+      - result.checksum == '1d644e5e2e51c67f1bd12d7bbe2686017f39923d'
+      - result.state == 'file'
+
+- name: Insert a line before an existing line using firstmatch
+  lineinfile:
+    path: "{{ output_dir }}/firstmatch.txt"
+    line: INSERT
+    insertafter: line1
+    firstmatch: yes
+  register: insertafter1
+
+- name: Insert a line before an existing line using firstmatch again
+  lineinfile:
+    path: "{{ output_dir }}/firstmatch.txt"
+    line: INSERT
+    insertafter: line1
+    firstmatch: yes
+  register: insertafter2
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/firstmatch.txt"
+  register: result
+
+- name: Assert that the file was modified appropriately
+  assert:
+    that:
+      - insertafter1 is changed
+      - insertafter2 is not changed
+      - result.stat.checksum == '114aae024073a3ee8ec8db0ada03c5483326dd86'
+
+########################################################################################
+# Tests of fixing the same issue as above (#58923) by @Andersson007 <aaklychkov@mail.ru>
+# and @samdoran <sdoran@redhat.com>:
+
+# Test insertafter with regexp
+- name: Deploy the test file
+  copy:
+    src: test_58923.txt
+    dest: "{{ output_dir }}/test_58923.txt"
+  register: initial_file
+
+- name: Assert that the test file was deployed
+  assert:
+    that:
+      - initial_file is changed
+      - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e'
+      - initial_file.state == 'file'
+
+# Regarding the documentation:
+# If regular expressions are passed to both regexp and
+# insertafter, insertafter is only honored if no match for regexp is found.
+# Therefore,
+# when regular expressions are passed to both regexp and insertafter, then:
+# 1. regexp was found -> ignore insertafter, replace the founded line
+# 2. regexp was not found -> insert the line after 'insertafter' line
+
+# Regexp is not present in the file, so the line must be inserted after ^#!/bin/sh
+- name: Add the line using firstmatch, regexp, and insertafter
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertafter: '^#!/bin/sh'
+    regexp: ^export FISHEYE_OPTS
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertafter_test1
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertafter_test1_file
+
+- name: Add the line using firstmatch, regexp, and insertafter again
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertafter: '^#!/bin/sh'
+    regexp: ^export FISHEYE_OPTS
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertafter_test2
+
+# Check of the prev step.
+# We tried to add the same line with the same playbook,
+# so nothing has been added:
+- name: Stat the file again
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertafter_test2_file
+
+- name: Assert insertafter tests gave the expected results
+  assert:
+    that:
+      - insertafter_test1 is changed
+      - insertafter_test1_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08'
+      - insertafter_test2 is not changed
+      - insertafter_test2_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08'
+
+# Test insertafter without regexp
+- name: Deploy the test file
+  copy:
+    src: test_58923.txt
+    dest: "{{ output_dir }}/test_58923.txt"
+  register: initial_file
+
+- name: Assert that the test file was deployed
+  assert:
+    that:
+      - initial_file is changed
+      - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e'
+      - initial_file.state == 'file'
+
+- name: Insert the line using firstmatch and insertafter without regexp
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertafter: '^#!/bin/sh'
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertafter_test3
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertafter_test3_file
+
+- name: Insert the line using firstmatch and insertafter without regexp again
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertafter: '^#!/bin/sh'
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertafter_test4
+
+- name: Stat the file again
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertafter_test4_file
+
+- name: Assert insertafter without regexp tests gave the expected results
+  assert:
+    that:
+      - insertafter_test3 is changed
+      - insertafter_test3_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08'
+      - insertafter_test4 is not changed
+      - insertafter_test4_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08'
+
+
+# Test insertbefore with regexp
+- name: Deploy the test file
+  copy:
+    src: test_58923.txt
+    dest: "{{ output_dir }}/test_58923.txt"
+  register: initial_file
+
+- name: Assert that the test file was deployed
+  assert:
+    that:
+      - initial_file is changed
+      - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e'
+      - initial_file.state == 'file'
+
+- name: Add the line using regexp, firstmatch, and insertbefore
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertbefore: '^#!/bin/sh'
+    regexp: ^export FISHEYE_OPTS
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertbefore_test1
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertbefore_test1_file
+
+- name: Add the line using regexp, firstmatch, and insertbefore again
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertbefore: '^#!/bin/sh'
+    regexp: ^export FISHEYE_OPTS
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertbefore_test2
+
+- name: Stat the file again
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertbefore_test2_file
+
+- name: Assert insertbefore with regexp tests gave the expected results
+  assert:
+    that:
+      - insertbefore_test1 is changed
+      - insertbefore_test1_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7'
+      - insertbefore_test2 is not changed
+      - insertbefore_test2_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7'
+
+
+# Test insertbefore without regexp
+- name: Deploy the test file
+  copy:
+    src: test_58923.txt
+    dest: "{{ output_dir }}/test_58923.txt"
+  register: initial_file
+
+- name: Assert that the test file was deployed
+  assert:
+    that:
+      - initial_file is changed
+      - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e'
+      - initial_file.state == 'file'
+
+- name: Add the line using insertbefore and firstmatch
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertbefore: '^#!/bin/sh'
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertbefore_test3
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertbefore_test3_file
+
+- name: Add the line using insertbefore and firstmatch again
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertbefore: '^#!/bin/sh'
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertbefore_test4
+
+- name: Stat the file again
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertbefore_test4_file
+
+# Test when the line is presented in the file but
+# not in the before/after spot and it does match the regexp:
+- name: >
+    Add the line using insertbefore and firstmatch when the regexp line
+    is presented but not close to insertbefore spot
+  lineinfile:
+    path: "{{ output_dir }}/test_58923.txt"
+    insertbefore: '   Darwin\*\) if \[ -z \"\$JAVA_HOME\" \] ; then'
+    firstmatch: true
+    line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m"
+  register: insertbefore_test5
+
+- name: Stat the file again
+  stat:
+    path: "{{ output_dir }}/test_58923.txt"
+  register: insertbefore_test5_file
+
+- name: Assert insertbefore with regexp tests gave the expected results
+  assert:
+    that:
+      - insertbefore_test3 is changed
+      - insertbefore_test3_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7'
+      - insertbefore_test4 is not changed
+      - insertbefore_test4_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7'
+      - insertbefore_test5 is not changed
+      - insertbefore_test5_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7'