Add warning when using an empty regexp in lineinfile (#42013) (#42207)

* Revert "Account for empty string regexp in lineinfile (#41451)"

This reverts commit 4b5b4a760c.

* Use context managers for interacting with files

* Store line and regexp parameters in a variable

* Add warning when regexp is an empty string

* Remove '=' from error messages

* Update warning message and add changelog

* Add tests

* Improve warning message

Offer an equivalent regexp that won't trigger the warning.
Update tests to match new warning.

* Add porting guide entry for lineinfile change

(cherry picked from commit fb55038d75)

changelogs/fragments/lineinfile-empty-regexp.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - lineinfile - add warning when using an empty regexp (https://github.com/ansible/ansible/issues/29443)

docs/docsite/rst/porting_guides/porting_guide_2.5.rst

@@ -184,6 +184,10 @@ and ``checksum_algorithm: md5`` can still be used if an MD5 checksum is
 desired.
 
 * ``osx_say`` module was renamed into :ref:`say <say_module>`.
+* The ``lineinfile`` module was changed in 2.5.6 to show a warning when using an empty string as a regexp.
+  Since an empty regexp matches every line in a file, it will replace the last line in a file rather
+  than inserting. If this is the desired behavior, use ``'^'`` which will match every line and
+  will not trigger the warning.
 * Several modules which could deal with symlinks had the default value of their ``follow`` option
   changed as part of a feature to `standardize the behavior of follow
   <https://github.com/ansible/proposals/issues/69>`_:

lib/ansible/modules/files/lineinfile.py

@@ -478,17 +478,25 @@ def main():
     backrefs = params['backrefs']
     path = params['path']
     firstmatch = params['firstmatch']
+    regexp = params['regexp']
+    line = params['line']
+
+    if regexp == '':
+        module.warn(
+            "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.")
 
     b_path = to_bytes(path, errors='surrogate_or_strict')
     if os.path.isdir(b_path):
         module.fail_json(rc=256, msg='Path %s is a directory !' % path)
 
     if params['state'] == 'present':
-        if backrefs and params['regexp'] is None:
+        if backrefs and regexp is None:
-            module.fail_json(msg='regexp= is required with backrefs=true')
+            module.fail_json(msg='regexp is required with backrefs=true')
 
-        if params.get('line', None) is None:
+        if line is None:
-            module.fail_json(msg='line= is required with state=present')
+            module.fail_json(msg='line is required with state=present')
 
         # Deal with the insertafter default value manually, to avoid errors
         # because of the mutually_exclusive mechanism.
@@ -496,13 +504,11 @@ def main():
         if ins_bef is None and ins_aft is None:
             ins_aft = 'EOF'
 
-        line = params['line']
-
         present(module, path, params['regexp'], line,
                 ins_aft, ins_bef, create, backup, backrefs, firstmatch)
     else:
-        if params['regexp'] is None and params.get('line', None) is None:
+        if regexp is None and line is None:
-            module.fail_json(msg='one of line= or regexp= is required with state=absent')
+            module.fail_json(msg='one of line or regexp is required with state=absent')
 
         absent(module, path, params['regexp'], params.get('line', None), backup)
 

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

@@ -717,3 +717,45 @@
   assert:
     that:
       - result.stat.checksum == 'eca8d8ea089d4ea57a3b87d4091599ca8b60dfd2'
+
+###################################################################
+# Issue 29443
+# When using an empty regexp, replace the last line (since it matches every line)
+# but also provide a warning.
+
+- name: Deploy the test file for lineinfile
+  copy:
+    src: test.txt
+    dest: "{{ output_dir }}/test.txt"
+  register: result
+
+- name: Assert that the test file was deployed
+  assert:
+    that:
+      - result is changed
+      - result.checksum == '5feac65e442c91f557fc90069ce6efc4d346ab51'
+      - result.state == 'file'
+
+- name: Insert a line in the file using an empty string as a regular expression
+  lineinfile:
+    path: "{{ output_dir }}/test.txt"
+    regexp: ''
+    line: This is line 6
+  register: insert_empty_regexp
+
+- name: Stat the file
+  stat:
+    path: "{{ output_dir }}/test.txt"
+  register: result
+
+- name: Assert that the file contents match what is expected and a warning was displayed
+  assert:
+    that:
+      - insert_empty_regexp is changed
+      - warning_message in insert_empty_regexp.warnings
+      - result.stat.checksum == '23555a98ceaa88756b4c7c7bba49d9f86eed868f'
+  vars:
+    warning_message: >-
+      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.