Change collection PS util import pattern (#61307)

* Change collection PS util import pattern

* Add changes for py2 compat

* fix up regex and doc errors

* fix up import analysis

* Sanity fix for 2.6 CI workers

* Get collection util path for coverage collection
This commit is contained in:
Jordan Borean 2019-08-28 09:02:27 +10:00 committed by Matt Clay
parent eaa6848932
commit 66f52b74b1
18 changed files with 267 additions and 56 deletions

View file

@ -0,0 +1,2 @@
minor_changes:
- Adjusted PowerShell and C# collection util imports to use a Python package name that reflects the location of the util in the collection. This is a breaking change, for more information see :ref:`porting_2.9_guide` for more information.

View file

@ -86,10 +86,10 @@ module_utils
When coding with ``module_utils`` in a collection, the Python ``import`` statement needs to take into account the FQCN along with the ``ansible_collections`` convention. The resulting Python import will look like ``from ansible_collections.{namespace}.{collection}.plugins.module_utils.{util} import {something}``
The following example snippet shows a module using both default Ansible ``module_utils`` and
those provided by a collection. In this example the namespace is
``ansible_example``, the collection is ``community``, and the ``module_util`` in
question is called ``qradar`` such that the FQCN is ``ansible_example.community.plugins.module_utils.qradar``:
The following example snippets show a Python and PowerShell module using both default Ansible ``module_utils`` and
those provided by a collection. In this example the namespace is ``ansible_example``, the collection is ``community``.
In the Python example the ``module_util`` in question is called ``qradar`` such that the FQCN is
``ansible_example.community.plugins.module_utils.qradar``:
.. code-block:: python
@ -117,6 +117,26 @@ question is called ``qradar`` such that the FQCN is ``ansible_example.community.
)
In the PowerShell example the ``module_util`` in question is called ``hyperv`` such that the FCQN is
``ansible_example.community.plugins.module_utils.hyperv``:
.. code-block:: powershell
#!powershell
#AnsibleRequires -CSharpUtil Ansible.Basic
#AnsibleRequires -PowerShell ansible_collections.ansible_example.community.plugins.module_utils.hyperv
$spec = @{
name = @{ required = $true; type = "str" }
state = @{ required = $true; choices = @("present", "absent") }
}
$module = [Ansible.Basic.AnsibleModule]::Create($args, $spec)
Invoke-HyperVFunction -Name $module.Params.name
$module.ExitJson()
roles directory
----------------

View file

@ -34,6 +34,29 @@ Deprecated
No notable changes
Collection loader changes
=========================
The way to import a PowerShell or C# module util from a collection has changed in the Ansible 2.9 release. In Ansible
2.8 a util was imported with the following syntax:
.. code-block:: powershell
#AnsibleRequires -CSharpUtil AnsibleCollections.namespace_name.collection_name.util_filename
#AnsibleRequires -PowerShell AnsibleCollections.namespace_name.collection_name.util_filename
In Ansible 2.9 this was changed to:
.. code-block:: powershell
#AnsibleRequires -CSharpUtil ansible_collections.namespace_name.collection_name.plugins.module_utils.util_filename
#AnsibleRequires -PowerShell ansible_collections.namespace_name.collection_name.plugins.module_utils.util_filename
The change in the collection import name also requires any C# util namespaces to be updated with the newer name
format. This is more verbose but is designed to make sure we avoid plugin name conflicts across separate plugin types
and to standardise how imports work in PowerShell with how Python modules work.
Modules
=======

View file

@ -5,6 +5,7 @@ from __future__ import (absolute_import, division, print_function)
__metaclass__ = type
import base64
import errno
import json
import os
import pkgutil
@ -13,9 +14,15 @@ import re
from distutils.version import LooseVersion
# HACK: keep Python 2.6 controller tests happy in CI until they're properly split
try:
from importlib import import_module
except ImportError:
import_module = __import__
from ansible import constants as C
from ansible.errors import AnsibleError
from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils._text import to_bytes, to_native, to_text
from ansible.plugins.loader import ps_module_utils_loader
@ -35,10 +42,32 @@ class PSModuleDepFinder(object):
self.os_version = None
self.become = False
self._re_cs_module = re.compile(to_bytes(r'(?i)^using\s((Ansible\..+)|(AnsibleCollections\.\w.+\.\w.+\w.+));\s*$'))
self._re_cs_in_ps_module = re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-csharputil\s+((Ansible\..+)|(AnsibleCollections\.\w.+\.\w.+\w.+))'))
self._re_coll_ps_in_ps_module = re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-powershell\s+((Ansible\..+)|(AnsibleCollections\.\w.+\.\w.+\w.+))'))
self._re_module = re.compile(to_bytes(r'(?i)^#\s*requires\s+\-module(?:s?)\s*(Ansible\.ModuleUtils\..+)'))
self._re_cs_module = [
# Reference C# module_util in another C# util
# '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*$')),
]
self._re_cs_in_ps_module = [
# Reference C# module_util in a PowerShell module
# '#AnsibleRequires -CSharpUtil Ansible.{name}'
# '#AnsibleRequires -CSharpUtil ansible_collections.{namespace}.{collection}.plugins.module_utils.{name}'
re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-csharputil\s+((Ansible\..+)|'
r'(ansible_collections\.\w+\.\w+\.plugins\.module_utils\.[\w\.]+))')),
]
self._re_ps_module = [
# Original way of referencing a builtin module_util
# '#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\.]+))')),
]
self._re_wrapper = re.compile(to_bytes(r'(?i)^#\s*ansiblerequires\s+-wrapper\s+(\w*)'))
self._re_ps_version = re.compile(to_bytes(r'(?i)^#requires\s+\-version\s+([0-9]+(\.[0-9]+){0,3})$'))
self._re_os_version = re.compile(to_bytes(r'(?i)^#ansiblerequires\s+\-osversion\s+([0-9]+(\.[0-9]+){0,3})$'))
@ -55,28 +84,30 @@ class PSModuleDepFinder(object):
if powershell:
checks = [
# PS module contains '#Requires -Module Ansible.ModuleUtils.*'
(self._re_module, self.ps_modules, ".psm1"),
# PS module contains '#AnsibleRequires -Powershell Ansible.*' (or FQ collections module_utils ref)
(self._re_coll_ps_in_ps_module, self.ps_modules, ".psm1"),
(self._re_ps_module, self.ps_modules, ".psm1"),
# PS module contains '#AnsibleRequires -CSharpUtil Ansible.*'
(self._re_cs_in_ps_module, cs_utils, ".cs"),
]
else:
checks = [
# CS module contains 'using Ansible.*;' or 'using AnsibleCollections.ns.coll.*;'
# CS module contains 'using Ansible.*;' or 'using ansible_collections.ns.coll.plugins.module_utils.*;'
(self._re_cs_module, cs_utils, ".cs"),
]
for line in lines:
for check in checks:
match = check[0].match(line)
if match:
# tolerate windows line endings by stripping any remaining
# newline chars
module_util_name = self._normalize_mu_name(match.group(1).rstrip())
for pattern in check[0]:
match = pattern.match(line)
if match:
# tolerate windows line endings by stripping any remaining
# newline chars
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]))
if module_util_name not in check[1].keys():
module_utils.add((module_util_name, check[2]))
break
if powershell:
ps_version_match = self._re_ps_version.match(line)
@ -128,12 +159,40 @@ class PSModuleDepFinder(object):
def _add_module(self, name, wrapper=False):
m, ext = name
m = to_text(m)
mu_path = ps_module_utils_loader.find_plugin(m, ext)
if not mu_path:
raise AnsibleError('Could not find imported module support code '
'for \'%s\'' % m)
if m.startswith("Ansible."):
# Builtin util, use plugin loader to get the data
mu_path = ps_module_utils_loader.find_plugin(m, ext)
if not mu_path:
raise AnsibleError('Could not find imported module support code '
'for \'%s\'' % m)
module_util_data = to_bytes(_slurp(mu_path))
else:
# Collection util, load the package data based on the util import.
submodules = tuple(m.split("."))
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_data = to_bytes(pkgutil.get_data(n_package_name, n_resource_name),
errors='surrogate_or_strict')
# Get the path of the util which is required for coverage collection.
resource_paths = list(module_util.__path__)
if len(resource_paths) != 1:
# This should never happen with a collection but we are just being defensive about it.
raise AnsibleError("Internal error: Referenced module_util package '%s' contains 0 or multiple "
"import locations when we only expect 1." % n_package_name)
mu_path = os.path.join(resource_paths[0], n_resource_name)
except OSError as err:
if err.errno == errno.ENOENT:
raise AnsibleError('Could not find collection imported module support code for \'%s\''
% to_native(m))
else:
raise
module_util_data = to_bytes(_slurp(mu_path))
util_info = {
'data': module_util_data,
'path': to_text(mu_path),
@ -164,15 +223,6 @@ class PSModuleDepFinder(object):
if LooseVersion(new_version) > LooseVersion(existing_version):
setattr(self, attribute, new_version)
def _normalize_mu_name(self, mu):
# normalize Windows module_utils to remove 'AnsibleCollections.' prefix so the plugin loader can find them
mu = to_text(mu)
if not mu.startswith(u'AnsibleCollections.'):
return mu
return mu.replace(u'AnsibleCollections.', u'', 1)
def _slurp(path):
if not os.path.exists(path):

View file

@ -73,9 +73,13 @@ Function Add-CSharpType {
$defined_symbols.Add("WINDOWS") > $null
}
# Store any TypeAccelerators shortcuts the util wants us to set
$type_accelerators = [System.Collections.Generic.List`1[Hashtable]]@()
# pattern used to find referenced assemblies in the code
$assembly_pattern = [Regex]"//\s*AssemblyReference\s+-Name\s+(?<Name>[\w.]*)(\s+-CLR\s+(?<CLR>Core|Framework))?"
$no_warn_pattern = [Regex]"//\s*NoWarn\s+-Name\s+(?<Name>[\w\d]*)(\s+-CLR\s+(?<CLR>Core|Framework))?"
$type_pattern = [Regex]"//\s*TypeAccelerator\s+-Name\s+(?<Name>[\w.]*)\s+-TypeName\s+(?<TypeName>[\w.]*)"
# PSCore vs PSDesktop use different methods to compile the code,
# PSCore uses Roslyn and can compile the code purely in memory
@ -105,6 +109,7 @@ Function Add-CSharpType {
# scan through code and add any assemblies that match
# //AssemblyReference -Name ... [-CLR Core]
# //NoWarn -Name ... [-CLR Core]
# //TypeAccelerator -Name ... -TypeName ...
$assembly_matches = $assembly_pattern.Matches($reference)
foreach ($match in $assembly_matches) {
$clr = $match.Groups["CLR"].Value
@ -126,6 +131,11 @@ Function Add-CSharpType {
$ignore_warnings.Add($match.Groups["Name"], [Microsoft.CodeAnalysis.ReportDiagnostic]::Suppress)
}
$syntax_trees.Add([Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree]::ParseText($reference, $parse_options)) > $null
$type_matches = $type_pattern.Matches($reference)
foreach ($match in $type_matches) {
$type_accelerators.Add(@{Name=$match.Groups["Name"].Value; TypeName=$match.Groups["TypeName"].Value})
}
}
# Release seems to contain the correct line numbers compared to
@ -239,6 +249,7 @@ Function Add-CSharpType {
# scan through code and add any assemblies that match
# //AssemblyReference -Name ... [-CLR Framework]
# //NoWarn -Name ... [-CLR Framework]
# //TypeAccelerator -Name ... -TypeName ...
$assembly_matches = $assembly_pattern.Matches($reference)
foreach ($match in $assembly_matches) {
$clr = $match.Groups["CLR"].Value
@ -261,6 +272,11 @@ Function Add-CSharpType {
$ignore_warnings.Add($warning_id) > $null
}
$compile_units.Add((New-Object -TypeName System.CodeDom.CodeSnippetCompileUnit -ArgumentList $reference)) > $null
$type_matches = $type_pattern.Matches($reference)
foreach ($match in $type_matches) {
$type_accelerators.Add(@{Name=$match.Groups["Name"].Value; TypeName=$match.Groups["TypeName"].Value})
}
}
if ($ignore_warnings.Count -gt 0) {
$compiler_options.Add("/nowarn:" + ([String]::Join(",", $ignore_warnings.ToArray()))) > $null
@ -281,6 +297,23 @@ Function Add-CSharpType {
$compiled_assembly = $compile.CompiledAssembly
}
$type_accelerator = [PSObject].Assembly.GetType("System.Management.Automation.TypeAccelerators")
foreach ($accelerator in $type_accelerators) {
$type_name = $accelerator.TypeName
$found = $false
foreach ($assembly_type in $compiled_assembly.GetTypes()) {
if ($assembly_type.Name -eq $type_name) {
$type_accelerator::Add($accelerator.Name, $assembly_type)
$found = $true
break
}
}
if (-not $found) {
throw "Failed to find compiled class '$type_name' for custom TypeAccelerator."
}
}
# return the compiled assembly if PassThru is set.
if ($PassThru) {
return $compiled_assembly

View file

@ -1,6 +1,6 @@
using System;
namespace AnsibleCollections.testns.testcoll.AnotherCSMU
namespace ansible_collections.testns.testcoll.plugins.module_utils.AnotherCSMU
{
public class AnotherThing
{

View file

@ -1,15 +1,19 @@
using System;
using AnsibleCollections.testns.testcoll.AnotherCSMU;
using ansible_collections.testns.testcoll.plugins.module_utils.AnotherCSMU;
using ansible_collections.testns.testcoll.plugins.module_utils.subpkg.subcs;
namespace AnsibleCollections.testns.testcoll.MyCSMU
//TypeAccelerator -Name MyCSMU -TypeName CustomThing
namespace ansible_collections.testns.testcoll.plugins.module_utils.MyCSMU
{
public class CustomThing
{
public static string HelloWorld()
{
string res = AnotherThing.CallMe();
return String.Format("Hello from user_mu collection-hosted MyCSMU, also {0}", res);
string res1 = AnotherThing.CallMe();
string res2 = NestedUtil.HelloWorld();
return String.Format("Hello from user_mu collection-hosted MyCSMU, also {0} and {1}", res1, res2);
}
}
}

View file

@ -1,4 +1,4 @@
Function CallMe-FromUserPSMU {
Function Invoke-FromUserPSMU {
<#
.SYNOPSIS
Test function
@ -6,4 +6,4 @@ Function CallMe-FromUserPSMU {
return "from user_mu"
}
Export-ModuleMember -Function CallMe-FromUserPSMU
Export-ModuleMember -Function Invoke-FromUserPSMU

View file

@ -0,0 +1,13 @@
using System;
namespace ansible_collections.testns.testcoll.plugins.module_utils.subpkg.subcs
{
public class NestedUtil
{
public static string HelloWorld()
{
string res = "Hello from subpkg.subcs";
return res;
}
}
}

View file

@ -0,0 +1,9 @@
Function Invoke-SubUserPSMU {
<#
.SYNOPSIS
Test function
#>
return "from subpkg.subps.psm1"
}
Export-ModuleMember -Function Invoke-SubUserPSMU

View file

@ -3,11 +3,12 @@
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
#AnsibleRequires -CSharpUtil Ansible.Basic
#AnsibleRequires -CSharpUtil AnsibleCollections.testns.testcoll.MyCSMU
#AnsibleRequires -CSharpUtil ansible_collections.testns.testcoll.plugins.module_utils.MyCSMU
#AnsibleRequires -CSharpUtil ansible_collections.testns.testcoll.plugins.module_utils.subpkg.subcs
$spec = @{
options = @{
data = @{ type = "str"; default = "called from $([AnsibleCollections.testns.testcoll.MyCSMU.CustomThing]::HelloWorld())" }
data = @{ type = "str"; default = "called from $([ansible_collections.testns.testcoll.plugins.module_utils.MyCSMU.CustomThing]::HelloWorld())" }
}
supports_check_mode = $true
}
@ -20,4 +21,6 @@ if ($data -eq "crash") {
$module.Result.ping = $data
$module.Result.source = "user"
$module.Result.subpkg = [ansible_collections.testns.testcoll.plugins.module_utils.subpkg.subcs.NestedUtil]::HelloWorld()
$module.Result.type_accelerator = "called from $([MyCSMU]::HelloWorld())"
$module.ExitJson()

View file

@ -3,11 +3,12 @@
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
#AnsibleRequires -CSharpUtil Ansible.Basic
#AnsibleRequires -Powershell AnsibleCollections.testns.testcoll.MyPSMU
#AnsibleRequires -Powershell ansible_collections.testns.testcoll.plugins.module_utils.MyPSMU
#AnsibleRequires -PowerShell ansible_collections.testns.testcoll.plugins.module_utils.subpkg.subps
$spec = @{
options = @{
data = @{ type = "str"; default = "called from $(CallMe-FromUserPSMU)" }
data = @{ type = "str"; default = "called from $(Invoke-FromUserPSMU)" }
}
supports_check_mode = $true
}
@ -20,4 +21,5 @@ if ($data -eq "crash") {
$module.Result.ping = $data
$module.Result.source = "user"
$module.Result.subpkg = Invoke-SubUserPSMU
$module.ExitJson()

View file

@ -29,7 +29,7 @@ else
fi
# run test playbook
ansible-playbook -i "${INVENTORY_PATH}" -i ./a.statichost.yml -v "${TEST_PLAYBOOK}"
ansible-playbook -i "${INVENTORY_PATH}" -i ./a.statichost.yml -v "${TEST_PLAYBOOK}" "$@"
# test adjacent with --playbook-dir
export ANSIBLE_COLLECTIONS_PATHS=''

View file

@ -16,5 +16,13 @@
that:
- selfcontained_out.source == 'user'
- csbasic_only_out.source == 'user'
- uses_coll_psmu.source == 'user' and 'user_mu' in uses_coll_psmu.ping
- uses_coll_csmu.source == 'user' and 'user_mu' in uses_coll_csmu.ping
# win_uses_coll_psmu
- uses_coll_psmu.source == 'user'
- "'user_mu' in uses_coll_psmu.ping"
- uses_coll_psmu.subpkg == 'from subpkg.subps.psm1'
# win_uses_coll_csmu
- uses_coll_csmu.source == 'user'
- "'user_mu' in uses_coll_csmu.ping"
- "'Hello from subpkg.subcs' in uses_coll_csmu.ping"
- uses_coll_csmu.subpkg == 'Hello from subpkg.subcs'
- uses_coll_csmu.type_accelerator == uses_coll_csmu.ping

View file

@ -227,5 +227,50 @@ Add-CSharpType -References $defined_symbol -CompileSymbols "SYMBOL1"
$actual = [Namespace8.Class8]::GetString()
Assert-Equals -actual $actual -expected "symbol"
$type_accelerator = @'
using System;
//TypeAccelerator -Name AnsibleType -TypeName Class9
namespace Namespace9
{
public class Class9
{
public static string GetString()
{
return "a";
}
}
}
'@
Add-CSharpType -Reference $type_accelerator
$actual = [AnsibleType]::GetString()
Assert-Equals -actual $actual -expected "a"
$missing_type_class = @'
using System;
//TypeAccelerator -Name AnsibleTypeMissing -TypeName MissingClass
namespace Namespace10
{
public class Class10
{
public static string GetString()
{
return "b";
}
}
}
'@
$failed = $false
try {
Add-CSharpType -Reference $missing_type_class
} catch {
$failed = $true
Assert-Equals -actual $_.Exception.Message -expected "Failed to find compiled class 'MissingClass' for custom TypeAccelerator."
}
Assert-Equals -actual $failed -expected $true
$result.res = "success"
Exit-Json -obj $result

View file

@ -49,7 +49,7 @@ def get_csharp_module_utils_name(path): # type: (str) -> str
base_path = data_context().content.module_utils_csharp_path
if data_context().content.collection:
prefix = 'AnsibleCollections.' + data_context().content.collection.prefix
prefix = 'ansible_collections.' + data_context().content.collection.prefix + 'plugins.module_utils.'
else:
prefix = ''
@ -78,7 +78,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|AnsibleCollections)\..+)')
pattern = re.compile(r'(?i)^#\s*ansiblerequires\s+-csharputil\s+((?:Ansible|ansible.collections)\..+)')
with open(path, 'r') as module_file:
for line_number, line in enumerate(module_file, 1):

View file

@ -45,7 +45,7 @@ def get_powershell_module_utils_name(path): # type: (str) -> str
base_path = data_context().content.module_utils_powershell_path
if data_context().content.collection:
prefix = 'AnsibleCollections.' + data_context().content.collection.prefix
prefix = 'ansible_collections.' + data_context().content.collection.prefix + '.plugins.module_utils.'
else:
prefix = ''
@ -82,7 +82,7 @@ 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|AnsibleCollections)\..+)', line)
match = re.search(r'(?i)^#\s*(?:requires\s+-module(?:s?)|ansiblerequires\s+-powershell)\s*((?:Ansible|ansible_collections)\..+)', line)
if not match:
continue

View file

@ -6027,7 +6027,6 @@ test/integration/targets/async_fail/library/async_test.py future-import-boilerpl
test/integration/targets/async_fail/library/async_test.py metaclass-boilerplate
test/integration/targets/aws_lambda/files/mini_lambda.py future-import-boilerplate
test/integration/targets/aws_lambda/files/mini_lambda.py metaclass-boilerplate
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/module_utils/MyPSMU.psm1 pslint:PSUseApprovedVerbs
test/integration/targets/expect/files/test_command.py future-import-boilerplate
test/integration/targets/expect/files/test_command.py metaclass-boilerplate
test/integration/targets/get_url/files/testserver.py future-import-boilerplate