From 1f4d1dfa0b94b8dd45762726c30c2b204b3416cd Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Mon, 12 Jul 2021 01:50:15 -0400 Subject: [PATCH] [stable-2.11] command - remove unreachable code and achieve full test coverage (#75036) (#75044) (cherry picked from commit 4ab791d501771d34fef6edb8f2f7932ed3885687) --- .../command-remove-unreachable-code.yml | 2 + lib/ansible/modules/command.py | 5 +- .../targets/command_shell/tasks/main.yml | 129 ++++++++++++++++++ 3 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/command-remove-unreachable-code.yml diff --git a/changelogs/fragments/command-remove-unreachable-code.yml b/changelogs/fragments/command-remove-unreachable-code.yml new file mode 100644 index 00000000000..a5b237a9ab9 --- /dev/null +++ b/changelogs/fragments/command-remove-unreachable-code.yml @@ -0,0 +1,2 @@ +bugfixes: + - command - remove unreachable code path when trying to convert the value for ``chdir`` to bytes (https://github.com/ansible/ansible/pull/75036) diff --git a/lib/ansible/modules/command.py b/lib/ansible/modules/command.py index 2f53ea83693..96d460eca3f 100644 --- a/lib/ansible/modules/command.py +++ b/lib/ansible/modules/command.py @@ -303,10 +303,7 @@ def main(): args = [to_native(arg, errors='surrogate_or_strict', nonstring='simplerepr') for arg in args] if chdir: - try: - chdir = to_bytes(os.path.abspath(chdir), errors='surrogate_or_strict') - except ValueError as e: - module.fail_json(msg='Unable to use supplied chdir: %s' % to_text(e)) + chdir = to_bytes(chdir, errors='surrogate_or_strict') try: os.chdir(chdir) diff --git a/test/integration/targets/command_shell/tasks/main.yml b/test/integration/targets/command_shell/tasks/main.yml index 1b2644bff18..3e7edcf1cd8 100644 --- a/test/integration/targets/command_shell/tasks/main.yml +++ b/test/integration/targets/command_shell/tasks/main.yml @@ -132,12 +132,21 @@ chdir: "{{ output_dir_test }}" register: command_result2 +- name: Check invalid chdir + command: echo + args: + chdir: "{{ output_dir }}/nope" + ignore_errors: yes + register: chdir_invalid + - name: assert that the script executed correctly with chdir assert: that: - command_result2.rc == 0 - command_result2.stderr == '' - command_result2.stdout == 'win' + - chdir_invalid is failed + - chdir_invalid.msg is search('Unable to change directory') # creates @@ -391,3 +400,123 @@ file: path: "{{ output_dir_test }}/afile.txt" state: absent + +- name: test warning with command + command: + cmd: "rm -h" + warn: yes + ignore_errors: yes + register: warn_result + +- name: assert warning exists + assert: + that: + - warn_result.warnings | length == 1 + - "'Consider using the file module with state=absent rather than running \\'rm\\'' in warn_result.warnings[0]" + +- name: test warning with shell + shell: "sed -h" + args: + warn: yes + ignore_errors: yes + register: warn_result + +- name: assert warning exists + assert: + that: + - warn_result.warnings | length == 1 + - "'Consider using the replace, lineinfile or template module rather than running \\'sed\\'' in warn_result.warnings[0]" + +- name: test become warning + command: + cmd: "sudo true" + warn: yes + ignore_errors: yes + register: warn_result + +- name: assert warning exists + assert: + that: + - warn_result.warnings | length == 1 + - "'Consider using \\'become\\', \\'become_method\\', and \\'become_user\\' rather than running sudo' in warn_result.warnings[0]" + +- name: test check mode skip message + command: + cmd: "true" + check_mode: yes + register: result + +- name: assert check message exists + assert: + that: + - "'skipped, running in check mode' in result.msg" + +- name: test check mode creates/removes message + command: + cmd: "true" + creates: yes + check_mode: yes + register: result + +- name: assert check message exists + assert: + that: + - "'Command would have run if not in check mode' in result.stderr" + +- name: command symlink handling + block: + - name: Create target folders + file: + path: '{{output_dir}}/www_root/site' + state: directory + + - name: Create symlink + file: + path: '{{output_dir}}/www' + state: link + src: '{{output_dir}}/www_root' + + - name: check parent using chdir + shell: dirname "$PWD" + args: + chdir: '{{output_dir}}/www/site' + register: parent_dir_chdir + + - name: check parent using cd + shell: cd "{{output_dir}}/www/site" && dirname "$PWD" + register: parent_dir_cd + + - name: check expected outputs + assert: + that: + - parent_dir_chdir.stdout != parent_dir_cd.stdout + - 'parent_dir_cd.stdout == "{{output_dir}}/www"' + - 'parent_dir_chdir.stdout == "{{output_dir}}/www_root"' + +- name: Set print error command for Python 2 + set_fact: + print_error_command: print >> sys.stderr, msg + when: ansible_facts.python_version is version('3', '<') + +- name: Set print error command for Python 3 + set_fact: + print_error_command: print(msg, file=sys.stderr) + when: ansible_facts.python_version is version('3', '>=') + +- name: run command with strip + command: '{{ ansible_playbook_python}} -c "import sys; msg=''hello \n \r''; print(msg); {{ print_error_command }}"' + register: command_strip + +- name: run command without strip + command: '{{ ansible_playbook_python}} -c "import sys; msg=''hello \n \r''; print(msg); {{ print_error_command }}"' + args: + strip_empty_ends: no + register: command_no_strip + +- name: Verify strip behavior worked as expected + assert: + that: + - command_strip.stdout == 'hello \n ' + - command_strip.stderr == 'hello \n ' + - command_no_strip.stdout== 'hello \n \r\n' + - command_no_strip.stderr == 'hello \n \r\n'