From f297229b5203f1404ca95a059c4218f212cb9761 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Tue, 12 Mar 2019 07:56:51 +1000 Subject: [PATCH] Add arg and doc validation for PowerShell modules (#53615) * Add arg and doc validation for PowerShell modules * Verify if pwsh exists before running it --- .../modules/windows/win_chocolatey.ps1 | 4 +- lib/ansible/modules/windows/win_get_url.py | 2 +- lib/ansible/modules/windows/win_psexec.py | 4 ++ lib/ansible/modules/windows/win_say.py | 1 - lib/ansible/modules/windows/win_stat.py | 1 + lib/ansible/modules/windows/win_tempfile.py | 1 + lib/ansible/modules/windows/win_uri.py | 4 +- .../modules/windows/win_wait_for_process.py | 2 +- test/sanity/validate-modules/main.py | 13 ++++- test/sanity/validate-modules/module_args.py | 37 +++++++++++- test/sanity/validate-modules/ps_argspec.ps1 | 58 +++++++++++++++++++ test/sanity/validate-modules/schema.py | 4 +- test/sanity/validate-modules/utils.py | 39 +++++++++++++ 13 files changed, 159 insertions(+), 11 deletions(-) create mode 100755 test/sanity/validate-modules/ps_argspec.ps1 diff --git a/lib/ansible/modules/windows/win_chocolatey.ps1 b/lib/ansible/modules/windows/win_chocolatey.ps1 index 5e7b3b6227b..4b3274a664f 100644 --- a/lib/ansible/modules/windows/win_chocolatey.ps1 +++ b/lib/ansible/modules/windows/win_chocolatey.ps1 @@ -24,7 +24,7 @@ $spec = @{ ignore_dependencies = @{ type = "bool"; default = $false } force = @{ type = "bool"; default = $false } name = @{ type = "list"; elements = "str"; required = $true } - package_params = @{ type = "str"; aliases = "params" } + package_params = @{ type = "str"; aliases = @("params") } pinned = @{ type = "bool" } proxy_url = @{ type = "str" } proxy_username = @{ type = "str" } @@ -34,7 +34,7 @@ $spec = @{ source_username = @{ type = "str" } source_password = @{ type = "str"; no_log = $true } state = @{ type = "str"; default = "present"; choices = "absent", "downgrade", "latest", "present", "reinstalled" } - timeout = @{ type = "int"; default = 2700; aliases = "execution_timeout" } + timeout = @{ type = "int"; default = 2700; aliases = @("execution_timeout") } validate_certs = @{ type = "bool"; default = $true } version = @{ type = "str" } } diff --git a/lib/ansible/modules/windows/win_get_url.py b/lib/ansible/modules/windows/win_get_url.py index 16ff6fd785c..21b56c23b0a 100644 --- a/lib/ansible/modules/windows/win_get_url.py +++ b/lib/ansible/modules/windows/win_get_url.py @@ -90,7 +90,7 @@ options: - md5 - sha1 - sha256 - - sha385 + - sha384 - sha512 default: sha1 version_added: "2.8" diff --git a/lib/ansible/modules/windows/win_psexec.py b/lib/ansible/modules/windows/win_psexec.py index c3fc37e4a60..7346a84f337 100644 --- a/lib/ansible/modules/windows/win_psexec.py +++ b/lib/ansible/modules/windows/win_psexec.py @@ -29,6 +29,10 @@ options: - The location of the PsExec utility (in case it is not located in your PATH). type: path default: psexec.exe + extra_opts: + description: + - Specify additional options to add onto the PsExec invocation. + type: list hostnames: description: - The hostnames to run the command. diff --git a/lib/ansible/modules/windows/win_say.py b/lib/ansible/modules/windows/win_say.py index cfd921631fa..b0ce27258a7 100644 --- a/lib/ansible/modules/windows/win_say.py +++ b/lib/ansible/modules/windows/win_say.py @@ -38,7 +38,6 @@ options: - If the requested voice is not available the default voice will be used. Example voice names from Windows 10 are C(Microsoft Zira Desktop) and C(Microsoft Hazel Desktop). type: str - default: system default voice speech_speed: description: - How fast or slow to speak the text. diff --git a/lib/ansible/modules/windows/win_stat.py b/lib/ansible/modules/windows/win_stat.py index 746c32d7641..5a8b8a6579a 100644 --- a/lib/ansible/modules/windows/win_stat.py +++ b/lib/ansible/modules/windows/win_stat.py @@ -26,6 +26,7 @@ options: back slashes are accepted. type: path required: yes + aliases: [ dest, name ] get_md5: description: - Whether to return the checksum sum of the file. Between Ansible 1.9 diff --git a/lib/ansible/modules/windows/win_tempfile.py b/lib/ansible/modules/windows/win_tempfile.py index 19d6c19166d..3efe1a18e90 100644 --- a/lib/ansible/modules/windows/win_tempfile.py +++ b/lib/ansible/modules/windows/win_tempfile.py @@ -29,6 +29,7 @@ options: - If path is not specified default system temporary directory (%TEMP%) will be used. type: path default: '%TEMP%' + aliases: [ dest ] prefix: description: - Prefix of file/directory name created by module. diff --git a/lib/ansible/modules/windows/win_uri.py b/lib/ansible/modules/windows/win_uri.py index 59e8efc9062..d018065a6a0 100644 --- a/lib/ansible/modules/windows/win_uri.py +++ b/lib/ansible/modules/windows/win_uri.py @@ -91,7 +91,7 @@ options: - A valid, numeric, HTTP status code that signifies success of the request. - Can also be comma separated list of status codes. type: list - default: 200 + default: [ 200 ] version_added: '2.4' timeout: description: @@ -123,7 +123,7 @@ options: or C(follow_redirects) is set to C(none), or set to C(safe) when not doing C(GET) or C(HEAD) it prevents all redirection. type: int - default: 5 + default: 50 version_added: '2.4' validate_certs: description: diff --git a/lib/ansible/modules/windows/win_wait_for_process.py b/lib/ansible/modules/windows/win_wait_for_process.py index 8d1883de12c..a1e062f77c6 100644 --- a/lib/ansible/modules/windows/win_wait_for_process.py +++ b/lib/ansible/modules/windows/win_wait_for_process.py @@ -23,7 +23,7 @@ options: process_name_exact: description: - The name of the process(es) for which to wait. - type: str + type: list process_name_pattern: description: - RegEx pattern matching desired process(es). diff --git a/test/sanity/validate-modules/main.py b/test/sanity/validate-modules/main.py index bc9505278d4..13593d644c3 100755 --- a/test/sanity/validate-modules/main.py +++ b/test/sanity/validate-modules/main.py @@ -230,6 +230,9 @@ class ModuleValidator(Validator): 'slurp.ps1', 'setup.ps1' )) + PS_ARG_VALIDATE_BLACKLIST = frozenset(( + 'win_dsc.ps1', # win_dsc is a dynamic arg spec, the docs won't ever match + )) WHITELIST_FUTURE_IMPORTS = frozenset(('absolute_import', 'division', 'print_function')) @@ -773,6 +776,7 @@ class ModuleValidator(Validator): code=503, msg='Missing python documentation file' ) + return py_path def _get_docs(self): docs = { @@ -1531,7 +1535,14 @@ class ModuleValidator(Validator): if self._powershell_module(): self._validate_ps_replacers() - self._find_ps_docs_py_file() + docs_path = self._find_ps_docs_py_file() + + # We can only validate PowerShell arg spec if it is using the new Ansible.Basic.AnsibleModule util + pattern = r'(?im)^#\s*ansiblerequires\s+\-csharputil\s*Ansible\.Basic' + if re.search(pattern, self.text) and self.object_name not in self.PS_ARG_VALIDATE_BLACKLIST: + with ModuleValidator(docs_path, base_branch=self.base_branch, git_cache=self.git_cache) as docs_mv: + docs = docs_mv._validate_docs()[1] + self._validate_ansible_module_call(docs) self._check_gpl3_header() if not self._just_docs() and not end_of_deprecation_should_be_removed_only: diff --git a/test/sanity/validate-modules/module_args.py b/test/sanity/validate-modules/module_args.py index 4a7644c2090..0745c7c3813 100644 --- a/test/sanity/validate-modules/module_args.py +++ b/test/sanity/validate-modules/module_args.py @@ -17,12 +17,17 @@ # along with this program. If not, see . import imp +import json +import os +import subprocess import sys from contextlib import contextmanager from ansible.module_utils.six import reraise +from utils import find_executable + class AnsibleModuleCallError(RuntimeError): pass @@ -75,7 +80,29 @@ def setup_env(filename): del sys.modules[k] -def get_argument_spec(filename): +def get_ps_argument_spec(filename): + # This uses a very small skeleton of Ansible.Basic.AnsibleModule to return the argspec defined by the module. This + # is pretty rudimentary and will probably require something better going forward. + pwsh = find_executable('pwsh') + if not pwsh: + raise FileNotFoundError('Required program for PowerShell arg spec inspection "pwsh" not found.') + + script_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'ps_argspec.ps1') + proc = subprocess.Popen([script_path, filename], stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=False) + stdout, stderr = proc.communicate() + + if proc.returncode != 0: + raise AnsibleModuleImportError(stderr.decode('utf-8')) + + kwargs = json.loads(stdout) + + # the validate-modules code expects the options spec to be under the argument_spec key not options as set in PS + kwargs['argument_spec'] = kwargs.pop('options', {}) + + return kwargs['argument_spec'], (), kwargs + + +def get_py_argument_spec(filename): with setup_env(filename) as fake: try: # We use ``module`` here instead of ``__main__`` @@ -91,8 +118,16 @@ def get_argument_spec(filename): try: try: + # for ping kwargs == {'argument_spec':{'data':{'type':'str','default':'pong'}}, 'supports_check_mode':True} return fake.kwargs['argument_spec'], fake.args, fake.kwargs except KeyError: return fake.args[0], fake.args, fake.kwargs except TypeError: return {}, (), {} + + +def get_argument_spec(filename): + if filename.endswith('.py'): + return get_py_argument_spec(filename) + else: + return get_ps_argument_spec(filename) diff --git a/test/sanity/validate-modules/ps_argspec.ps1 b/test/sanity/validate-modules/ps_argspec.ps1 new file mode 100755 index 00000000000..d4fef59afb7 --- /dev/null +++ b/test/sanity/validate-modules/ps_argspec.ps1 @@ -0,0 +1,58 @@ +#!/usr/bin/env pwsh +#Requires -Version 6 + +Set-StrictMode -Version 2.0 +$ErrorActionPreference = "Stop" +$WarningPreference = "Stop" + +$module_path = $args[0] +if (-not $module_path) { + Write-Error -Message "No module specified." + exit 1 +} + +# Check if the path is relative and get the full path to the module +if (-not ([System.IO.Path]::IsPathRooted($module_path))) { + $module_path = $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($module_path) +} + +if (-not (Test-Path -LiteralPath $module_path -PathType Leaf)) { + Write-Error -Message "The module at '$module_path' does not exist." + exit 1 +} + +$dummy_ansible_basic = @' +using System; +using System.Collections; +using System.Management.Automation; + +namespace Ansible.Basic +{ + public class AnsibleModule + { + public AnsibleModule(string[] args, IDictionary argumentSpec) + { + PSObject rawOut = ScriptBlock.Create("ConvertTo-Json -InputObject $args[0] -Depth 99 -Compress").Invoke(argumentSpec)[0]; + Console.WriteLine(rawOut.BaseObject.ToString()); + ScriptBlock.Create("Set-Variable -Name LASTEXITCODE -Value 0 -Scope Global; exit 0").Invoke(); + } + + public static AnsibleModule Create(string[] args, IDictionary argumentSpec) + { + return new AnsibleModule(args, argumentSpec); + } + } +} +'@ +Add-Type -TypeDefinition $dummy_ansible_basic + +$module_code = Get-Content -LiteralPath $module_path -Raw + +$powershell = [PowerShell]::Create() +$powershell.AddScript($module_code) > $null +$powershell.Invoke() > $null + +if ($powershell.HadErrors) { + $powershell.Streams.Error + exit 1 +} diff --git a/test/sanity/validate-modules/schema.py b/test/sanity/validate-modules/schema.py index 2feb41c0180..e00c7f6843c 100644 --- a/test/sanity/validate-modules/schema.py +++ b/test/sanity/validate-modules/schema.py @@ -81,7 +81,7 @@ suboption_schema = Schema( 'version_added': Any(float, *string_types), 'default': Any(None, float, int, bool, list, dict, *string_types), # Note: Types are strings, not literal bools, such as True or False - 'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'str'), + 'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'), # Recursive suboptions 'suboptions': Any(None, *list({str_type: Self} for str_type in string_types)), }, @@ -102,7 +102,7 @@ option_schema = Schema( 'default': Any(None, float, int, bool, list, dict, *string_types), 'suboptions': Any(None, *list_dict_suboption_schema), # Note: Types are strings, not literal bools, such as True or False - 'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'str'), + 'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'), }, extra=PREVENT_EXTRA ) diff --git a/test/sanity/validate-modules/utils.py b/test/sanity/validate-modules/utils.py index 121abe0afcd..fde2e1597df 100644 --- a/test/sanity/validate-modules/utils.py +++ b/test/sanity/validate-modules/utils.py @@ -17,6 +17,7 @@ # along with this program. If not, see . import ast +import os import sys from io import BytesIO, TextIOWrapper @@ -33,6 +34,44 @@ class AnsibleTextIOWrapper(TextIOWrapper): super(AnsibleTextIOWrapper, self).write(to_text(s, self.encoding, errors='replace')) +def find_executable(executable, cwd=None, path=None): + """Finds the full path to the executable specified""" + # This is mostly a copy from test/runner/lib/util.py. Should be removed once validate-modules has been integrated + # into ansible-test + match = None + real_cwd = os.getcwd() + + if not cwd: + cwd = real_cwd + + if os.path.dirname(executable): + target = os.path.join(cwd, executable) + if os.path.exists(target) and os.access(target, os.F_OK | os.X_OK): + match = executable + else: + path = os.environ.get('PATH', os.path.defpath) + + path_dirs = path.split(os.path.pathsep) + seen_dirs = set() + + for path_dir in path_dirs: + if path_dir in seen_dirs: + continue + + seen_dirs.add(path_dir) + + if os.path.abspath(path_dir) == real_cwd: + path_dir = cwd + + candidate = os.path.join(path_dir, executable) + + if os.path.exists(candidate) and os.access(candidate, os.F_OK | os.X_OK): + match = candidate + break + + return match + + def find_globals(g, tree): """Uses AST to find globals in an ast tree""" for child in tree: