diff --git a/lib/ansible/modules/commands/script.py b/lib/ansible/modules/commands/script.py index ad89d48d004..2d186a33083 100644 --- a/lib/ansible/modules/commands/script.py +++ b/lib/ansible/modules/commands/script.py @@ -26,7 +26,7 @@ description: options: free_form: description: - - path to the local script file followed by optional arguments. There is no parameter actually named 'free form'; see the examples! + - Path to the local script file followed by optional arguments. There is no parameter actually named 'free form'; see the examples! required: true default: null aliases: [] @@ -53,6 +53,7 @@ notes: - The ssh connection plugin will force pseudo-tty allocation via -tt when scripts are executed. pseudo-ttys do not have a stderr channel and all stderr is sent to stdout. If you depend on separated stdout and stderr result keys, please switch to a copy+command set of tasks instead of using script. - This module is also supported for Windows targets. + - If the path to the local script contains spaces, it needs to be quoted. author: - Ansible Core Team - Michael DeHaan diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index f37964da3cf..3549e233212 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -19,9 +19,10 @@ __metaclass__ = type import os import re +import shlex from ansible.errors import AnsibleError -from ansible.module_utils._text import to_native +from ansible.module_utils._text import to_native, to_text from ansible.plugins.action import ActionBase @@ -72,30 +73,46 @@ class ActionModule(ActionBase): if self._connection._shell.SHELL_FAMILY != 'powershell' and not chdir.startswith('/'): return dict(failed=True, msg='chdir %s must be an absolute path for a Unix-aware remote node' % chdir) - # the script name is the first item in the raw params, so we split it - # out now so we know the file name we need to transfer to the remote, - # and everything else is an argument to the script which we need later - # to append to the remote command - parts = self._task.args.get('_raw_params', '').strip().split() + # Split out the script as the first item in raw_params using + # shlex.split() in order to support paths and files with spaces in the name. + # Any arguments passed to the script will be added back later. + raw_params = to_native(self._task.args.get('_raw_params', ''), errors='surrogate_or_strict') + parts = [to_text(s, errors='surrogate_or_strict') for s in shlex.split(raw_params.strip())] source = parts[0] - args = ' '.join(parts[1:]) try: source = self._loader.get_real_file(self._find_needle('files', source), decrypt=self._task.args.get('decrypt', True)) except AnsibleError as e: return dict(failed=True, msg=to_native(e)) - # transfer the file to a remote tmp location - tmp_src = self._connection._shell.join_path(tmp, os.path.basename(source)) - self._transfer_file(source, tmp_src) + if not self._play_context.check_mode: + # transfer the file to a remote tmp location + tmp_src = self._connection._shell.join_path(tmp, os.path.basename(source)) - # set file permissions, more permissive when the copy is done as a different user - self._fixup_perms2((tmp, tmp_src), execute=True) + # Convert raw_params to text for the purpose of replacing the script since + # parts and tmp_src are both unicode strings and raw_params will be different + # depending on Python version. + # + # Once everything is encoded consistently, replace the script path on the remote + # system with the remainder of the raw_params. This preserves quoting in parameters + # that would have been removed by shlex.split(). + target_command = to_text(raw_params).strip().replace(parts[0], tmp_src) + + self._transfer_file(source, tmp_src) + + # set file permissions, more permissive when the copy is done as a different user + self._fixup_perms2((tmp, tmp_src), execute=True) + + # add preparation steps to one ssh roundtrip executing the script + env_dict = dict() + env_string = self._compute_environment_string(env_dict) + script_cmd = ' '.join([env_string, target_command]) + + if self._play_context.check_mode: + result['changed'] = True + self._remove_tmp_path(tmp) + return result - # add preparation steps to one ssh roundtrip executing the script - env_dict = dict() - env_string = self._compute_environment_string(env_dict) - script_cmd = ' '.join([env_string, tmp_src, args]) script_cmd = self._connection._shell.wrap_for_exec(script_cmd) exec_data = None diff --git a/test/integration/targets/script/files/space path/test.sh b/test/integration/targets/script/files/space path/test.sh new file mode 100755 index 00000000000..6f6334d71fa --- /dev/null +++ b/test/integration/targets/script/files/space path/test.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +echo -n "Script with space in path" \ No newline at end of file diff --git a/test/integration/targets/script/files/test_with_args.sh b/test/integration/targets/script/files/test_with_args.sh new file mode 100755 index 00000000000..13dce4f24c7 --- /dev/null +++ b/test/integration/targets/script/files/test_with_args.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +for i in "$@"; do + echo "$i" +done \ No newline at end of file diff --git a/test/integration/targets/script/tasks/main.yml b/test/integration/targets/script/tasks/main.yml index 7b314441901..85c9a8dcbc2 100644 --- a/test/integration/targets/script/tasks/main.yml +++ b/test/integration/targets/script/tasks/main.yml @@ -20,13 +20,18 @@ ## prep ## -- set_fact: output_dir_test={{output_dir}}/test_script +- set_fact: + output_dir_test: "{{ output_dir }}/test_script" - 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 ## ## script @@ -43,36 +48,101 @@ - "script_result0.stderr == ''" - "script_result0.stdout == 'win'" -# creates +- name: Execute a script with a space in the path + script: "'space path/test.sh'" + register: _space_path_test + tags: + - spacepath + +- name: Assert that script with space in path ran successfully + assert: + that: + - _space_path_test is success + - _space_path_test.stdout == 'Script with space in path' + tags: + - spacepath + +- name: Execute a script with arguments including a unicode character + script: test_with_args.sh -this -that -Ӧther + register: unicode_args +- name: Assert that script with unicode character ran successfully + assert: + that: + - unicode_args is success + - unicode_args.stdout_lines[0] == '-this' + - unicode_args.stdout_lines[1] == '-that' + - unicode_args.stdout_lines[2] == '-Ӧther' + +# creates - name: verify that afile.txt is absent - file: path={{output_dir_test}}/afile.txt state=absent + file: + path: "{{ output_dir_test }}/afile.txt" + state: absent - name: create afile.txt with create_afile.sh via command - script: create_afile.sh {{output_dir_test | expanduser}}/afile.txt creates={{output_dir_test | expanduser}}/afile.txt + script: create_afile.sh {{ output_dir_test | expanduser }}/afile.txt + args: + creates: "{{ output_dir_test | expanduser }}/afile.txt" + register: _create_test1 + +- name: Check state of created file + stat: + path: "{{ output_dir_test | expanduser }}/afile.txt" + register: _create_stat1 + +- name: Run create_afile.sh again to ensure it is skipped + script: create_afile.sh {{ output_dir_test | expanduser }}/afile.txt + args: + creates: "{{ output_dir_test | expanduser }}/afile.txt" + register: _create_test2 + +- name: Assert that script report a change, file was created, second run was skipped + assert: + that: + - _create_test1 | changed + - _create_stat1.stat.exists + - _create_test2 | skipped -- name: verify that afile.txt is present - file: path={{output_dir_test}}/afile.txt state=file # removes +- name: verify that afile.txt is present + file: + path: "{{ output_dir_test }}/afile.txt" + state: file - name: remove afile.txt with remote_afile.sh via command - script: remove_afile.sh {{output_dir_test | expanduser}}/afile.txt removes={{output_dir_test | expanduser}}/afile.txt - register: script_result1 - -- name: verify that afile.txt is absent - file: path={{output_dir_test}}/afile.txt state=absent - register: script_result2 - -- name: assert that the file was removed by the script + script: remove_afile.sh {{ output_dir_test | expanduser }}/afile.txt + args: + removes: "{{ output_dir_test | expanduser }}/afile.txt" + register: _remove_test1 + +- name: Check state of removed file + stat: + path: "{{ output_dir_test | expanduser }}/afile.txt" + register: _remove_stat1 + +- name: Run remote_afile.sh again to enure it is skipped + script: remove_afile.sh {{ output_dir_test | expanduser }}/afile.txt + args: + removes: "{{ output_dir_test | expanduser }}/afile.txt" + register: _remove_test2 + +- name: Assert that script report a change, file was removed, second run was skipped assert: that: - - "script_result1|changed" - - "script_result2.state == 'absent'" + - _remove_test1 | changed + - not _remove_stat1.stat.exists + - _remove_test2 | skipped + # async -- name: test task failure with async param +- name: verify that afile.txt is absent + file: + path: "{{ output_dir_test }}/afile.txt" + state: absent +- name: test task failure with async param script: /some/script.sh async: 2 ignore_errors: true @@ -81,5 +151,73 @@ - name: assert task with async param failed assert: that: - - script_result3|failed + - script_result3 | failed - script_result3.msg == "async is not supported for this task." + + +# check mode +- name: Run script to create a file in check mode + script: create_afile.sh {{ output_dir_test | expanduser }}/afile2.txt + check_mode: yes + register: _check_mode_test + +- debug: + var: _check_mode_test + verbosity: 2 + +- name: Get state of file created by script + stat: + path: "{{ output_dir_test | expanduser }}/afile2.txt" + register: _afile_stat + +- debug: + var: _afile_stat + verbosity: 2 + +- name: Assert that a change was reported but the script did not make changes + assert: + that: + - _check_mode_test | changed + - not _afile_stat.stat.exists + +- name: Run script to create a file + script: create_afile.sh {{ output_dir_test | expanduser }}/afile2.txt + +- name: Run script to create a file in check mode with 'creates' argument + script: create_afile.sh {{ output_dir_test | expanduser }}/afile2.txt + args: + creates: "{{ output_dir_test | expanduser }}/afile2.txt" + register: _check_mode_test2 + check_mode: yes + +- debug: + var: _check_mode_test2 + verbosity: 2 + +- name: Assert that task was skipped and mesage was returned + assert: + that: + - _check_mode_test2 | skipped + - '_check_mode_test2.msg == "skipped, since {{ output_dir_test | expanduser }}/afile2.txt exists"' + +- name: Remove afile2.txt + file: + path: "{{ output_dir_test | expanduser }}/afile2.txt" + state: absent + +- name: Run script to remove a file in check mode with 'removes' argument + script: remove_afile.sh {{ output_dir_test | expanduser }}/afile2.txt + args: + removes: "{{ output_dir_test | expanduser }}/afile2.txt" + register: _check_mode_test3 + check_mode: yes + +- debug: + var: _check_mode_test3 + verbosity: 2 + +- name: Assert that task was skipped and message was returned + assert: + that: + - _check_mode_test3 | skipped + - '_check_mode_test3.msg == "skipped, since {{ output_dir_test | expanduser }}/afile2.txt does not exist"' diff --git a/test/integration/targets/win_script/files/space path/test_script.ps1 b/test/integration/targets/win_script/files/space path/test_script.ps1 new file mode 100644 index 00000000000..24e0ab2ed55 --- /dev/null +++ b/test/integration/targets/win_script/files/space path/test_script.ps1 @@ -0,0 +1 @@ +Write-Host "Ansible supports spaces in the path to the script." diff --git a/test/integration/targets/win_script/tasks/main.yml b/test/integration/targets/win_script/tasks/main.yml index 1a9c25924f3..32b637630af 100644 --- a/test/integration/targets/win_script/tasks/main.yml +++ b/test/integration/targets/win_script/tasks/main.yml @@ -35,8 +35,8 @@ - "test_script_result.stdout" - "'Woohoo' in test_script_result.stdout" - "not test_script_result.stderr" - - "not test_script_result|failed" - - "test_script_result|changed" + - "not test_script_result | failed" + - "test_script_result | changed" - name: run test script that takes arguments including a unicode char script: test_script_with_args.ps1 /this /that /Ӧther @@ -51,8 +51,8 @@ - "test_script_with_args_result.stdout_lines[1] == '/that'" - "test_script_with_args_result.stdout_lines[2] == '/Ӧther'" - "not test_script_with_args_result.stderr" - - "not test_script_with_args_result|failed" - - "test_script_with_args_result|changed" + - "not test_script_with_args_result | failed" + - "test_script_with_args_result | changed" - name: run test script that takes parameters passed via splatting script: test_script_with_splatting.ps1 @{ This = 'this'; That = '{{ test_win_script_value }}'; Other = 'other'} @@ -67,8 +67,8 @@ - "test_script_with_splatting_result.stdout_lines[1] == test_win_script_value" - "test_script_with_splatting_result.stdout_lines[2] == 'other'" - "not test_script_with_splatting_result.stderr" - - "not test_script_with_splatting_result|failed" - - "test_script_with_splatting_result|changed" + - "not test_script_with_splatting_result | failed" + - "test_script_with_splatting_result | changed" - name: run test script that takes splatted parameters from a variable script: test_script_with_splatting.ps1 {{ test_win_script_splat }} @@ -83,8 +83,8 @@ - "test_script_with_splatting2_result.stdout_lines[1] == 'THAT'" - "test_script_with_splatting2_result.stdout_lines[2] == 'OTHER'" - "not test_script_with_splatting2_result.stderr" - - "not test_script_with_splatting2_result|failed" - - "test_script_with_splatting2_result|changed" + - "not test_script_with_splatting2_result | failed" + - "test_script_with_splatting2_result | changed" - name: run test script that has errors script: test_script_with_errors.ps1 @@ -97,15 +97,17 @@ - "test_script_with_errors_result.rc != 0" - "not test_script_with_errors_result.stdout" - "test_script_with_errors_result.stderr" - - "test_script_with_errors_result|failed" - - "test_script_with_errors_result|changed" + - "test_script_with_errors_result | failed" + - "test_script_with_errors_result | changed" - name: cleanup test file if it exists - raw: Remove-Item "{{test_win_script_filename}}" -Force + raw: Remove-Item "{{ test_win_script_filename }}" -Force ignore_errors: true - name: run test script that creates a file - script: test_script_creates_file.ps1 "{{test_win_script_filename}}" creates="{{test_win_script_filename}}" + script: test_script_creates_file.ps1 {{ test_win_script_filename }} + args: + creates: "{{ test_win_script_filename }}" register: test_script_creates_file_result - name: check that script ran and indicated a change @@ -114,22 +116,26 @@ - "test_script_creates_file_result.rc == 0" - "not test_script_creates_file_result.stdout" - "not test_script_creates_file_result.stderr" - - "not test_script_creates_file_result|failed" - - "test_script_creates_file_result|changed" + - "not test_script_creates_file_result | failed" + - "test_script_creates_file_result | changed" - name: run test script that creates a file again - script: test_script_creates_file.ps1 "{{test_win_script_filename}}" creates="{{test_win_script_filename}}" + script: test_script_creates_file.ps1 {{ test_win_script_filename }} + args: + creates: "{{ test_win_script_filename }}" register: test_script_creates_file_again_result - name: check that the script did not run since the remote file exists assert: that: - - "not test_script_creates_file_again_result|failed" - - "not test_script_creates_file_again_result|changed" - - "test_script_creates_file_again_result|skipped" + - "not test_script_creates_file_again_result | failed" + - "not test_script_creates_file_again_result | changed" + - "test_script_creates_file_again_result | skipped" - name: run test script that removes a file - script: test_script_removes_file.ps1 "{{test_win_script_filename}}" removes="{{test_win_script_filename}}" + script: test_script_removes_file.ps1 {{ test_win_script_filename }} + args: + removes: "{{ test_win_script_filename }}" register: test_script_removes_file_result - name: check that the script ran since the remote file exists @@ -138,19 +144,21 @@ - "test_script_removes_file_result.rc == 0" - "not test_script_removes_file_result.stdout" - "not test_script_removes_file_result.stderr" - - "not test_script_removes_file_result|failed" - - "test_script_removes_file_result|changed" + - "not test_script_removes_file_result | failed" + - "test_script_removes_file_result | changed" - name: run test script that removes a file again - script: test_script_removes_file.ps1 "{{test_win_script_filename}}" removes="{{test_win_script_filename}}" + script: test_script_removes_file.ps1 {{ test_win_script_filename }} + args: + removes: "{{ test_win_script_filename }}" register: test_script_removes_file_again_result - name: check that the script did not run since the remote file does not exist assert: that: - - "not test_script_removes_file_again_result|failed" - - "not test_script_removes_file_again_result|changed" - - "test_script_removes_file_again_result|skipped" + - "not test_script_removes_file_again_result | failed" + - "not test_script_removes_file_again_result | changed" + - "test_script_removes_file_again_result | skipped" # TODO: these tests fail on 2008 (not even R2) with no output. It's related to the default codepage being UTF8- if we force it back to 437, it works fine. # Need to figure out a sane place to do that under the new exec wrapper. @@ -165,8 +173,8 @@ # - "test_batch_result.stdout" # - "'batch' in test_batch_result.stdout" # - "not test_batch_result.stderr" -# - "not test_batch_result|failed" -# - "test_batch_result|changed" +# - "not test_batch_result | failed" +# - "test_batch_result | changed" #- name: run simple batch file with .cmd extension @@ -180,8 +188,8 @@ # - "test_cmd_result.stdout" # - "'cmd extension' in test_cmd_result.stdout" # - "not test_cmd_result.stderr" -# - "not test_cmd_result|failed" -# - "test_cmd_result|changed" +# - "not test_cmd_result | failed" +# - "test_cmd_result | changed" - name: run test script that takes a boolean parameter script: test_script_bool.ps1 $true @@ -205,4 +213,47 @@ # that: # - test_script_env_result | succeeded # - test_script_env_result.stdout_lines[0] == 'task' -# \ No newline at end of file +# + + +# check mode +- name: Run test script that creates a file in check mode + script: test_script_creates_file.ps1 {{ test_win_script_filename }} + args: + creates: "{{ test_win_script_filename }}" + check_mode: yes + register: test_script_creates_file_check_mode + +- name: Get state of file created by script + win_stat: + path: "{{ test_win_script_filename }}" + register: create_file_stat + +- name: Assert that a change was reported but the script did not make changes + assert: + that: + - test_script_creates_file_check_mode | changed + - not create_file_stat.stat.exists + +- name: Run test script that creates a file + script: test_script_creates_file.ps1 {{ test_win_script_filename }} + args: + creates: "{{ test_win_script_filename }}" + +- name: Run test script that removes a file in check mode + script: test_script_removes_file.ps1 {{ test_win_script_filename }} + args: + removes: "{{ test_win_script_filename }}" + check_mode: yes + register: test_script_removes_file_check_mode + +- name: Get state of file removed by script + win_stat: + path: "{{ test_win_script_filename }}" + register: remove_file_stat + +- name: Assert that a change was reported but the script did not make changes + assert: + that: + - test_script_removes_file_check_mode | changed + - remove_file_stat.stat.exists