[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 <sdoran@redhat.com>

12 files changed, 309 insertions, 142 deletions

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 0000000000..b4d52c004a
--- /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 38eecf574c..2f80a65edc 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 0000000000..dbb93ecce3
--- /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 0000000000..57dac60eb4
--- /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 0000000000..94e4720350
--- /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 0000000000..4a2f945477
--- /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 0000000000..fe4e2abce2
--- /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 0000000000..daf7bcf1e9
--- /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 0000000000..6089af157d
--- /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 ffcfd18d3e..4bc0b8d16c 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 0000000000..bb2dee2974
--- /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 0000000000..105bca5334
--- /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