diff --git a/changelogs/fragments/lineinfile-empty-regexp.yml b/changelogs/fragments/lineinfile-empty-regexp.yml new file mode 100644 index 00000000000..8ea3a3a5f8e --- /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.6.rst b/docs/docsite/rst/porting_guides/porting_guide_2.6.rst index 96b10bf7f91..10d8a53739a 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.6.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.6.rst @@ -73,6 +73,11 @@ Noteworthy module changes * The ``k8s`` module will not automatically change ``Project`` creation requests into ``ProjectRequest`` creation requests as the ``openshift_raw`` module did. You must now specify the ``ProjectRequest`` kind explicitly. * The ``k8s`` module will not automatically remove secrets from the Ansible return values (and by extension the log). In order to prevent secret values in a task from being logged, specify the ``no_log`` parameter on the task block. * The ``k8s_scale`` module now supports scalable OpenShift objects, such as ``DeploymentConfig``. +* 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. + Plugins diff --git a/lib/ansible/modules/files/lineinfile.py b/lib/ansible/modules/files/lineinfile.py index d3777db9457..0e330cb186e 100644 --- a/lib/ansible/modules/files/lineinfile.py +++ b/lib/ansible/modules/files/lineinfile.py @@ -479,17 +479,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: - 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 +505,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: - 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, params['regexp'], params.get('line', None), backup) diff --git a/test/integration/targets/lineinfile/tasks/main.yml b/test/integration/targets/lineinfile/tasks/main.yml index d18aa999fd1..e9c76c4aef7 100644 --- a/test/integration/targets/lineinfile/tasks/main.yml +++ b/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.