From 6db66bcaddaafc0c650031745543573f0209a419 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 1 Apr 2020 09:01:18 +1000 Subject: [PATCH] Add relative module_util support for powershell (#68321) * Add relative module_util support for powershell * Added ansible-test classification support --- .../fragments/win_collection_relative.yaml | 2 + lib/ansible/executor/module_common.py | 28 +++++----- .../executor/powershell/module_manifest.py | 51 ++++++++++++------- lib/ansible/plugins/action/script.py | 2 +- .../collections_relative_imports/aliases | 3 ++ .../my_col/plugins/module_utils/PSRel1.psm1 | 11 ++++ .../plugins/module_utils/sub_pkg/PSRel2.psm1 | 11 ++++ .../my_col/plugins/modules/win_relative.ps1 | 10 ++++ .../my_col2/plugins/module_utils/PSRel3.psm1 | 11 ++++ .../plugins/module_utils/sub_pkg/CSRel4.cs | 14 +++++ .../collections_relative_imports/runme.sh | 10 +++- .../collections_relative_imports/windows.yml | 11 ++++ .../_internal/csharp_import_analysis.py | 8 ++- .../_internal/powershell_import_analysis.py | 8 ++- .../lib/ansible_test/_internal/util_common.py | 21 ++++++++ 15 files changed, 164 insertions(+), 37 deletions(-) create mode 100644 changelogs/fragments/win_collection_relative.yaml create mode 100644 test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/PSRel1.psm1 create mode 100644 test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/sub_pkg/PSRel2.psm1 create mode 100644 test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/win_relative.ps1 create mode 100644 test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/PSRel3.psm1 create mode 100644 test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/sub_pkg/CSRel4.cs create mode 100644 test/integration/targets/collections_relative_imports/windows.yml diff --git a/changelogs/fragments/win_collection_relative.yaml b/changelogs/fragments/win_collection_relative.yaml new file mode 100644 index 00000000000..538c41bcb3d --- /dev/null +++ b/changelogs/fragments/win_collection_relative.yaml @@ -0,0 +1,2 @@ +minor_changes: +- windows collections - Support relative module util imports in PowerShell modules and module_utils diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 3f5e2f1d730..a29502e5336 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -411,8 +411,8 @@ else: # Do this instead of getting site-packages from distutils.sysconfig so we work when we # haven't been installed site_packages = os.path.dirname(os.path.dirname(os.path.dirname(__file__))) -CORE_LIBRARY_PATH_RE = re.compile(r'%s/(?Pansible/modules/.*)\.py$' % site_packages) -COLLECTION_PATH_RE = re.compile(r'/(?Pansible_collections/[^/]+/[^/]+/plugins/modules/.*)\.py$') +CORE_LIBRARY_PATH_RE = re.compile(r'%s/(?Pansible/modules/.*)\.(py|ps1)$' % site_packages) +COLLECTION_PATH_RE = re.compile(r'/(?Pansible_collections/[^/]+/[^/]+/plugins/modules/.*)\.(py|ps1)$') # Detect new-style Python modules by looking for required imports: # import ansible_collections.[my_ns.my_col.plugins.module_utils.my_module_util] @@ -1000,6 +1000,17 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas output = BytesIO() py_module_names = set() + try: + remote_module_fqn = _get_ansible_module_fqn(module_path) + except ValueError: + # Modules in roles currently are not found by the fqn heuristic so we + # fallback to this. This means that relative imports inside a module from + # a role may fail. Absolute imports should be used for future-proofness. + # People should start writing collections instead of modules in roles so we + # may never fix this + display.debug('ANSIBALLZ: Could not determine module FQN') + remote_module_fqn = 'ansible.modules.%s' % module_name + if module_substyle == 'python': params = dict(ANSIBLE_MODULE_ARGS=module_args,) try: @@ -1013,17 +1024,6 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas display.warning(u'Bad module compression string specified: %s. Using ZIP_STORED (no compression)' % module_compression) compression_method = zipfile.ZIP_STORED - try: - remote_module_fqn = _get_ansible_module_fqn(module_path) - except ValueError: - # Modules in roles currently are not found by the fqn heuristic so we - # fallback to this. This means that relative imports inside a module from - # a role may fail. Absolute imports should be used for future-proofness. - # People should start writing collections instead of modules in roles so we - # may never fix this - display.debug('ANSIBALLZ: Could not determine module FQN') - remote_module_fqn = 'ansible.modules.%s' % module_name - lookup_path = os.path.join(C.DEFAULT_LOCAL_TMP, 'ansiballz_cache') cached_module_filename = os.path.join(lookup_path, "%s-%s" % (module_name, module_compression)) @@ -1191,7 +1191,7 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas b_module_data = ps_manifest._create_powershell_wrapper( b_module_data, module_path, module_args, environment, async_timeout, become, become_method, become_user, become_password, - become_flags, module_substyle, task_vars + become_flags, module_substyle, task_vars, remote_module_fqn ) elif module_substyle == 'jsonargs': diff --git a/lib/ansible/executor/powershell/module_manifest.py b/lib/ansible/executor/powershell/module_manifest.py index c3bc0fc2f1c..853ee264ce9 100644 --- a/lib/ansible/executor/powershell/module_manifest.py +++ b/lib/ansible/executor/powershell/module_manifest.py @@ -39,7 +39,7 @@ class PSModuleDepFinder(object): self.become = False self._re_cs_module = [ - # Reference C# module_util in another C# util + # Reference C# module_util in another C# util, this must always be the fully qualified name. # 'using ansible_collections.{namespace}.{collection}.plugins.module_utils.{name}' re.compile(to_bytes(r'(?i)^using\s((Ansible\..+)|' r'(ansible_collections\.\w+\.\w+\.plugins\.module_utils\.[\w\.]+));\s*$')), @@ -49,8 +49,10 @@ class PSModuleDepFinder(object): # Reference C# module_util in a PowerShell module # '#AnsibleRequires -CSharpUtil Ansible.{name}' # '#AnsibleRequires -CSharpUtil ansible_collections.{namespace}.{collection}.plugins.module_utils.{name}' + # '#AnsibleRequires -CSharpUtil ..module_utils.{name}' re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-csharputil\s+((Ansible\..+)|' - r'(ansible_collections\.\w+\.\w+\.plugins\.module_utils\.[\w\.]+))')), + r'(ansible_collections\.\w+\.\w+\.plugins\.module_utils\.[\w\.]+)|' + r'(\.[\w\.]+))')), ] self._re_ps_module = [ @@ -58,10 +60,12 @@ class PSModuleDepFinder(object): # '#Requires -Module Ansible.ModuleUtils.{name} re.compile(to_bytes(r'(?i)^#\s*requires\s+\-module(?:s?)\s*(Ansible\.ModuleUtils\..+)')), # New way of referencing a builtin and collection module_util - # '#AnsibleRequires -PowerShell ansible_collections.{namespace}.{collection}.plugins.module_utils.{name}' # '#AnsibleRequires -PowerShell Ansible.ModuleUtils.{name}' - re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-powershell\s+(((Ansible\.ModuleUtils\..+))|' - r'(ansible_collections\.\w+\.\w+\.plugins\.module_utils\.[\w\.]+))')), + # '#AnsibleRequires -PowerShell ansible_collections.{namespace}.{collection}.plugins.module_utils.{name}' + # '#AnsibleRequires -PowerShell ..module_utils.{name}' + re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-powershell\s+((Ansible\.ModuleUtils\..+)|' + r'(ansible_collections\.\w+\.\w+\.plugins\.module_utils\.[\w\.]+)|' + r'(\.[\w\.]+))')), ] self._re_wrapper = re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-wrapper\s+(\w*)')) @@ -69,7 +73,7 @@ class PSModuleDepFinder(object): self._re_os_version = re.compile(to_bytes(r'(?i)^#ansiblerequires\s+\-osversion\s+([0-9]+(\.[0-9]+){0,3})$')) self._re_become = re.compile(to_bytes(r'(?i)^#ansiblerequires\s+\-become$')) - def scan_module(self, module_data, wrapper=False, powershell=True): + def scan_module(self, module_data, fqn=None, wrapper=False, powershell=True): lines = module_data.split(b'\n') module_utils = set() if wrapper: @@ -80,9 +84,9 @@ class PSModuleDepFinder(object): if powershell: checks = [ # PS module contains '#Requires -Module Ansible.ModuleUtils.*' - # PS module contains '#AnsibleRequires -Powershell Ansible.*' (or FQ collections module_utils ref) + # PS module contains '#AnsibleRequires -Powershell Ansible.*' (or collections module_utils ref) (self._re_ps_module, self.ps_modules, ".psm1"), - # PS module contains '#AnsibleRequires -CSharpUtil Ansible.*' + # PS module contains '#AnsibleRequires -CSharpUtil Ansible.*' (or collections module_utils ref) (self._re_cs_in_ps_module, cs_utils, ".cs"), ] else: @@ -101,7 +105,7 @@ class PSModuleDepFinder(object): module_util_name = to_text(match.group(1).rstrip()) if module_util_name not in check[1].keys(): - module_utils.add((module_util_name, check[2])) + module_utils.add((module_util_name, check[2], fqn)) break @@ -153,8 +157,11 @@ class PSModuleDepFinder(object): self.scan_module(b_data, wrapper=True, powershell=True) def _add_module(self, name, wrapper=False): - m, ext = name + m, ext, fqn = name m = to_text(m) + + util_fqn = None + if m.startswith("Ansible."): # Builtin util, use plugin loader to get the data mu_path = ps_module_utils_loader.find_plugin(m, ext) @@ -166,14 +173,25 @@ class PSModuleDepFinder(object): module_util_data = to_bytes(_slurp(mu_path)) else: # Collection util, load the package data based on the util import. - submodules = tuple(m.split(".")) + + submodules = m.split(".") + if m.startswith('.'): + fqn_submodules = fqn.split('.') + for submodule in submodules: + if submodule: + break + del fqn_submodules[-1] + + submodules = fqn_submodules + [s for s in submodules if s] + n_package_name = to_native('.'.join(submodules[:-1]), errors='surrogate_or_strict') n_resource_name = to_native(submodules[-1] + ext, errors='surrogate_or_strict') try: - module_util = import_module(to_native(n_package_name)) + module_util = import_module(n_package_name) module_util_data = to_bytes(pkgutil.get_data(n_package_name, n_resource_name), errors='surrogate_or_strict') + util_fqn = to_text("%s.%s " % (n_package_name, submodules[-1]), errors='surrogate_or_strict') # Get the path of the util which is required for coverage collection. resource_paths = list(module_util.__path__) @@ -200,8 +218,7 @@ class PSModuleDepFinder(object): self.cs_utils_wrapper[m] = util_info else: self.cs_utils_module[m] = util_info - self.scan_module(module_util_data, wrapper=wrapper, - powershell=(ext == ".psm1")) + self.scan_module(module_util_data, fqn=util_fqn, wrapper=wrapper, powershell=(ext == ".psm1")) def _parse_version_match(self, match, attribute): new_version = to_text(match.group(1)).rstrip() @@ -255,7 +272,7 @@ def _strip_comments(source): def _create_powershell_wrapper(b_module_data, module_path, module_args, environment, async_timeout, become, become_method, become_user, become_password, - become_flags, substyle, task_vars): + become_flags, substyle, task_vars, module_fqn): # creates the manifest/wrapper used in PowerShell/C# modules to enable # things like become and async - this is also called in action/script.py @@ -266,7 +283,7 @@ def _create_powershell_wrapper(b_module_data, module_path, module_args, if substyle != 'script': # don't scan the module for util dependencies and other Ansible related # flags if the substyle is 'script' which is set by action/script - finder.scan_module(b_module_data, powershell=(substyle == "powershell")) + finder.scan_module(b_module_data, fqn=module_fqn, powershell=(substyle == "powershell")) module_wrapper = "module_%s_wrapper" % substyle exec_manifest = dict( @@ -327,7 +344,7 @@ def _create_powershell_wrapper(b_module_data, module_path, module_args, # make sure Ansible.ModuleUtils.AddType is added if any C# utils are used if len(finder.cs_utils_wrapper) > 0 or len(finder.cs_utils_module) > 0: - finder._add_module((b"Ansible.ModuleUtils.AddType", ".psm1"), + finder._add_module((b"Ansible.ModuleUtils.AddType", ".psm1", None), wrapper=False) # exec_wrapper is only required to be part of the payload if using diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index d4fcf6ffe91..ad73be88f4b 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -132,7 +132,7 @@ class ActionModule(ActionBase): exec_data = ps_manifest._create_powershell_wrapper( to_bytes(script_cmd), source, {}, env_dict, self._task.async_val, pc.become, pc.become_method, pc.become_user, - pc.become_pass, pc.become_flags, "script", task_vars + pc.become_pass, pc.become_flags, "script", task_vars, None ) # build the necessary exec wrapper command # FUTURE: this still doesn't let script work on Windows with non-pipelined connections or diff --git a/test/integration/targets/collections_relative_imports/aliases b/test/integration/targets/collections_relative_imports/aliases index a6dafcf8cd8..996481b4c8d 100644 --- a/test/integration/targets/collections_relative_imports/aliases +++ b/test/integration/targets/collections_relative_imports/aliases @@ -1 +1,4 @@ +posix shippable/posix/group1 +shippable/windows/group1 +windows diff --git a/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/PSRel1.psm1 b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/PSRel1.psm1 new file mode 100644 index 00000000000..bf812643a08 --- /dev/null +++ b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/PSRel1.psm1 @@ -0,0 +1,11 @@ +#AnsibleRequires -PowerShell .sub_pkg.PSRel2 + +Function Invoke-FromPSRel1 { + <# + .SYNOPSIS + Test function + #> + return "$(Invoke-FromPSRel2) -> Invoke-FromPSRel1" +} + +Export-ModuleMember -Function Invoke-FromPSRel1 diff --git a/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/sub_pkg/PSRel2.psm1 b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/sub_pkg/PSRel2.psm1 new file mode 100644 index 00000000000..d0aa3686ec7 --- /dev/null +++ b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/module_utils/sub_pkg/PSRel2.psm1 @@ -0,0 +1,11 @@ +#AnsibleRequires -PowerShell ansible_collections.my_ns.my_col2.plugins.module_utils.PSRel3 + +Function Invoke-FromPSRel2 { + <# + .SYNOPSIS + Test function + #> + return "$(Invoke-FromPSRel3) -> Invoke-FromPSRel2" +} + +Export-ModuleMember -Function Invoke-FromPSRel2 diff --git a/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/win_relative.ps1 b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/win_relative.ps1 new file mode 100644 index 00000000000..383df0a3769 --- /dev/null +++ b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col/plugins/modules/win_relative.ps1 @@ -0,0 +1,10 @@ +#!powershell + +#AnsibleRequires -CSharpUtil Ansible.Basic +#AnsibleRequires -PowerShell ..module_utils.PSRel1 + +$module = [Ansible.Basic.AnsibleModule]::Create($args, @{}) + +$module.Result.data = Invoke-FromPSRel1 + +$module.ExitJson() diff --git a/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/PSRel3.psm1 b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/PSRel3.psm1 new file mode 100644 index 00000000000..46edd5a921a --- /dev/null +++ b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/PSRel3.psm1 @@ -0,0 +1,11 @@ +#AnsibleRequires -CSharpUtil .sub_pkg.CSRel4 + +Function Invoke-FromPSRel3 { + <# + .SYNOPSIS + Test function + #> + return "$([CSRel4]::Invoke()) -> Invoke-FromPSRel3" +} + +Export-ModuleMember -Function Invoke-FromPSRel3 diff --git a/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/sub_pkg/CSRel4.cs b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/sub_pkg/CSRel4.cs new file mode 100644 index 00000000000..c50024b6aaa --- /dev/null +++ b/test/integration/targets/collections_relative_imports/collection_root/ansible_collections/my_ns/my_col2/plugins/module_utils/sub_pkg/CSRel4.cs @@ -0,0 +1,14 @@ +using System; + +//TypeAccelerator -Name CSRel4 -TypeName TestClass + +namespace ansible_collections.my_ns.my_col.plugins.module_utils.sub_pkg.CSRel4 +{ + public class TestClass + { + public static string Invoke() + { + return "CSRel4.Invoke()"; + } + } +} diff --git a/test/integration/targets/collections_relative_imports/runme.sh b/test/integration/targets/collections_relative_imports/runme.sh index 5800750358d..8e209e72de5 100755 --- a/test/integration/targets/collections_relative_imports/runme.sh +++ b/test/integration/targets/collections_relative_imports/runme.sh @@ -2,4 +2,12 @@ set -eux -ANSIBLE_COLLECTIONS_PATHS="${PWD}/collection_root" ansible-playbook test.yml -i ../../inventory "$@" +# we need multiple plays, and conditional import_playbook is noisy and causes problems, so choose here which one to use... +if [[ ${INVENTORY_PATH} == *.winrm ]]; then + export TEST_PLAYBOOK=windows.yml +else + export TEST_PLAYBOOK=test.yml + +fi + +ANSIBLE_COLLECTIONS_PATHS="${PWD}/collection_root" ansible-playbook "${TEST_PLAYBOOK}" -i "${INVENTORY_PATH}" "$@" diff --git a/test/integration/targets/collections_relative_imports/windows.yml b/test/integration/targets/collections_relative_imports/windows.yml new file mode 100644 index 00000000000..aa6badfa816 --- /dev/null +++ b/test/integration/targets/collections_relative_imports/windows.yml @@ -0,0 +1,11 @@ +- hosts: windows + gather_facts: no + tasks: + - name: test out relative imports on Windows modules + my_ns.my_col.win_relative: + register: win_relative + + - name: assert relative imports on Windows modules + assert: + that: + - win_relative.data == 'CSRel4.Invoke() -> Invoke-FromPSRel3 -> Invoke-FromPSRel2 -> Invoke-FromPSRel1' diff --git a/test/lib/ansible_test/_internal/csharp_import_analysis.py b/test/lib/ansible_test/_internal/csharp_import_analysis.py index 75fb2fff454..daa8892c027 100644 --- a/test/lib/ansible_test/_internal/csharp_import_analysis.py +++ b/test/lib/ansible_test/_internal/csharp_import_analysis.py @@ -13,6 +13,10 @@ from .util import ( display, ) +from .util_common import ( + resolve_csharp_ps_util, +) + from .data import ( data_context, ) @@ -82,7 +86,7 @@ def extract_csharp_module_utils_imports(path, module_utils, is_pure_csharp): if is_pure_csharp: pattern = re.compile(r'(?i)^using\s((?:Ansible|AnsibleCollections)\..+);$') else: - pattern = re.compile(r'(?i)^#\s*ansiblerequires\s+-csharputil\s+((?:Ansible|ansible.collections)\..+)') + pattern = re.compile(r'(?i)^#\s*ansiblerequires\s+-csharputil\s+((?:Ansible|ansible.collections|\.)\..+)') with open_text_file(path) as module_file: for line_number, line in enumerate(module_file, 1): @@ -91,7 +95,7 @@ def extract_csharp_module_utils_imports(path, module_utils, is_pure_csharp): if not match: continue - import_name = match.group(1) + import_name = resolve_csharp_ps_util(match.group(1), path) if import_name in module_utils: imports.add(import_name) diff --git a/test/lib/ansible_test/_internal/powershell_import_analysis.py b/test/lib/ansible_test/_internal/powershell_import_analysis.py index 4bec5aba989..cfc618593bc 100644 --- a/test/lib/ansible_test/_internal/powershell_import_analysis.py +++ b/test/lib/ansible_test/_internal/powershell_import_analysis.py @@ -13,6 +13,10 @@ from .util import ( display, ) +from .util_common import ( + resolve_csharp_ps_util, +) + from .data import ( data_context, ) @@ -85,12 +89,12 @@ def extract_powershell_module_utils_imports(path, module_utils): for line in lines: line_number += 1 - match = re.search(r'(?i)^#\s*(?:requires\s+-module(?:s?)|ansiblerequires\s+-powershell)\s*((?:Ansible|ansible_collections)\..+)', line) + match = re.search(r'(?i)^#\s*(?:requires\s+-module(?:s?)|ansiblerequires\s+-powershell)\s*((?:Ansible|ansible_collections|\.)\..+)', line) if not match: continue - import_name = match.group(1) + import_name = resolve_csharp_ps_util(match.group(1), path) if import_name in module_utils: imports.add(import_name) diff --git a/test/lib/ansible_test/_internal/util_common.py b/test/lib/ansible_test/_internal/util_common.py index 079648ecbe2..0739fd9ea81 100644 --- a/test/lib/ansible_test/_internal/util_common.py +++ b/test/lib/ansible_test/_internal/util_common.py @@ -353,6 +353,27 @@ def intercept_command(args, cmd, target_name, env, capture=False, data=None, cwd return run_command(args, cmd, capture=capture, env=env, data=data, cwd=cwd) +def resolve_csharp_ps_util(import_name, path): + """ + :type import_name: str + :type path: str + """ + if data_context().content.is_ansible or not import_name.startswith('.'): + # We don't support relative paths for builtin utils, there's no point. + return import_name + + packages = import_name.split('.') + module_packages = path.split(os.path.sep) + + for package in packages: + if not module_packages or package: + break + del module_packages[-1] + + return 'ansible_collections.%s%s' % (data_context().content.prefix, + '.'.join(module_packages + [p for p in packages if p])) + + def run_command(args, cmd, capture=False, env=None, data=None, cwd=None, always=False, stdin=None, stdout=None, cmd_verbosity=1, str_errors='strict'): """