From c422bc64dc3675644b6b72ef8649682d0b39f214 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Mon, 7 Dec 2020 18:32:56 -0500 Subject: [PATCH] [stable-2.10] blockinfile - properly insert block when no trailing new line exists (#72350) (#72360) (cherry picked from commit c51438312a) Co-authored-by: Sam Doran --- ...nfile-fix-insert-after-line-no-linesep.yml | 4 + lib/ansible/modules/blockinfile.py | 7 +- .../tasks/add_block_to_existing_file.yml | 47 ++++++ .../tasks/block_without_trailing_newline.yml | 30 ++++ .../targets/blockinfile/tasks/create_file.yml | 32 ++++ .../targets/blockinfile/tasks/diff.yml | 18 +++ .../tasks/file_without_trailing_newline.yml | 36 +++++ .../targets/blockinfile/tasks/insertafter.yml | 37 +++++ .../blockinfile/tasks/insertbefore.yml | 39 +++++ .../targets/blockinfile/tasks/main.yml | 149 ++---------------- .../tasks/preserve_line_endings.yml | 24 +++ .../targets/blockinfile/tasks/validate.yml | 28 ++++ 12 files changed, 309 insertions(+), 142 deletions(-) create mode 100644 changelogs/fragments/72055-blockinfile-fix-insert-after-line-no-linesep.yml create mode 100644 test/integration/targets/blockinfile/tasks/add_block_to_existing_file.yml create mode 100644 test/integration/targets/blockinfile/tasks/block_without_trailing_newline.yml create mode 100644 test/integration/targets/blockinfile/tasks/create_file.yml create mode 100644 test/integration/targets/blockinfile/tasks/diff.yml create mode 100644 test/integration/targets/blockinfile/tasks/file_without_trailing_newline.yml create mode 100644 test/integration/targets/blockinfile/tasks/insertafter.yml create mode 100644 test/integration/targets/blockinfile/tasks/insertbefore.yml create mode 100644 test/integration/targets/blockinfile/tasks/preserve_line_endings.yml create mode 100644 test/integration/targets/blockinfile/tasks/validate.yml diff --git a/changelogs/fragments/72055-blockinfile-fix-insert-after-line-no-linesep.yml b/changelogs/fragments/72055-blockinfile-fix-insert-after-line-no-linesep.yml new file mode 100644 index 00000000000..b4d52c004a7 --- /dev/null +++ b/changelogs/fragments/72055-blockinfile-fix-insert-after-line-no-linesep.yml @@ -0,0 +1,4 @@ +bugfixes: + - > + blockinfile - properly insert a block at the end of a file that does not + have a trailing newline character (https://github.com/ansible/ansible/issues/72055) diff --git a/lib/ansible/modules/blockinfile.py b/lib/ansible/modules/blockinfile.py index 38eecf574cb..2f80a65edcc 100644 --- a/lib/ansible/modules/blockinfile.py +++ b/lib/ansible/modules/blockinfile.py @@ -209,7 +209,6 @@ def main(): add_file_common_args=True, supports_check_mode=True ) - params = module.params path = params['path'] @@ -302,8 +301,12 @@ def main(): lines[n1:n0 + 1] = [] n0 = n1 - lines[n0:n0] = blocklines + # Ensure there is a line separator before the block of lines to be inserted + if n0 > 0: + if not lines[n0 - 1].endswith(b(os.linesep)): + lines[n0 - 1] += b(os.linesep) + lines[n0:n0] = blocklines if lines: result = b''.join(lines) else: diff --git a/test/integration/targets/blockinfile/tasks/add_block_to_existing_file.yml b/test/integration/targets/blockinfile/tasks/add_block_to_existing_file.yml new file mode 100644 index 00000000000..dbb93ecce3b --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/add_block_to_existing_file.yml @@ -0,0 +1,47 @@ +- name: copy the sshd_config to the test dir + copy: + src: sshd_config + dest: "{{ output_dir_test }}" + +- name: insert/update "Match User" configuration block in sshd_config + blockinfile: + path: "{{ output_dir_test }}/sshd_config" + block: | + Match User ansible-agent + PasswordAuthentication no + backup: yes + register: blockinfile_test0 + +- name: check content + shell: 'grep -c -e "Match User ansible-agent" -e "PasswordAuthentication no" {{ output_dir_test }}/sshd_config' + register: blockinfile_test0_grep + +- debug: + var: blockinfile_test0 + verbosity: 1 + +- debug: + var: blockinfile_test0_grep + verbosity: 1 + +- name: validate first example results + assert: + that: + - 'blockinfile_test0.changed is defined' + - 'blockinfile_test0.msg is defined' + - 'blockinfile_test0.changed' + - 'blockinfile_test0.msg == "Block inserted"' + - 'blockinfile_test0_grep.stdout == "2"' + +- name: check idemptotence + blockinfile: + path: "{{ output_dir_test }}/sshd_config" + block: | + Match User ansible-agent + PasswordAuthentication no + register: blockinfile_test1 + +- name: validate idempotence results + assert: + that: + - 'not blockinfile_test1.changed' diff --git a/test/integration/targets/blockinfile/tasks/block_without_trailing_newline.yml b/test/integration/targets/blockinfile/tasks/block_without_trailing_newline.yml new file mode 100644 index 00000000000..57dac60eb4d --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/block_without_trailing_newline.yml @@ -0,0 +1,30 @@ +- name: Add block without trailing line separator + blockinfile: + path: "{{ output_dir_test }}/chomped_block_test.txt" + create: yes + content: |- + one + two + three + register: chomptest1 + +- name: Add block without trailing line separator again + blockinfile: + path: "{{ output_dir_test }}/chomped_block_test.txt" + content: |- + one + two + three + register: chomptest2 + +- name: Check output file + stat: + path: "{{ output_dir_test }}/chomped_block_test.txt" + register: chomptest_file + +- name: Ensure chomptest results are correct + assert: + that: + - chomptest1 is changed + - chomptest2 is not changed + - chomptest_file.stat.checksum == '50d49f528a5f7147c7029ed6220c326b1ee2c4ae' diff --git a/test/integration/targets/blockinfile/tasks/create_file.yml b/test/integration/targets/blockinfile/tasks/create_file.yml new file mode 100644 index 00000000000..94e47203509 --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/create_file.yml @@ -0,0 +1,32 @@ +- name: Create a file with blockinfile + blockinfile: + path: "{{ output_dir_test }}/empty.txt" + block: | + Hey + there + state: present + create: yes + register: empty_test_1 + +- name: Run a task that results in an empty file + blockinfile: + path: "{{ output_dir_test }}/empty.txt" + block: | + Hey + there + state: absent + create: yes + register: empty_test_2 + +- stat: + path: "{{ output_dir_test }}/empty.txt" + register: empty_test_stat + +- name: Ensure empty file was created + assert: + that: + - empty_test_1 is changed + - "'File created' in empty_test_1.msg" + - empty_test_2 is changed + - "'Block removed' in empty_test_2.msg" + - empty_test_stat.stat.size == 0 diff --git a/test/integration/targets/blockinfile/tasks/diff.yml b/test/integration/targets/blockinfile/tasks/diff.yml new file mode 100644 index 00000000000..4a2f9454771 --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/diff.yml @@ -0,0 +1,18 @@ +- name: Create a test file + copy: + content: diff test + dest: "{{ output_dir_test }}/diff.txt" + +- name: Add block to file with diff + blockinfile: + path: "{{ output_dir_test }}/diff.txt" + block: | + line 1 + line 2 + register: difftest + diff: yes + +- name: Ensure diff was shown + assert: + that: + - difftest.diff | length > 0 diff --git a/test/integration/targets/blockinfile/tasks/file_without_trailing_newline.yml b/test/integration/targets/blockinfile/tasks/file_without_trailing_newline.yml new file mode 100644 index 00000000000..fe4e2abce26 --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/file_without_trailing_newline.yml @@ -0,0 +1,36 @@ +- name: Create file without trailing newline + copy: + content: '# File with no newline' + dest: "{{ output_dir_test }}/no_newline_at_end.txt" + register: no_newline + + +- name: Add block to file that does not have a newline at the end + blockinfile: + path: "{{ output_dir_test }}/no_newline_at_end.txt" + content: | + one + two + three + register: no_newline_test1 + +- name: Add block to file that does not have a newline at the end again + blockinfile: + path: "{{ output_dir_test }}/no_newline_at_end.txt" + content: | + one + two + three + register: no_newline_test2 + +- name: Stat the file + stat: + path: "{{ output_dir_test }}/no_newline_at_end.txt" + register: no_newline_file + +- name: Ensure block was correctly written to file with no newline at end + assert: + that: + - no_newline_test1 is changed + - no_newline_test2 is not changed + - no_newline_file.stat.checksum == 'dab16f864025e59125e74d1498ffb2bb048224e6' diff --git a/test/integration/targets/blockinfile/tasks/insertafter.yml b/test/integration/targets/blockinfile/tasks/insertafter.yml new file mode 100644 index 00000000000..daf7bcf1e90 --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/insertafter.yml @@ -0,0 +1,37 @@ +- name: Create insertafter test file + copy: + dest: "{{ output_dir }}/after.txt" + content: | + line1 + line2 + line3 + +- name: Add block using insertafter + blockinfile: + path: "{{ output_dir }}/after.txt" + insertafter: line2 + block: | + block1 + block2 + register: after1 + +- name: Add block using insertafter again + blockinfile: + path: "{{ output_dir }}/after.txt" + insertafter: line2 + block: | + block1 + block2 + register: after2 + +- name: Stat the after.txt file + stat: + path: "{{ output_dir }}/after.txt" + register: after_file + +- name: Ensure insertafter worked correctly + assert: + that: + - after1 is changed + - after2 is not changed + - after_file.stat.checksum == 'a8adeb971358230a28ce554f3b8fdd1ef65fdf1c' diff --git a/test/integration/targets/blockinfile/tasks/insertbefore.yml b/test/integration/targets/blockinfile/tasks/insertbefore.yml new file mode 100644 index 00000000000..6089af157d3 --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/insertbefore.yml @@ -0,0 +1,39 @@ +- name: Create insertbefore test file + copy: + dest: "{{ output_dir }}/before.txt" + content: | + line1 + line2 + line3 + +- name: Add block using insertbefore + blockinfile: + path: "{{ output_dir }}/before.txt" + insertbefore: line2 + block: | + block1 + block2 + register: after1 + +- name: Add block using insertbefore again + blockinfile: + path: "{{ output_dir }}/before.txt" + insertbefore: line2 + block: | + block1 + block2 + register: after2 + +- name: Stat the before.txt file + stat: + path: "{{ output_dir }}/before.txt" + register: after_file + +- command: cat {{ output_dir }}/before.txt + +- name: Ensure insertbefore worked correctly + assert: + that: + - after1 is changed + - after2 is not changed + - after_file.stat.checksum == '16681d1d7f29d173243bb951d6afb9c0824d7bf4' diff --git a/test/integration/targets/blockinfile/tasks/main.yml b/test/integration/targets/blockinfile/tasks/main.yml index ffcfd18d3ef..4bc0b8d16cd 100644 --- a/test/integration/targets/blockinfile/tasks/main.yml +++ b/test/integration/targets/blockinfile/tasks/main.yml @@ -29,143 +29,12 @@ path: "{{ output_dir_test }}" state: directory -## -## blockinfile -## - -- name: copy the sshd_config to the test dir - copy: - src: sshd_config - dest: "{{ output_dir_test }}" - -- name: insert/update "Match User" configuration block in sshd_config - blockinfile: - path: "{{ output_dir_test }}/sshd_config" - block: | - Match User ansible-agent - PasswordAuthentication no - register: blockinfile_test0 - -- name: check content - shell: 'grep -c -e "Match User ansible-agent" -e "PasswordAuthentication no" {{ output_dir_test }}/sshd_config' - register: blockinfile_test0_grep - -- debug: - var: blockinfile_test0 - verbosity: 1 - -- debug: - var: blockinfile_test0_grep - verbosity: 1 - -- name: validate first example results - assert: - that: - - 'blockinfile_test0.changed is defined' - - 'blockinfile_test0.msg is defined' - - 'blockinfile_test0.changed' - - 'blockinfile_test0.msg == "Block inserted"' - - 'blockinfile_test0_grep.stdout == "2"' - -- name: check idemptotence - blockinfile: - path: "{{ output_dir_test }}/sshd_config" - block: | - Match User ansible-agent - PasswordAuthentication no - register: blockinfile_test1 - -- name: validate idempotence results - assert: - that: - - 'not blockinfile_test1.changed' - -- name: Create a file with blockinfile - blockinfile: - path: "{{ output_dir_test }}/empty.txt" - block: | - Hey - there - state: present - create: yes - register: empty_test_1 - -- name: Run a task that results in an empty file - blockinfile: - path: "{{ output_dir_test }}/empty.txt" - block: | - Hey - there - state: absent - create: yes - register: empty_test_2 - -- stat: - path: "{{ output_dir_test }}/empty.txt" - register: empty_test_stat - -- name: Ensure empty file was created - assert: - that: - - empty_test_1 is changed - - "'File created' in empty_test_1.msg" - - empty_test_2 is changed - - "'Block removed' in empty_test_2.msg" - - empty_test_stat.stat.size == 0 - -- name: create line_endings_test.txt in the test dir - copy: - dest: "{{ output_dir_test }}/line_endings_test.txt" - # generating the content like this instead of copying a fixture file - # prevents sanity checks from warning about mixed line endings - content: "unix\nunix\nunix\n\ndos\r\ndos\r\ndos\r\n\nunix\nunix\n# BEGIN ANSIBLE MANAGED BLOCK\ndos\r\n# END ANSIBLE MANAGED BLOCK\nunix\nunix\nunix\nunix\n" - -- name: insert/update "dos" configuration block in line_endings_test.txt - blockinfile: - path: "{{ output_dir_test }}/line_endings_test.txt" - block: "dos\r\ndos\r\ndos\r\n" - register: blockinfile_test2 - -- name: check content - # using the more precise `grep -Pc "^dos\\r$" ...` fails on BSD/macOS - shell: 'grep -c "^dos.$" {{ output_dir_test }}/line_endings_test.txt' - register: blockinfile_test2_grep - -- name: validate line_endings_test.txt results - assert: - that: - - 'blockinfile_test2 is changed' - - 'blockinfile_test2.msg == "Block inserted"' - - 'blockinfile_test2_grep.stdout == "6"' - - -- name: Add block without trailing line separator - blockinfile: - path: "{{ output_dir_test }}/chomped_block_test.txt" - create: yes - content: |- - one - two - three - register: chomptest1 - -- name: Add block without trailing line separator again - blockinfile: - path: "{{ output_dir_test }}/chomped_block_test.txt" - content: |- - one - two - three - register: chomptest2 - -- name: Check output file - stat: - path: "{{ output_dir_test }}/chomped_block_test.txt" - register: chomptest_file - -- name: Ensure chomptest results are correct - assert: - that: - - chomptest1 is changed - - chomptest2 is not changed - - chomptest_file.stat.checksum == '50d49f528a5f7147c7029ed6220c326b1ee2c4ae' +- import_tasks: add_block_to_existing_file.yml +- import_tasks: create_file.yml +- import_tasks: preserve_line_endings.yml +- import_tasks: block_without_trailing_newline.yml +- import_tasks: file_without_trailing_newline.yml +- import_tasks: diff.yml +- import_tasks: validate.yml +- import_tasks: insertafter.yml +- import_tasks: insertbefore.yml diff --git a/test/integration/targets/blockinfile/tasks/preserve_line_endings.yml b/test/integration/targets/blockinfile/tasks/preserve_line_endings.yml new file mode 100644 index 00000000000..bb2dee29742 --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/preserve_line_endings.yml @@ -0,0 +1,24 @@ +- name: create line_endings_test.txt in the test dir + copy: + dest: "{{ output_dir_test }}/line_endings_test.txt" + # generating the content like this instead of copying a fixture file + # prevents sanity checks from warning about mixed line endings + content: "unix\nunix\nunix\n\ndos\r\ndos\r\ndos\r\n\nunix\nunix\n# BEGIN ANSIBLE MANAGED BLOCK\ndos\r\n# END ANSIBLE MANAGED BLOCK\nunix\nunix\nunix\nunix\n" + +- name: insert/update "dos" configuration block in line_endings_test.txt + blockinfile: + path: "{{ output_dir_test }}/line_endings_test.txt" + block: "dos\r\ndos\r\ndos\r\n" + register: blockinfile_test2 + +- name: check content + # using the more precise `grep -Pc "^dos\\r$" ...` fails on BSD/macOS + shell: 'grep -c "^dos.$" {{ output_dir_test }}/line_endings_test.txt' + register: blockinfile_test2_grep + +- name: validate line_endings_test.txt results + assert: + that: + - 'blockinfile_test2 is changed' + - 'blockinfile_test2.msg == "Block inserted"' + - 'blockinfile_test2_grep.stdout == "6"' diff --git a/test/integration/targets/blockinfile/tasks/validate.yml b/test/integration/targets/blockinfile/tasks/validate.yml new file mode 100644 index 00000000000..105bca53346 --- /dev/null +++ b/test/integration/targets/blockinfile/tasks/validate.yml @@ -0,0 +1,28 @@ +- name: EXPECTED FAILURE test improper validate + blockinfile: + path: "{{ output_dir }}/validate.txt" + block: | + line1 + line2 + create: yes + validate: grep + ignore_errors: yes + +- name: EXPECTED FAILURE test failure to validate + blockinfile: + path: "{{ output_dir }}/validate.txt" + block: | + line1 + line2 + create: yes + validate: grep line47 %s + ignore_errors: yes + +- name: Test proper validate + blockinfile: + path: "{{ output_dir }}/validate.txt" + block: | + line1 + line2 + create: yes + validate: grep line1 %s