Filter BLACKLIST_EXTS in PluginLoader (#69029)

Change:
Rather than hardcoding .pyo and .pyc, filter on all BLACKLIST_EXTS in
the non-legacy logic of PluginLoader (_find_fq_plugin). The two harcoded
extensions are part of BLACKLIST_EXTS already and this simply adds the
rest of the blacklisted extensions to the check.

In addition, check .endswith() instead of an exact match of the suffix,
like everywhere else that uses BLACKLIST_EXTS. This allows for
blacklisting, for example, emacs's backup files which can appear after
any extension, leading to things like `foo.py~`.

Test Plan:
Ran `ansible-playbook` against a collection where a `foo.py~` module was
getting executed instead of `foo.py` which also appeared in the same
directory. `foo.py~` is no longer executed.

Tickets:
Fixes #22268
Refs #27235

Signed-off-by: Rick Elrod <rick@elrod.me>

changelogs/fragments/69029-module-ignore-exts.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - plugin loader - Add MODULE_IGNORE_EXTS config option to skip over certain extensions when looking for script and binary modules.

lib/ansible/config/base.yml

@@ -1588,7 +1588,7 @@ INVENTORY_EXPORT:
   type: bool
 INVENTORY_IGNORE_EXTS:
   name: Inventory ignore extensions
-  default: "{{(BLACKLIST_EXTS + ( '.orig', '.ini', '.cfg', '.retry'))}}"
+  default: "{{(BLACKLIST_EXTS + ('.orig', '.ini', '.cfg', '.retry'))}}"
   description: List of extensions to ignore when using a directory as an inventory source
   env: [{name: ANSIBLE_INVENTORY_IGNORE}]
   ini:
@@ -1648,6 +1648,16 @@ INJECT_FACTS_AS_VARS:
   - {key: inject_facts_as_vars, section: defaults}
   type: boolean
   version_added: "2.5"
+MODULE_IGNORE_EXTS:
+  name: Module ignore extensions
+  default: "{{(BLACKLIST_EXTS + ('.yaml', '.yml', '.ini'))}}"
+  description:
+    - List of extensions to ignore when looking for modules to load
+    - This is for blacklisting script and binary module fallback extensions
+  env: [{name: ANSIBLE_MODULE_IGNORE_EXTS}]
+  ini:
+  - {key: module_ignore_exts, section: defaults}
+  type: list
 OLD_PLUGIN_CACHE_CLEARING:
   description: Previouslly Ansible would only clear some of the plugin loading caches when loading new roles, this led to some behaviours in which a plugin loaded in prevoius plays would be unexpectedly 'sticky'. This setting allows to return to that behaviour.
   env: [{name: ANSIBLE_OLD_PLUGIN_CACHE_CLEAR}]

lib/ansible/plugins/loader.py

@@ -347,8 +347,9 @@ class PluginLoader:
             return full_name, to_text(n_resource_path)
 
         # look for any matching extension in the package location (sans filter)
-        ext_blacklist = ['.pyc', '.pyo']
-        found_files = [f for f in glob.iglob(os.path.join(pkg_path, n_resource) + '.*') if os.path.isfile(f) and os.path.splitext(f)[1] not in ext_blacklist]
+        found_files = [f
+                       for f in glob.iglob(os.path.join(pkg_path, n_resource) + '.*')
+                       if os.path.isfile(f) and not f.endswith(C.MODULE_IGNORE_EXTS)]
 
         if not found_files:
             return None, None
@@ -448,7 +449,7 @@ class PluginLoader:
                 # HACK: We have no way of executing python byte compiled files as ansible modules so specifically exclude them
                 # FIXME: I believe this is only correct for modules and module_utils.
                 # For all other plugins we want .pyc and .pyo should be valid
-                if any(full_path.endswith(x) for x in C.BLACKLIST_EXTS):
+                if any(full_path.endswith(x) for x in C.MODULE_IGNORE_EXTS):
                     continue
 
                 splitname = os.path.splitext(full_name)

test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.bak

@@ -0,0 +1,3 @@
+# Intentionally blank, and intentionally attempting to shadow
+# uses_leaf_mu_flat_import.py. MODULE_IGNORE_EXTS should prevent this file
+# from ever being loaded.

test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.yml

@@ -0,0 +1,3 @@
+# Intentionally blank, and intentionally attempting to shadow
+# uses_leaf_mu_flat_import.py. MODULE_IGNORE_EXTS should prevent this file
+# from ever being loaded.

test/integration/targets/module_precedence/lib_with_extension/a.ini

@@ -0,0 +1,13 @@
+#!/usr/bin/python
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+
+def main():
+    print(json.dumps(dict(changed=False, location='a.ini')))
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/module_precedence/lib_with_extension/a.py

@@ -0,0 +1,13 @@
+#!/usr/bin/python
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+
+def main():
+    print(json.dumps(dict(changed=False, location='a.py')))
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/module_precedence/lib_with_extension/ping.ini

@@ -0,0 +1,13 @@
+#!/usr/bin/python
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+
+def main():
+    print(json.dumps(dict(changed=False, location='ping.ini')))
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/module_precedence/modules_test_envvar_ext.yml

@@ -0,0 +1,16 @@
+- hosts: testhost
+  gather_facts: no
+  tasks:
+  - name: Use ping from library path
+    ping:
+    register: result
+
+  - name: Use a from library path
+    a:
+    register: a_res
+
+  - assert:
+      that:
+        - '"location" in result'
+        - 'result["location"] == "library"'
+        - 'a_res["location"] == "a.py"'

test/integration/targets/module_precedence/modules_test_role_ext.yml

@@ -0,0 +1,18 @@
+- hosts: testhost
+  gather_facts: no
+  roles:
+    - foo
+  tasks:
+  - name: Use ping from role
+    ping:
+    register: result
+
+  - name: Use  from role
+    a:
+    register: a_res
+
+  - assert:
+      that:
+        - '"location" in result'
+        - 'result["location"] == "role: foo"'
+        - 'a_res["location"] == "role: foo, a.py"'

test/integration/targets/module_precedence/roles_with_extension/foo/library/a.ini

@@ -0,0 +1,13 @@
+#!/usr/bin/python
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+
+def main():
+    print(json.dumps(dict(changed=False, location='role: foo, a.ini')))
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/module_precedence/roles_with_extension/foo/library/a.py

@@ -0,0 +1,13 @@
+#!/usr/bin/python
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+
+def main():
+    print(json.dumps(dict(changed=False, location='role: foo, a.py')))
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.ini

@@ -0,0 +1,13 @@
+#!/usr/bin/python
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import json
+
+
+def main():
+    print(json.dumps(dict(changed=False, location='role: foo, ping.ini')))
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/module_precedence/runme.sh

@@ -28,3 +28,22 @@ ANSIBLE_LIBRARY=lib_with_extension ANSIBLE_ROLES_PATH=multiple_roles ansible-pla
 
 # And prove that with multiple roles, it's the order the roles are listed in the play that matters
 ANSIBLE_LIBRARY=lib_with_extension ANSIBLE_ROLES_PATH=multiple_roles ansible-playbook modules_test_multiple_roles_reverse_order.yml -i ../../inventory -v "$@"
+
+# Tests for MODULE_IGNORE_EXTS.
+#
+# Very similar to two tests above, but adds a check to test extension
+# precedence. Separate from the above playbooks because we *only* care about
+# extensions here and 'a' will not exist when the above playbooks run with
+# non-extension library/role paths. There is also no way to guarantee that
+# these tests will be useful due to how the pluginloader seems to work. It uses
+# os.listdir which returns things in an arbitrary order (likely dependent on
+# filesystem). If it happens to return 'a.py' on the test node before it
+# returns 'a.ini', then this test is pointless anyway because there's no chance
+# that 'a.ini' would ever have run regardless of what MODULE_IGNORE_EXTS is set
+# to. The hope is that we test across enough systems that one would fail this
+# test if the MODULE_IGNORE_EXTS broke, but there is no guarantee. This would
+# perhaps be better as a mocked unit test because of this but would require
+# a fair bit of work to be feasible as none of that loader logic is tested at
+# all right now.
+ANSIBLE_LIBRARY=lib_with_extension ansible-playbook modules_test_envvar_ext.yml -i ../../inventory -v "$@"
+ANSIBLE_ROLES_PATH=roles_with_extension ansible-playbook modules_test_role_ext.yml -i ../../inventory -v "$@"

test/sanity/ignore.txt

@@ -287,12 +287,16 @@ test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.1/xTestDsc.psd1 p
 test/integration/targets/incidental_win_ping/library/win_ping_syntax_error.ps1 pslint!skip
 test/integration/targets/incidental_win_reboot/templates/post_reboot.ps1 pslint!skip
 test/integration/targets/lookup_ini/lookup-8859-15.ini no-smart-quotes
+test/integration/targets/module_precedence/lib_with_extension/a.ini shebang
+test/integration/targets/module_precedence/lib_with_extension/ping.ini shebang
 test/integration/targets/module_precedence/lib_with_extension/ping.py future-import-boilerplate
 test/integration/targets/module_precedence/lib_with_extension/ping.py metaclass-boilerplate
 test/integration/targets/module_precedence/multiple_roles/bar/library/ping.py future-import-boilerplate
 test/integration/targets/module_precedence/multiple_roles/bar/library/ping.py metaclass-boilerplate
 test/integration/targets/module_precedence/multiple_roles/foo/library/ping.py future-import-boilerplate
 test/integration/targets/module_precedence/multiple_roles/foo/library/ping.py metaclass-boilerplate
+test/integration/targets/module_precedence/roles_with_extension/foo/library/a.ini shebang
+test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.ini shebang
 test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.py future-import-boilerplate
 test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.py metaclass-boilerplate
 test/integration/targets/module_utils/library/test.py future-import-boilerplate