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

* 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

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.7.rst

@@ -70,6 +70,11 @@ Major changes in popular modules are detailed here
   :ref:`DEFAULT_SYSLOG_FACILITY`. If you have :ref:`DEFAULT_SYSLOG_FACILITY` configured, the
   location of remote logs on systems which use journald may change.
 
+* The ``lineinfile`` module was changed 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.
+
 
 Modules removed
 ---------------

lib/ansible/modules/files/lineinfile.py

@@ -252,7 +252,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
     if module._diff:
         diff['before'] = to_native(b('').join(b_lines))
 
-    if regexp:
+    if regexp is not None:
         bre_m = re.compile(to_bytes(regexp, errors='surrogate_or_strict'))
 
     if insertafter not in (None, 'BOF', 'EOF'):
@@ -268,7 +268,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create,
     m = None
     b_line = to_bytes(line, errors='surrogate_or_strict')
     for lineno, b_cur_line in enumerate(b_lines):
-        if regexp:
+        if regexp is not None:
             match_found = bre_m.search(b_cur_line)
         else:
             match_found = b_line == b_cur_line.rstrip(b('\r\n'))
@@ -410,14 +410,14 @@ def absent(module, dest, regexp, line, backup):
     if module._diff:
         diff['before'] = to_native(b('').join(b_lines))
 
-    if regexp:
+    if regexp is not None:
         bre_c = re.compile(to_bytes(regexp, errors='surrogate_or_strict'))
     found = []
 
     b_line = to_bytes(line, errors='surrogate_or_strict')
 
     def matcher(b_cur_line):
-        if regexp:
+        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'))
@@ -478,18 +478,24 @@ def main():
     path = params['path']
     firstmatch = params['firstmatch']
     regexp = params['regexp']
-    line = params.get('line', None)
+    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 not regexp:
-            module.fail_json(msg='regexp= is required with backrefs=true')
+        if backrefs and regexp is None:
+            module.fail_json(msg='regexp is required with backrefs=true')
 
-        if params.get('line', None) is None:
-            module.fail_json(msg='line= is required with state=present')
+        if line is None:
+            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.
@@ -497,13 +503,11 @@ def main():
         if ins_bef is None and ins_aft is None:
             ins_aft = 'EOF'
 
-        line = params['line']
-
         present(module, path, regexp, line,
                 ins_aft, ins_bef, create, backup, backrefs, firstmatch)
     else:
-        if not regexp and line is None:
-            module.fail_json(msg='one of line= or regexp= is required with state=absent')
+        if regexp is None and line is None:
+            module.fail_json(msg='one of line or regexp is required with state=absent')
 
         absent(module, path, regexp, line, backup)
 

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

@@ -720,6 +720,8 @@
 
 ###################################################################
 # 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:
@@ -746,8 +748,14 @@
     path: "{{ output_dir }}/test.txt"
   register: result
 
-- name: Assert that the file contents match what is expected
+- name: Assert that the file contents match what is expected and a warning was displayed
   assert:
     that:
       - insert_empty_regexp is changed
-      - result.stat.checksum == '3baeade8eb2ecf4b01d70d541e9b8258b67c7f9f'
+      - 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.