From fb55038d7516149aecd9a617572437b8c2c13f50 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Fri, 29 Jun 2018 20:15:43 -0400 Subject: [PATCH] Add warning when using an empty regexp in lineinfile (#42013) * Revert "Account for empty string regexp in lineinfile (#41451)" This reverts commit 4b5b4a760c5217bdfd1d45efd224e2762e9898b0. * 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 --- .../fragments/lineinfile-empty-regexp.yml | 2 ++ .../rst/porting_guides/porting_guide_2.7.rst | 5 ++++ lib/ansible/modules/files/lineinfile.py | 30 +++++++++++-------- .../targets/lineinfile/tasks/main.yml | 12 ++++++-- 4 files changed, 34 insertions(+), 15 deletions(-) create mode 100644 changelogs/fragments/lineinfile-empty-regexp.yml diff --git a/changelogs/fragments/lineinfile-empty-regexp.yml b/changelogs/fragments/lineinfile-empty-regexp.yml new file mode 100644 index 0000000000..8ea3a3a5f8 --- /dev/null +++ b/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) diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.7.rst b/docs/docsite/rst/porting_guides/porting_guide_2.7.rst index de68321d94..cb7db78bb4 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.7.rst +++ b/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 --------------- diff --git a/lib/ansible/modules/files/lineinfile.py b/lib/ansible/modules/files/lineinfile.py index 3b8c14c1cb..1da7c500fb 100644 --- a/lib/ansible/modules/files/lineinfile.py +++ b/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) diff --git a/test/integration/targets/lineinfile/tasks/main.yml b/test/integration/targets/lineinfile/tasks/main.yml index 4838040ca9..e9c76c4aef 100644 --- a/test/integration/targets/lineinfile/tasks/main.yml +++ b/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' \ No newline at end of file + - 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.