From cc244f4e039bfa4f78475eba22dc3ccd27c7f520 Mon Sep 17 00:00:00 2001 From: Andrey Klychkov Date: Tue, 8 Oct 2019 17:01:36 +0300 Subject: [PATCH] lineinfile - fix bug with insertbefore/insertafter and firstmatch (#63194) (cherry picked from commit 3b18337cac332e24bb2bcdca3f4a4451770efc68) --- ...3194-lineinfile_insertafter_duplicate.yaml | 2 + lib/ansible/modules/files/lineinfile.py | 85 ++++-- .../targets/lineinfile/files/firstmatch.txt | 5 + .../targets/lineinfile/files/test_58923.txt | 4 + .../targets/lineinfile/tasks/main.yml | 278 ++++++++++++++++++ 5 files changed, 346 insertions(+), 28 deletions(-) create mode 100644 changelogs/fragments/63194-lineinfile_insertafter_duplicate.yaml create mode 100644 test/integration/targets/lineinfile/files/firstmatch.txt create mode 100644 test/integration/targets/lineinfile/files/test_58923.txt diff --git a/changelogs/fragments/63194-lineinfile_insertafter_duplicate.yaml b/changelogs/fragments/63194-lineinfile_insertafter_duplicate.yaml new file mode 100644 index 00000000000..831a2110b6f --- /dev/null +++ b/changelogs/fragments/63194-lineinfile_insertafter_duplicate.yaml @@ -0,0 +1,2 @@ +bugfixes: +- lineinfile - fix bug that caused multiple line insertions (https://github.com/ansible/ansible/issues/58923). diff --git a/lib/ansible/modules/files/lineinfile.py b/lib/ansible/modules/files/lineinfile.py index b22d48960a2..6647b28e229 100644 --- a/lib/ansible/modules/files/lineinfile.py +++ b/lib/ansible/modules/files/lineinfile.py @@ -208,7 +208,6 @@ import tempfile # import module snippets from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.six import b from ansible.module_utils._text import to_bytes, to_native @@ -272,7 +271,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, b_lines = f.readlines() if module._diff: - diff['before'] = to_native(b('').join(b_lines)) + diff['before'] = to_native(b''.join(b_lines)) if regexp is not None: bre_m = re.compile(to_bytes(regexp, errors='surrogate_or_strict')) @@ -289,26 +288,47 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, index = [-1, -1] m = None b_line = to_bytes(line, errors='surrogate_or_strict') - for lineno, b_cur_line in enumerate(b_lines): - if regexp is not None: + + # The module's doc says + # "If regular expressions are passed to both regexp and + # insertafter, insertafter is only honored if no match for regexp is found." + # Therefore: + # 1. regexp was found -> ignore insertafter, replace the founded line + # 2. regexp was not found -> insert the line after 'insertafter' or 'insertbefore' line + + # Given the above: + # 1. First check that there is no match for regexp: + if regexp is not None: + for lineno, b_cur_line in enumerate(b_lines): match_found = bre_m.search(b_cur_line) - else: - match_found = b_line == b_cur_line.rstrip(b('\r\n')) - if match_found: - index[0] = lineno - m = match_found - elif bre_ins is not None and bre_ins.search(b_cur_line): - if insertafter: - # + 1 for the next line - index[1] = lineno + 1 - if firstmatch: - break - if insertbefore: - # index[1] for the previous line - index[1] = lineno + if match_found: + index[0] = lineno + m = match_found if firstmatch: break + # 2. When no match found on the previous step, + # parse for searching insertafter/insertbefore: + if not m: + for lineno, b_cur_line in enumerate(b_lines): + match_found = b_line == b_cur_line.rstrip(b'\r\n') + if match_found: + index[0] = lineno + m = match_found + + elif bre_ins is not None and bre_ins.search(b_cur_line): + if insertafter: + # + 1 for the next line + index[1] = lineno + 1 + if firstmatch: + break + + if insertbefore: + # index[1] for the previous line + index[1] = lineno + if firstmatch: + break + msg = '' changed = False b_linesep = to_bytes(os.linesep, errors='surrogate_or_strict') @@ -331,17 +351,17 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, 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')): + 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: + 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: + 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 @@ -350,12 +370,12 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, # 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: + if 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] - 1].rstrip(b('\r\n')) != b_line: + elif b_lines[index[1] - 1].rstrip(b'\r\n') != b_line: b_lines.insert(index[1], b_line + b_linesep) msg = 'line added' changed = True @@ -380,12 +400,21 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, elif insertafter == 'EOF' or index[1] == -1: # If the file is not empty then ensure there's a newline before the added line - if b_lines and not b_lines[-1][-1:] in (b('\n'), b('\r')): + if b_lines and not b_lines[-1][-1:] in (b'\n', b'\r'): b_lines.append(b_linesep) b_lines.append(b_line + b_linesep) msg = 'line added' changed = True + + elif insertafter and index[1] != -1: + + # Don't insert the line if it already matches at the index + if b_line != b_lines[index[1]].rstrip(b'\n\r'): + b_lines.insert(index[1], b_line + b_linesep) + msg = 'line added' + changed = True + # insert matched, but not the regexp else: b_lines.insert(index[1], b_line + b_linesep) @@ -393,7 +422,7 @@ def present(module, dest, regexp, line, insertafter, insertbefore, create, changed = True if module._diff: - diff['after'] = to_native(b('').join(b_lines)) + diff['after'] = to_native(b''.join(b_lines)) backupdest = "" if changed and not module.check_mode: @@ -430,7 +459,7 @@ def absent(module, dest, regexp, line, backup): b_lines = f.readlines() if module._diff: - diff['before'] = to_native(b('').join(b_lines)) + diff['before'] = to_native(b''.join(b_lines)) if regexp is not None: bre_c = re.compile(to_bytes(regexp, errors='surrogate_or_strict')) @@ -442,7 +471,7 @@ def absent(module, dest, regexp, line, backup): 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')) + match_found = b_line == b_cur_line.rstrip(b'\r\n') if match_found: found.append(b_cur_line) return not match_found @@ -451,7 +480,7 @@ def absent(module, dest, regexp, line, backup): changed = len(found) > 0 if module._diff: - diff['after'] = to_native(b('').join(b_lines)) + diff['after'] = to_native(b''.join(b_lines)) backupdest = "" if changed and not module.check_mode: diff --git a/test/integration/targets/lineinfile/files/firstmatch.txt b/test/integration/targets/lineinfile/files/firstmatch.txt new file mode 100644 index 00000000000..347132c6cba --- /dev/null +++ b/test/integration/targets/lineinfile/files/firstmatch.txt @@ -0,0 +1,5 @@ +line1 +line1 +line1 +line2 +line3 diff --git a/test/integration/targets/lineinfile/files/test_58923.txt b/test/integration/targets/lineinfile/files/test_58923.txt new file mode 100644 index 00000000000..34579fde33a --- /dev/null +++ b/test/integration/targets/lineinfile/files/test_58923.txt @@ -0,0 +1,4 @@ +#!/bin/sh + +case "`uname`" in + Darwin*) if [ -z "$JAVA_HOME" ] ; then diff --git a/test/integration/targets/lineinfile/tasks/main.yml b/test/integration/targets/lineinfile/tasks/main.yml index 7aa37f15a83..3ede5ded6bf 100644 --- a/test/integration/targets/lineinfile/tasks/main.yml +++ b/test/integration/targets/lineinfile/tasks/main.yml @@ -818,3 +818,281 @@ 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. + +################################################################### +## Issue #58923 +## Using firstmatch with insertafter and ensure multiple lines are not inserted + +- name: Deploy the firstmatch test file + copy: + src: firstmatch.txt + dest: "{{ output_dir }}/firstmatch.txt" + register: result + +- name: Assert that the test file was deployed + assert: + that: + - result is changed + - result.checksum == '1d644e5e2e51c67f1bd12d7bbe2686017f39923d' + - result.state == 'file' + +- name: Insert a line before an existing line using firstmatch + lineinfile: + path: "{{ output_dir }}/firstmatch.txt" + line: INSERT + insertafter: line1 + firstmatch: yes + register: insertafter1 + +- name: Insert a line before an existing line using firstmatch again + lineinfile: + path: "{{ output_dir }}/firstmatch.txt" + line: INSERT + insertafter: line1 + firstmatch: yes + register: insertafter2 + +- name: Stat the file + stat: + path: "{{ output_dir }}/firstmatch.txt" + register: result + +- name: Assert that the file was modified appropriately + assert: + that: + - insertafter1 is changed + - insertafter2 is not changed + - result.stat.checksum == '114aae024073a3ee8ec8db0ada03c5483326dd86' + +######################################################################################## +# Tests of fixing the same issue as above (#58923) by @Andersson007 +# and @samdoran : + +# Test insertafter with regexp +- name: Deploy the test file + copy: + src: test_58923.txt + dest: "{{ output_dir }}/test_58923.txt" + register: initial_file + +- name: Assert that the test file was deployed + assert: + that: + - initial_file is changed + - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e' + - initial_file.state == 'file' + +# Regarding the documentation: +# If regular expressions are passed to both regexp and +# insertafter, insertafter is only honored if no match for regexp is found. +# Therefore, +# when regular expressions are passed to both regexp and insertafter, then: +# 1. regexp was found -> ignore insertafter, replace the founded line +# 2. regexp was not found -> insert the line after 'insertafter' line + +# Regexp is not present in the file, so the line must be inserted after ^#!/bin/sh +- name: Add the line using firstmatch, regexp, and insertafter + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertafter: '^#!/bin/sh' + regexp: ^export FISHEYE_OPTS + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertafter_test1 + +- name: Stat the file + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertafter_test1_file + +- name: Add the line using firstmatch, regexp, and insertafter again + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertafter: '^#!/bin/sh' + regexp: ^export FISHEYE_OPTS + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertafter_test2 + +# Check of the prev step. +# We tried to add the same line with the same playbook, +# so nothing has been added: +- name: Stat the file again + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertafter_test2_file + +- name: Assert insertafter tests gave the expected results + assert: + that: + - insertafter_test1 is changed + - insertafter_test1_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08' + - insertafter_test2 is not changed + - insertafter_test2_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08' + +# Test insertafter without regexp +- name: Deploy the test file + copy: + src: test_58923.txt + dest: "{{ output_dir }}/test_58923.txt" + register: initial_file + +- name: Assert that the test file was deployed + assert: + that: + - initial_file is changed + - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e' + - initial_file.state == 'file' + +- name: Insert the line using firstmatch and insertafter without regexp + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertafter: '^#!/bin/sh' + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertafter_test3 + +- name: Stat the file + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertafter_test3_file + +- name: Insert the line using firstmatch and insertafter without regexp again + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertafter: '^#!/bin/sh' + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertafter_test4 + +- name: Stat the file again + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertafter_test4_file + +- name: Assert insertafter without regexp tests gave the expected results + assert: + that: + - insertafter_test3 is changed + - insertafter_test3_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08' + - insertafter_test4 is not changed + - insertafter_test4_file.stat.checksum == '9232aed6fe88714964d9e29d13e42cd782070b08' + + +# Test insertbefore with regexp +- name: Deploy the test file + copy: + src: test_58923.txt + dest: "{{ output_dir }}/test_58923.txt" + register: initial_file + +- name: Assert that the test file was deployed + assert: + that: + - initial_file is changed + - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e' + - initial_file.state == 'file' + +- name: Add the line using regexp, firstmatch, and insertbefore + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertbefore: '^#!/bin/sh' + regexp: ^export FISHEYE_OPTS + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertbefore_test1 + +- name: Stat the file + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertbefore_test1_file + +- name: Add the line using regexp, firstmatch, and insertbefore again + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertbefore: '^#!/bin/sh' + regexp: ^export FISHEYE_OPTS + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertbefore_test2 + +- name: Stat the file again + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertbefore_test2_file + +- name: Assert insertbefore with regexp tests gave the expected results + assert: + that: + - insertbefore_test1 is changed + - insertbefore_test1_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7' + - insertbefore_test2 is not changed + - insertbefore_test2_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7' + + +# Test insertbefore without regexp +- name: Deploy the test file + copy: + src: test_58923.txt + dest: "{{ output_dir }}/test_58923.txt" + register: initial_file + +- name: Assert that the test file was deployed + assert: + that: + - initial_file is changed + - initial_file.checksum == 'b6379ba43261c451a62102acb2c7f438a177c66e' + - initial_file.state == 'file' + +- name: Add the line using insertbefore and firstmatch + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertbefore: '^#!/bin/sh' + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertbefore_test3 + +- name: Stat the file + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertbefore_test3_file + +- name: Add the line using insertbefore and firstmatch again + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertbefore: '^#!/bin/sh' + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertbefore_test4 + +- name: Stat the file again + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertbefore_test4_file + +# Test when the line is presented in the file but +# not in the before/after spot and it does match the regexp: +- name: > + Add the line using insertbefore and firstmatch when the regexp line + is presented but not close to insertbefore spot + lineinfile: + path: "{{ output_dir }}/test_58923.txt" + insertbefore: ' Darwin\*\) if \[ -z \"\$JAVA_HOME\" \] ; then' + firstmatch: true + line: export FISHEYE_OPTS="-Xmx4096m -Xms2048m" + register: insertbefore_test5 + +- name: Stat the file again + stat: + path: "{{ output_dir }}/test_58923.txt" + register: insertbefore_test5_file + +- name: Assert insertbefore with regexp tests gave the expected results + assert: + that: + - insertbefore_test3 is changed + - insertbefore_test3_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7' + - insertbefore_test4 is not changed + - insertbefore_test4_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7' + - insertbefore_test5 is not changed + - insertbefore_test5_file.stat.checksum == '3c6630b9d44f561ea9ad999be56a7504cadc12f7'