Fixes for validate-modules import handling. (#63932)

* Fix validate-modules support for collections.

- Relative imports now work correctly.
- The collection loader is now used.
- Modules are invoked as `__main__`.

* Remove obsolete validate-modules code ignores.

* Handle sys.exit in validate-modules.

* Add check for AnsibleModule initialization.

* Remove `missing-module-utils-import` check.

This check does not support relative imports or collections.

Instead of trying to overhaul the test, we can rely on the `ansible-module-not-initialized` test instead.

* Fix badly named error codes with `c#` in the name.

The `#` conflicts with comments in the sanity test ignore files.

* Add changelog entries.

changelogs/fragments/ansible-test-validate-modules-fixes.yml

@@ -0,0 +1,8 @@
+bugfixes:
+    - ansible-test validate-modules sanity test now properly handles collections imports using the Ansible collection loader.
+    - ansible-test validate-modules sanity test now properly handles relative imports.
+    - ansible-test validate-modules sanity test now properly invokes Ansible modules as scripts.
+    - ansible-test validate-modules sanity test now properly handles sys.exit in modules.
+    - ansible-test validate-modules sanity test now checks for AnsibleModule initialization instead of module_utils imports, which did not work in many cases.
+    - ansible-test validate-modules sanity test code ``multiple-c#-utils-per-requires`` is now ``multiple-csharp-utils-per-requires`` (fixes ignore bug).
+    - ansible-test validate-modules sanity test code ``missing-module-utils-import-c#-requirements`` is now ``missing-module-utils-import-csharp-requirements`` (fixes ignore bug).

docs/docsite/rst/dev_guide/testing_validate-modules.rst

@@ -61,6 +61,7 @@ Codes
 ============================================================   ==================   ====================   =========================================================================================
   **Error Code**                                                 **Type**             **Level**            **Sample Message**
 ------------------------------------------------------------   ------------------   --------------------   -----------------------------------------------------------------------------------------
+  ansible-module-not-initialized                               Syntax               Error                  Execution of the module did not result in initialization of AnsibleModule
   deprecation-mismatch                                         Documentation        Error                  Module marked as deprecated or removed in at least one of the filename, its metadata, or in DOCUMENTATION (setting DOCUMENTATION.deprecated for deprecation or removing all Documentation for removed) but not in all three places.
   doc-choices-do-not-match-spec                                Documentation        Error                  Value for "choices" from the argument_spec does not match the documentation
   doc-choices-incompatible-type                                Documentation        Error                  Choices value from the documentation is not compatible with type defined in the argument_spec
@@ -98,8 +99,7 @@ Codes
   missing-main-call                                            Syntax               Error                  Did not find a call to ``main()`` (or ``removed_module()`` in the case of deprecated & docs only modules)
   missing-metadata                                             Documentation        Error                  No ``ANSIBLE_METADATA`` provided
   missing-module-utils-basic-import                            Imports              Warning                Did not find ``ansible.module_utils.basic`` import
-  missing-module-utils-import                                  Imports              Error                  Did not find a ``module_utils`` import
-  missing-module-utils-import-c#                               Imports              Error                  No ``Ansible.ModuleUtils`` or C# Ansible util requirements/imports found
+  missing-module-utils-import-csharp-requirements              Imports              Error                  No ``Ansible.ModuleUtils`` or C# Ansible util requirements/imports found
   missing-powershell-interpreter                               Syntax               Error                  Interpreter line is not ``#!powershell``
   missing-python-doc                                           Naming               Error                  Missing python documentation file
   missing-python-interpreter                                   Syntax               Error                  Interpreter line is not ``#!/usr/bin/python``
@@ -110,7 +110,7 @@ Codes
   module-invalid-version-added                                 Documentation        Error                  Module level ``version_added`` is not a valid version number
   module-utils-specific-import                                 Imports              Error                  ``module_utils`` imports should import specific components, not ``*``
   multiple-utils-per-requires                                  Imports              Error                  ``Ansible.ModuleUtils`` requirements do not support multiple modules per statement
-  multiple-c#-utils-per-requires                               Imports              Error                  Ansible C# util requirements do not support multiple utils per statement
+  multiple-csharp-utils-per-requires                           Imports              Error                  Ansible C# util requirements do not support multiple utils per statement
   no-default-for-required-parameter                            Documentation        Error                  Option is marked as required but specifies a default. Arguments with a default should not be marked as required
   nonexistent-parameter-documented                             Documentation        Error                  Argument is listed in DOCUMENTATION.options, but not accepted by the module
   option-incorrect-version-added                               Documentation        Error                  ``version_added`` for new option is incorrect

test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py

@@ -38,10 +38,12 @@ from fnmatch import fnmatch
 from ansible import __version__ as ansible_version
 from ansible.executor.module_common import REPLACER_WINDOWS
 from ansible.module_utils.common._collections_compat import Mapping
+from ansible.module_utils._text import to_bytes
 from ansible.plugins.loader import fragment_loader
+from ansible.utils.collection_loader import AnsibleCollectionLoader
 from ansible.utils.plugin_docs import BLACKLIST, add_fragments, get_docstring
 
-from .module_args import AnsibleModuleImportError, get_argument_spec
+from .module_args import AnsibleModuleImportError, AnsibleModuleNotInitialized, get_argument_spec
 
 from .schema import ansible_module_kwargs_schema, doc_schema, metadata_1_1_schema, return_schema
 
@@ -517,13 +519,7 @@ class ModuleValidator(Validator):
                                 name.name == 'basic'):
                             found_basic = True
 
-        if not linenos:
-            self.reporter.error(
-                path=self.object_path,
-                code='missing-module-utils-import',
-                msg='Did not find a module_utils import'
-            )
-        elif not found_basic:
+        if not found_basic:
             self.reporter.warning(
                 path=self.object_path,
                 code='missing-module-utils-basic-import',
@@ -751,7 +747,7 @@ class ModuleValidator(Validator):
             if len(module_list) > 1:
                 self.reporter.error(
                     path=self.object_path,
-                    code='multiple-c#-utils-per-requires',
+                    code='multiple-csharp-utils-per-requires',
                     msg='Ansible C# util requirements do not support multiple utils per statement: "%s"' % req_stmt.group(0)
                 )
                 continue
@@ -769,7 +765,7 @@ class ModuleValidator(Validator):
         if not found_requires and REPLACER_WINDOWS not in self.text:
             self.reporter.error(
                 path=self.object_path,
-                code='missing-module-utils-import-c#-requirements',
+                code='missing-module-utils-import-csharp-requirements',
                 msg='No Ansible.ModuleUtils or C# Ansible util requirements/imports found'
             )
 
@@ -1133,7 +1129,14 @@ class ModuleValidator(Validator):
 
     def _validate_ansible_module_call(self, docs):
         try:
-            spec, args, kwargs = get_argument_spec(self.path)
+            spec, args, kwargs = get_argument_spec(self.path, self.collection)
+        except AnsibleModuleNotInitialized:
+            self.reporter.error(
+                path=self.object_path,
+                code='ansible-module-not-initialized',
+                msg="Execution of the module did not result in initialization of AnsibleModule",
+            )
+            return
         except AnsibleModuleImportError as e:
             self.reporter.error(
                 path=self.object_path,
@@ -1698,6 +1701,42 @@ class PythonPackageValidator(Validator):
             )
 
 
+def setup_collection_loader():
+    def get_source(self, fullname):
+        mod = sys.modules.get(fullname)
+        if not mod:
+            mod = self.load_module(fullname)
+
+        with open(to_bytes(mod.__file__), 'rb') as mod_file:
+            source = mod_file.read()
+
+        return source
+
+    def get_code(self, fullname):
+        return compile(source=self.get_source(fullname), filename=self.get_filename(fullname), mode='exec', flags=0, dont_inherit=True)
+
+    def is_package(self, fullname):
+        return self.get_filename(fullname).endswith('__init__.py')
+
+    def get_filename(self, fullname):
+        mod = sys.modules.get(fullname) or self.load_module(fullname)
+
+        return mod.__file__
+
+    # monkeypatch collection loader to work with runpy
+    # remove this (and the associated code above) once implemented natively in the collection loader
+    AnsibleCollectionLoader.get_source = get_source
+    AnsibleCollectionLoader.get_code = get_code
+    AnsibleCollectionLoader.is_package = is_package
+    AnsibleCollectionLoader.get_filename = get_filename
+
+    collection_loader = AnsibleCollectionLoader()
+
+    # allow importing code from collections when testing a collection
+    # noinspection PyCallingNonCallable
+    sys.meta_path.insert(0, collection_loader)
+
+
 def re_compile(value):
     """
     Argparse expects things to raise TypeError, re.compile raises an re.error
@@ -1745,6 +1784,9 @@ def run():
 
     check_dirs = set()
 
+    if args.collection:
+        setup_collection_loader()
+
     for module in args.modules:
         if os.path.isfile(module):
             path = module

test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/module_args.py

@@ -18,7 +18,7 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import imp
+import runpy
 import json
 import os
 import subprocess
@@ -28,7 +28,7 @@ from contextlib import contextmanager
 
 from ansible.module_utils.six import reraise
 
-from .utils import CaptureStd, find_executable
+from .utils import CaptureStd, find_executable, get_module_name_from_filename
 
 
 class AnsibleModuleCallError(RuntimeError):
@@ -39,6 +39,10 @@ class AnsibleModuleImportError(ImportError):
     pass
 
 
+class AnsibleModuleNotInitialized(Exception):
+    pass
+
+
 class _FakeAnsibleModuleInit:
     def __init__(self):
         self.args = tuple()
@@ -104,33 +108,22 @@ def get_ps_argument_spec(filename):
     return kwargs['argument_spec'], (), kwargs
 
 
-def get_py_argument_spec(filename):
-    # Calculate the module's name so that relative imports work correctly
-    name = None
-    try:
-        idx = filename.index('ansible/modules')
-    except ValueError:
-        try:
-            idx = filename.index('ansible_collections/')
-        except ValueError:
-            # We default to ``module`` here instead of ``__main__``
-            # which helps with some import issues in this tool
-            # where modules may import things that conflict
-            name = 'module'
-    if name is None:
-        name = filename[idx:-len('.py')].replace('/', '.')
+def get_py_argument_spec(filename, collection):
+    name = get_module_name_from_filename(filename, collection)
 
     with setup_env(filename) as fake:
         try:
             with CaptureStd():
-                mod = imp.load_source(name, filename)
-                if not fake.called:
-                    mod.main()
+                runpy.run_module(name, run_name='__main__')
         except AnsibleModuleCallError:
             pass
-        except Exception as e:
+        except BaseException as e:
+            # we want to catch all exceptions here, including sys.exit
             reraise(AnsibleModuleImportError, AnsibleModuleImportError('%s' % e), sys.exc_info()[2])
 
+        if not fake.called:
+            raise AnsibleModuleNotInitialized()
+
     try:
         try:
             # for ping kwargs == {'argument_spec':{'data':{'type':'str','default':'pong'}}, 'supports_check_mode':True}
@@ -141,8 +134,8 @@ def get_py_argument_spec(filename):
         return {}, (), {}
 
 
-def get_argument_spec(filename):
+def get_argument_spec(filename, collection):
     if filename.endswith('.py'):
-        return get_py_argument_spec(filename)
+        return get_py_argument_spec(filename, collection)
     else:
         return get_ps_argument_spec(filename)

test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/utils.py

@@ -115,6 +115,21 @@ class CaptureStd():
         return self.stdout.buffer.getvalue(), self.stderr.buffer.getvalue()
 
 
+def get_module_name_from_filename(filename, collection):
+    # Calculate the module's name so that relative imports work correctly
+    if collection:
+        # collection is a relative path, example: ansible_collections/my_namespace/my_collection
+        # filename is a relative path, example: plugins/modules/my_module.py
+        path = os.path.join(collection, filename)
+    else:
+        # filename is a relative path, example: lib/ansible/modules/system/ping.py
+        path = os.path.relpath(filename, 'lib')
+
+    name = os.path.splitext(path)[0].replace(os.path.sep, '.')
+
+    return name
+
+
 def parse_yaml(value, lineno, module, name, load_all=False):
     traces = []
     errors = []

test/lib/ansible_test/_internal/sanity/validate_modules.py

@@ -60,13 +60,6 @@ class ValidateModulesTest(SanitySingleVersion):
         :type python_version: str
         :rtype: TestResult
         """
-        if data_context().content.is_ansible:
-            ignore_codes = ()
-        else:
-            ignore_codes = ((
-                'E502',  # only ansible content requires __init__.py for module subdirectories
-            ))
-
         env = ansible_environment(args, color=False)
 
         settings = self.load_processor(args)
@@ -121,7 +114,6 @@ class ValidateModulesTest(SanitySingleVersion):
                     message=item['msg'],
                 ))
 
-        errors = [error for error in errors if error.code not in ignore_codes]
         errors = settings.process_errors(errors, paths)
 
         if errors: