From 84debca5fb0014b3c85c6e2ab1228d96df60a3aa Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 21 Feb 2018 21:08:45 -0500 Subject: [PATCH] Check for regexp match when using insertbefore or insertafter (#36474) (#36552) Add tests to cover this scenario Add changelog fragment Fixes #36156 (cherry picked from commit 723daf3e3a6ab750ba9d1b38b94f618609692bb5) --- ...einfile_insertbefore_after_regexp_fix.yaml | 2 + lib/ansible/modules/files/lineinfile.py | 69 ++++---- .../targets/lineinfile/files/test.conf | 5 + .../targets/lineinfile/tasks/main.yml | 164 ++++++++++++++++++ .../targets/lineinfile/vars/main.yml | 17 ++ 5 files changed, 225 insertions(+), 32 deletions(-) create mode 100644 changelogs/fragments/lineinfile_insertbefore_after_regexp_fix.yaml create mode 100644 test/integration/targets/lineinfile/files/test.conf diff --git a/changelogs/fragments/lineinfile_insertbefore_after_regexp_fix.yaml b/changelogs/fragments/lineinfile_insertbefore_after_regexp_fix.yaml new file mode 100644 index 00000000000..6c7c6909129 --- /dev/null +++ b/changelogs/fragments/lineinfile_insertbefore_after_regexp_fix.yaml @@ -0,0 +1,2 @@ +bugfixes: +- lineinfile - fixed regexp used with insert(before|after) inserting duplicate lines (https://github.com/ansible/ansible/pull/36156) diff --git a/lib/ansible/modules/files/lineinfile.py b/lib/ansible/modules/files/lineinfile.py index 31b09bd237d..4c0c62ac91a 100644 --- a/lib/ansible/modules/files/lineinfile.py +++ b/lib/ansible/modules/files/lineinfile.py @@ -42,10 +42,10 @@ options: regexp: description: - The regular expression to look for in every line of the file. For - C(state=present), the pattern to replace if found; only the last line + C(state=present), the pattern to replace if found. Only the last line found will be replaced. For C(state=absent), the pattern of the line(s) - to remove. Uses Python regular expressions; see - U(http://docs.python.org/2/library/re.html). + to remove. Uses Python regular expressions. + See U(http://docs.python.org/2/library/re.html). version_added: '1.7' state: description: @@ -282,7 +282,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, if firstmatch: break if insertbefore: - # + 1 for the previous line + # index[1] for the previous line index[1] = lineno if firstmatch: break @@ -301,44 +301,49 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, if not b_new_line.endswith(b_linesep): b_new_line += b_linesep - # Add lines when the regexp match already exists somewhere else in the file - 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')): - 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: - b_lines.append(b_line + b_linesep) + # If a regexp is specified and a match is found anywhere in the file, do + # not insert the line before or after. + if regexp is None and m: + + # Insert lines + 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')): + 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: + b_lines.append(b_line + b_linesep) + msg = 'line added' + changed = True + 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 - 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 - - elif insertbefore: - # 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: + + elif insertbefore: + # 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: + b_lines.insert(index[1], b_line + b_linesep) + msg = 'line replaced' + changed = True + + elif b_lines[index[1] - 1].rstrip(b('\r\n')) != b_line: b_lines.insert(index[1], b_line + b_linesep) msg = 'line replaced' changed = True - elif b_lines[index[1] - 1].rstrip(b('\r\n')) != b_line: - b_lines.insert(index[1], b_line + b_linesep) - msg = 'line replaced' - changed = True - elif b_lines[index[0]] != b_new_line: b_lines[index[0]] = b_new_line msg = 'line replaced' changed = True + elif backrefs: # Do absolutely nothing, since it's not safe generating the line # without the regexp matching to populate the backrefs. diff --git a/test/integration/targets/lineinfile/files/test.conf b/test/integration/targets/lineinfile/files/test.conf new file mode 100644 index 00000000000..15404cd692d --- /dev/null +++ b/test/integration/targets/lineinfile/files/test.conf @@ -0,0 +1,5 @@ +[section_one] + +[section_two] + +[section_three] diff --git a/test/integration/targets/lineinfile/tasks/main.yml b/test/integration/targets/lineinfile/tasks/main.yml index 48404f46a46..c497907ea6d 100644 --- a/test/integration/targets/lineinfile/tasks/main.yml +++ b/test/integration/targets/lineinfile/tasks/main.yml @@ -544,3 +544,167 @@ assert: that: - result.stat.checksum == 'a8452bb3643be8d18ba3fc212632b1633bd9f885' + +################################################################### +# Issue 36156 +# Test insertbefore and insertafter with regexp + +- name: Deploy the test.conf file + copy: + src: test.conf + dest: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the test.conf file was deployed + assert: + that: + - result is changed + - result.checksum == '6037f13e419b132eb3fd20a89e60c6c87a6add38' + - result.state == 'file' + +# Test instertafter +- name: Insert lines after with regexp + lineinfile: + path: "{{ output_dir }}/test.conf" + regexp: "{{ item.regexp }}" + line: "{{ item.line }}" + insertafter: "{{ item.after }}" + with_items: "{{ test_befaf_regexp }}" + register: _multitest_5 + +- name: Do the same thing again and check for changes + lineinfile: + path: "{{ output_dir }}/test.conf" + regexp: "{{ item.regexp }}" + line: "{{ item.line }}" + insertafter: "{{ item.after }}" + with_items: "{{ test_befaf_regexp }}" + register: _multitest_6 + +- name: Assert that the file was changed the first time but not the second time + assert: + that: + - item.0 is changed + - item.1 is not changed + with_together: + - "{{ _multitest_5.results }}" + - "{{ _multitest_6.results }}" + +- name: Stat the file + stat: + path: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the file contents match what is expected + assert: + that: + - result.stat.checksum == '06e2c456e5028dd7bcd0b117b5927a1139458c82' + +- name: Do the same thing a third time without regexp and check for changes + lineinfile: + path: "{{ output_dir }}/test.conf" + line: "{{ item.line }}" + insertafter: "{{ item.after }}" + with_items: "{{ test_befaf_regexp }}" + register: _multitest_7 + +- name: Stat the file + stat: + path: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the file was changed when no regexp was provided + assert: + that: + - item is changed + with_items: "{{ _multitest_7.results }}" + +- name: Stat the file + stat: + path: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the file contents match what is expected + assert: + that: + - result.stat.checksum == '5bf50f3d74afd20de4010ca5c04bc7037b062d30' + +# Test insertbefore +- name: Deploy the test.conf file + copy: + src: test.conf + dest: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the test.conf file was deployed + assert: + that: + - result is changed + - result.checksum == '6037f13e419b132eb3fd20a89e60c6c87a6add38' + - result.state == 'file' + +- name: Insert lines before with regexp + lineinfile: + path: "{{ output_dir }}/test.conf" + regexp: "{{ item.regexp }}" + line: "{{ item.line }}" + insertbefore: "{{ item.before }}" + with_items: "{{ test_befaf_regexp }}" + register: _multitest_8 + +- name: Do the same thing again and check for changes + lineinfile: + path: "{{ output_dir }}/test.conf" + regexp: "{{ item.regexp }}" + line: "{{ item.line }}" + insertbefore: "{{ item.before }}" + with_items: "{{ test_befaf_regexp }}" + register: _multitest_9 + +- name: Assert that the file was changed the first time but not the second time + assert: + that: + - item.0 is changed + - item.1 is not changed + with_together: + - "{{ _multitest_8.results }}" + - "{{ _multitest_9.results }}" + +- name: Stat the file + stat: + path: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the file contents match what is expected + assert: + that: + - result.stat.checksum == 'c3be9438a07c44d4c256cebfcdbca15a15b1db91' + +- name: Do the same thing a third time without regexp and check for changes + lineinfile: + path: "{{ output_dir }}/test.conf" + line: "{{ item.line }}" + insertbefore: "{{ item.before }}" + with_items: "{{ test_befaf_regexp }}" + register: _multitest_10 + +- name: Stat the file + stat: + path: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the file was changed when no regexp was provided + assert: + that: + - item is changed + with_items: "{{ _multitest_10.results }}" + +- name: Stat the file + stat: + path: "{{ output_dir }}/test.conf" + register: result + +- name: Assert that the file contents match what is expected + assert: + that: + - result.stat.checksum == 'eca8d8ea089d4ea57a3b87d4091599ca8b60dfd2' diff --git a/test/integration/targets/lineinfile/vars/main.yml b/test/integration/targets/lineinfile/vars/main.yml index bee5e932c56..6e99d4f1a4a 100644 --- a/test/integration/targets/lineinfile/vars/main.yml +++ b/test/integration/targets/lineinfile/vars/main.yml @@ -10,3 +10,20 @@ test_regexp: - regex: '4' replace: 'bar' + + +test_befaf_regexp: + - before: section_three + after: section_one + regexp: option_one= + line: option_one=1 + + - before: section_three + after: section_one + regexp: option_two= + line: option_two=2 + + - before: section_three + after: section_one + regexp: option_three= + line: option_three=3