From 8b0c46a51a33834d1eac7dc578197ea2eedcb712 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Fri, 28 Sep 2018 16:04:50 -0400 Subject: [PATCH] Use bytes rather than native string for result (#46281) This prevents a stack trace in Python 3 when the result is an empty file since the file is open in binary mode and a native string in Python 3 is str, not bytes. (cherry picked from commit 8b1ae30e2ee86cdb8b68944b94846bbaf46228b4) --- .../fragments/blockinfile-bytes-fix.yaml | 2 + lib/ansible/modules/files/blockinfile.py | 2 +- .../targets/blockinfile/tasks/main.yml | 93 ++++++++++++++----- 3 files changed, 73 insertions(+), 24 deletions(-) create mode 100644 changelogs/fragments/blockinfile-bytes-fix.yaml diff --git a/changelogs/fragments/blockinfile-bytes-fix.yaml b/changelogs/fragments/blockinfile-bytes-fix.yaml new file mode 100644 index 00000000000..48c5755b2f5 --- /dev/null +++ b/changelogs/fragments/blockinfile-bytes-fix.yaml @@ -0,0 +1,2 @@ +bugfixes: + - blockinfile - use bytes rather than a native string to prevent a stacktrace in Python 3 when writing to the file (https://github.com/ansible/ansible/issues/46237) diff --git a/lib/ansible/modules/files/blockinfile.py b/lib/ansible/modules/files/blockinfile.py index 26beb497d5e..50977ffd1ff 100644 --- a/lib/ansible/modules/files/blockinfile.py +++ b/lib/ansible/modules/files/blockinfile.py @@ -309,7 +309,7 @@ def main(): if original is None or original.endswith(b('\n')): result += b('\n') else: - result = '' + result = b'' if module._diff: diff['after'] = result diff --git a/test/integration/targets/blockinfile/tasks/main.yml b/test/integration/targets/blockinfile/tasks/main.yml index e1a17d358d3..86e066d944e 100644 --- a/test/integration/targets/blockinfile/tasks/main.yml +++ b/test/integration/targets/blockinfile/tasks/main.yml @@ -16,13 +16,18 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . -- set_fact: output_dir_test={{output_dir}}/test_blockinfile +- set_fact: + output_dir_test: "{{ output_dir }}/test_blockinfile" - name: make sure our testing sub-directory does not exist - file: path="{{ output_dir_test }}" state=absent + file: + path: "{{ output_dir_test }}" + state: absent - name: create our testing sub-directory - file: path="{{ output_dir_test }}" state=directory + file: + path: "{{ output_dir_test }}" + state: directory ## ## blockinfile @@ -30,38 +35,80 @@ - name: copy the sshd_config to the test dir copy: - src: sshd_config - dest: "{{ output_dir_test }}" + 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 + path: "{{ output_dir_test }}/sshd_config" + block: | + Match User ansible-agent + PasswordAuthentication no register: blockinfile_test0 + - name: check content shell: 'grep -e "Match User ansible-agent" -e "PasswordAuthentication no" {{ output_dir_test }}/sshd_config' register: blockinfile_test0_grep -- debug: var=blockinfile_test0 -- debug: var=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_lines | length == 2' + that: + - 'blockinfile_test0.changed is defined' + - 'blockinfile_test0.msg is defined' + - 'blockinfile_test0.changed' + - 'blockinfile_test0.msg == "Block inserted"' + - 'blockinfile_test0_grep.stdout_lines | length == 2' - name: check idemptotence blockinfile: - path: "{{ output_dir_test }}/sshd_config" - block: | - Match User ansible-agent - PasswordAuthentication no + 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' + 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