From cace616aab91480f2139dc7b14d7d13929f34104 Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Wed, 22 Apr 2020 17:20:12 -0500 Subject: [PATCH] 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 --- .../fragments/69029-module-ignore-exts.yml | 2 ++ lib/ansible/config/base.yml | 12 +++++++++++- lib/ansible/plugins/loader.py | 7 ++++--- .../modules/uses_leaf_mu_flat_import.bak | 3 +++ .../modules/uses_leaf_mu_flat_import.yml | 3 +++ .../lib_with_extension/a.ini | 13 +++++++++++++ .../module_precedence/lib_with_extension/a.py | 13 +++++++++++++ .../lib_with_extension/ping.ini | 13 +++++++++++++ .../modules_test_envvar_ext.yml | 16 ++++++++++++++++ .../modules_test_role_ext.yml | 18 ++++++++++++++++++ .../roles_with_extension/foo/library/a.ini | 13 +++++++++++++ .../roles_with_extension/foo/library/a.py | 13 +++++++++++++ .../roles_with_extension/foo/library/ping.ini | 13 +++++++++++++ .../targets/module_precedence/runme.sh | 19 +++++++++++++++++++ test/sanity/ignore.txt | 4 ++++ 15 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/69029-module-ignore-exts.yml create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.bak create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.yml create mode 100644 test/integration/targets/module_precedence/lib_with_extension/a.ini create mode 100644 test/integration/targets/module_precedence/lib_with_extension/a.py create mode 100644 test/integration/targets/module_precedence/lib_with_extension/ping.ini create mode 100644 test/integration/targets/module_precedence/modules_test_envvar_ext.yml create mode 100644 test/integration/targets/module_precedence/modules_test_role_ext.yml create mode 100644 test/integration/targets/module_precedence/roles_with_extension/foo/library/a.ini create mode 100644 test/integration/targets/module_precedence/roles_with_extension/foo/library/a.py create mode 100644 test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.ini diff --git a/changelogs/fragments/69029-module-ignore-exts.yml b/changelogs/fragments/69029-module-ignore-exts.yml new file mode 100644 index 00000000000..a18ba0d504d --- /dev/null +++ b/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. diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index dfb5fd136db..f76088e7b7c 100644 --- a/lib/ansible/config/base.yml +++ b/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}] diff --git a/lib/ansible/plugins/loader.py b/lib/ansible/plugins/loader.py index e6d697a62cd..fb61fd2084c 100644 --- a/lib/ansible/plugins/loader.py +++ b/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) diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.bak b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.bak new file mode 100644 index 00000000000..703f454865f --- /dev/null +++ b/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. diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.yml b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_flat_import.yml new file mode 100644 index 00000000000..703f454865f --- /dev/null +++ b/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. diff --git a/test/integration/targets/module_precedence/lib_with_extension/a.ini b/test/integration/targets/module_precedence/lib_with_extension/a.ini new file mode 100644 index 00000000000..80278c9e5c6 --- /dev/null +++ b/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() diff --git a/test/integration/targets/module_precedence/lib_with_extension/a.py b/test/integration/targets/module_precedence/lib_with_extension/a.py new file mode 100644 index 00000000000..8eda1419313 --- /dev/null +++ b/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() diff --git a/test/integration/targets/module_precedence/lib_with_extension/ping.ini b/test/integration/targets/module_precedence/lib_with_extension/ping.ini new file mode 100644 index 00000000000..6f4b6a1a571 --- /dev/null +++ b/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() diff --git a/test/integration/targets/module_precedence/modules_test_envvar_ext.yml b/test/integration/targets/module_precedence/modules_test_envvar_ext.yml new file mode 100644 index 00000000000..48f27c4ff35 --- /dev/null +++ b/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"' diff --git a/test/integration/targets/module_precedence/modules_test_role_ext.yml b/test/integration/targets/module_precedence/modules_test_role_ext.yml new file mode 100644 index 00000000000..f8816f935fa --- /dev/null +++ b/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"' diff --git a/test/integration/targets/module_precedence/roles_with_extension/foo/library/a.ini b/test/integration/targets/module_precedence/roles_with_extension/foo/library/a.ini new file mode 100644 index 00000000000..8b170291f40 --- /dev/null +++ b/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() diff --git a/test/integration/targets/module_precedence/roles_with_extension/foo/library/a.py b/test/integration/targets/module_precedence/roles_with_extension/foo/library/a.py new file mode 100644 index 00000000000..4bc5906d9b5 --- /dev/null +++ b/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() diff --git a/test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.ini b/test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.ini new file mode 100644 index 00000000000..f9c04f5c690 --- /dev/null +++ b/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() diff --git a/test/integration/targets/module_precedence/runme.sh b/test/integration/targets/module_precedence/runme.sh index 45357d5cc47..0f6a98fecc4 100755 --- a/test/integration/targets/module_precedence/runme.sh +++ b/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 "$@" diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 7ab3628665c..2a637c93dc3 100644 --- a/test/sanity/ignore.txt +++ b/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