From da63f32d59fe882bc77532e734af7348b65cb6cb Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Fri, 25 Aug 2023 10:27:26 -0700 Subject: [PATCH] script: add argument validation (#81469) partially fixes: #81349 Signed-off-by: Abhijeet Kasurde --- lib/ansible/modules/command.py | 2 +- lib/ansible/modules/script.py | 19 +++++++----- lib/ansible/plugins/action/script.py | 29 ++++++++++++++----- .../integration/targets/script/tasks/main.yml | 11 +++++++ 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/lib/ansible/modules/command.py b/lib/ansible/modules/command.py index 846ac36afaf..c305952975c 100644 --- a/lib/ansible/modules/command.py +++ b/lib/ansible/modules/command.py @@ -276,7 +276,7 @@ def main(): strip = module.params['strip_empty_ends'] expand_argument_vars = module.params['expand_argument_vars'] - # we promissed these in 'always' ( _lines get autoaded on action plugin) + # we promised these in 'always' ( _lines get auto-added on action plugin) r = {'changed': False, 'stdout': '', 'stderr': '', 'rc': None, 'cmd': None, 'start': None, 'end': None, 'delta': None, 'msg': ''} if not shell and executable: diff --git a/lib/ansible/modules/script.py b/lib/ansible/modules/script.py index e9d3c244f47..c96da0f6f34 100644 --- a/lib/ansible/modules/script.py +++ b/lib/ansible/modules/script.py @@ -12,15 +12,16 @@ version_added: "0.9" short_description: Runs a local script on a remote node after transferring it description: - The M(ansible.builtin.script) module takes the script name followed by a list of space-delimited arguments. - - Either a free form command or O(cmd) parameter is required, see the examples. - - The local script at path will be transferred to the remote node and then executed. + - Either a free-form command or O(cmd) parameter is required, see the examples. + - The local script at the path will be transferred to the remote node and then executed. - The given script will be processed through the shell environment on the remote node. - - This module does not require python on the remote system, much like the M(ansible.builtin.raw) module. + - This module does not require Python on the remote system, much like the M(ansible.builtin.raw) module. - This module is also supported for Windows targets. options: free_form: description: - Path to the local script file followed by optional arguments. + type: str cmd: type: str description: @@ -29,26 +30,30 @@ options: description: - A filename on the remote node, when it already exists, this step will B(not) be run. version_added: "1.5" + type: str removes: description: - A filename on the remote node, when it does not exist, this step will B(not) be run. version_added: "1.5" + type: str chdir: description: - Change into this directory on the remote node before running the script. version_added: "2.4" + type: str executable: description: - - Name or path of a executable to invoke the script with. + - Name or path of an executable to invoke the script with. version_added: "2.6" + type: str notes: - It is usually preferable to write Ansible modules rather than pushing scripts. Convert your script to an Ansible module for bonus points! - The P(ansible.builtin.ssh#connection) connection plugin will force pseudo-tty allocation via C(-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. + please switch to a set of tasks that comprises M(ansible.builtin.copy) with M(ansible.builtin.command) instead of using M(ansible.builtin.script). - If the path to the local script contains spaces, it needs to be quoted. - This module is also supported for Windows targets. - - If the script returns non UTF-8 data, it must be encoded to avoid issues. One option is to pipe + - If the script returns non-UTF-8 data, it must be encoded to avoid issues. One option is to pipe the output through C(base64). seealso: - module: ansible.builtin.shell @@ -106,6 +111,6 @@ EXAMPLES = r''' args: executable: python3 -- name: Run a Powershell script on a windows host +- name: Run a Powershell script on a Windows host script: subdirectories/under/path/with/your/playbook/script.ps1 ''' diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index 8008a4e703a..e6ebd094167 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -40,11 +40,25 @@ class ActionModule(ActionBase): if task_vars is None: task_vars = dict() + validation_result, new_module_args = self.validate_argument_spec( + argument_spec={ + '_raw_params': {}, + 'cmd': {'type': 'str'}, + 'creates': {'type': 'str'}, + 'removes': {'type': 'str'}, + 'chdir': {'type': 'str'}, + 'executable': {'type': 'str'}, + }, + required_one_of=[ + ['_raw_params', 'cmd'] + ] + ) + result = super(ActionModule, self).run(tmp, task_vars) del tmp # tmp no longer has any effect try: - creates = self._task.args.get('creates') + creates = new_module_args['creates'] if creates: # do not run the command if the line contains creates=filename # and the filename already exists. This allows idempotence @@ -52,7 +66,7 @@ class ActionModule(ActionBase): if self._remote_file_exists(creates): raise AnsibleActionSkip("%s exists, matching creates option" % creates) - removes = self._task.args.get('removes') + removes = new_module_args['removes'] if removes: # do not run the command if the line contains removes=filename # and the filename does not exist. This allows idempotence @@ -62,7 +76,7 @@ class ActionModule(ActionBase): # The chdir must be absolute, because a relative path would rely on # remote node behaviour & user config. - chdir = self._task.args.get('chdir') + chdir = new_module_args['chdir'] if chdir: # Powershell is the only Windows-path aware shell if getattr(self._connection._shell, "_IS_WINDOWS", False) and \ @@ -75,13 +89,14 @@ class ActionModule(ActionBase): # 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') + raw_params = to_native(new_module_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] # Support executable paths and files with spaces in the name. - executable = to_native(self._task.args.get('executable', ''), errors='surrogate_or_strict') - + executable = new_module_args['executable'] + if executable: + executable = to_native(new_module_args['executable'], errors='surrogate_or_strict') try: source = self._loader.get_real_file(self._find_needle('files', source), decrypt=self._task.args.get('decrypt', True)) except AnsibleError as e: @@ -90,7 +105,7 @@ class ActionModule(ActionBase): if self._task.check_mode: # check mode is supported if 'creates' or 'removes' are provided # the task has already been skipped if a change would not occur - if self._task.args.get('creates') or self._task.args.get('removes'): + if new_module_args['creates'] or new_module_args['removes']: result['changed'] = True raise _AnsibleActionDone(result=result) # If the script doesn't return changed in the result, it defaults to True, diff --git a/test/integration/targets/script/tasks/main.yml b/test/integration/targets/script/tasks/main.yml index 989513d5319..8720ae020b1 100644 --- a/test/integration/targets/script/tasks/main.yml +++ b/test/integration/targets/script/tasks/main.yml @@ -37,6 +37,17 @@ ## script ## +- name: Required one of free-form and cmd + script: + ignore_errors: yes + register: script_required + +- name: assert that the script fails if neither free-form nor cmd is given + assert: + that: + - script_required.failed + - "'one of the following' in script_required.msg" + - name: execute the test.sh script via command script: test.sh register: script_result0