diff --git a/changelogs/fragments/78562-deprecate-vars-plugin-attr.yml b/changelogs/fragments/78562-deprecate-vars-plugin-attr.yml new file mode 100644 index 00000000000..b61a0fb9614 --- /dev/null +++ b/changelogs/fragments/78562-deprecate-vars-plugin-attr.yml @@ -0,0 +1,2 @@ +deprecated_features: + - vars plugins - determining whether or not to run ansible.legacy vars plugins with the class attribute REQUIRES_WHITELIST is deprecated, set REQUIRES_ENABLED instead. diff --git a/docs/docsite/rst/dev_guide/developing_collections_structure.rst b/docs/docsite/rst/dev_guide/developing_collections_structure.rst index 51f65685c5b..4ec666ce3a6 100644 --- a/docs/docsite/rst/dev_guide/developing_collections_structure.rst +++ b/docs/docsite/rst/dev_guide/developing_collections_structure.rst @@ -94,7 +94,9 @@ plugins directory Add a 'per plugin type' specific subdirectory here, including ``module_utils`` which is usable not only by modules, but by most plugins by using their FQCN. This is a way to distribute modules, lookups, filters, and so on without having to import a role in every play. -Vars plugins are supported in collections as long as they require being explicitly enabled (using ``REQUIRES_ENABLED``) and they are included using their fully qualified collection name. See :ref:`enable_vars` and :ref:`developing_vars_plugins` for details. Cache plugins may be used in collections for fact caching, but are not supported for inventory plugins. +Vars plugins in collections are not loaded automatically, and always require being explicitly enabled by using their fully qualified collection name. See :ref:`enable_vars` for details. + +Cache plugins in collections may be used for fact caching, but are not supported for inventory plugins. .. _collection_module_utils: diff --git a/docs/docsite/rst/dev_guide/developing_plugins.rst b/docs/docsite/rst/dev_guide/developing_plugins.rst index de02e8d6c47..dbf516ffa9b 100644 --- a/docs/docsite/rst/dev_guide/developing_plugins.rst +++ b/docs/docsite/rst/dev_guide/developing_plugins.rst @@ -501,7 +501,9 @@ This ``get_vars`` method just needs to return a dictionary structure with the va Since Ansible version 2.4, vars plugins only execute as needed when preparing to execute a task. This avoids the costly 'always execute' behavior that occurred during inventory construction in older versions of Ansible. Since Ansible version 2.10, vars plugin execution can be toggled by the user to run when preparing to execute a task or after importing an inventory source. -You can create vars plugins that are not enabled by default using the class variable ``REQUIRES_ENABLED``. If your vars plugin resides in a collection, it cannot be enabled by default. You must use ``REQUIRES_ENABLED`` in all collections-based vars plugins. To require users to enable your plugin, set the class variable ``REQUIRES_ENABLED``: +The user must explicitly enable vars plugins that reside in a collection. See :ref:`enable_vars` for details. + +Legacy vars plugins are always loaded and run by default. You can prevent them from automatically running by setting ``REQUIRES_ENABLED`` to True. .. code-block:: python diff --git a/lib/ansible/plugins/vars/host_group_vars.py b/lib/ansible/plugins/vars/host_group_vars.py index 7eb02949c07..521b3b6eddc 100644 --- a/lib/ansible/plugins/vars/host_group_vars.py +++ b/lib/ansible/plugins/vars/host_group_vars.py @@ -67,7 +67,7 @@ FOUND = {} # type: dict[str, list[str]] class VarsModule(BaseVarsPlugin): - REQUIRES_WHITELIST = True + REQUIRES_ENABLED = True def get_vars(self, loader, path, entities, cache=True): ''' parses the inventory file ''' diff --git a/lib/ansible/vars/plugins.py b/lib/ansible/vars/plugins.py index c4b7dd8f8ae..d3377a89c9a 100644 --- a/lib/ansible/vars/plugins.py +++ b/lib/ansible/vars/plugins.py @@ -54,9 +54,44 @@ def get_vars_from_path(loader, path, entities, stage): vars_plugin_list.append(vars_plugin) for plugin in vars_plugin_list: - if plugin._load_name not in C.VARIABLE_PLUGINS_ENABLED and getattr(plugin, 'REQUIRES_WHITELIST', False): - # 2.x plugins shipped with ansible should require enabling, older or non shipped should load automatically - continue + # legacy plugins always run by default, but they can set REQUIRES_ENABLED=True to opt out. + + # The name in config corresponds to the following _load_name: + # - legacy_plugin == legacy_plugin + # - ansible.legacy.legacy_plugin == legacy_plugin + # - builtin_plugin == builtin_plugin + # - ansible.builtin.builtin_plugin == ansible_collections.ansible.builtin.plugins.vars.builtin_plugin + builtin_or_legacy = '.' not in plugin._load_name or plugin._load_name.startswith('ansible_collections.ansible.builtin.') + + # builtin is supposed to have REQUIRES_ENABLED=True, the following is for legacy plugins... + needs_enabled = not builtin_or_legacy + if hasattr(plugin, 'REQUIRES_ENABLED'): + needs_enabled = plugin.REQUIRES_ENABLED + elif hasattr(plugin, 'REQUIRES_WHITELIST'): + display.deprecated("The VarsModule class variable 'REQUIRES_WHITELIST' is deprecated. " + "Use 'REQUIRES_ENABLED' instead.", version=2.18) + needs_enabled = plugin.REQUIRES_WHITELIST + + # A collection plugin was enabled to get to this point because vars_loader.all() does not include collection plugins. + # Warn if a collection plugin has REQUIRES_ENABLED because it has no effect. + if not builtin_or_legacy and (hasattr(plugin, 'REQUIRES_ENABLED') or hasattr(plugin, 'REQUIRES_WHITELIST')): + display.warning( + "Vars plugins in collections must be enabled to be loaded, REQUIRES_ENABLED is not supported. " + "This should be removed from the plugin %s." % plugin._load_name # FIXME: display ns.coll.resource instead of _load_name + ) + elif builtin_or_legacy and plugin._load_name not in C.VARIABLE_PLUGINS_ENABLED and needs_enabled: + # Maybe it was enabled by FQCN. + is_builtin = plugin._load_name == 'ansible_collections.ansible.builtin.plugins.vars.host_group_vars' + if is_builtin: + fqcn_builtin = 'ansible.builtin.host_group_vars' + fqcn_legacy = 'ansible.legacy.host_group_vars' + if fqcn_builtin not in C.VARIABLE_PLUGINS_ENABLED and fqcn_legacy not in C.VARIABLE_PLUGINS_ENABLED: + continue + else: + # legacy plugin + fqcn_legacy = 'ansible.legacy.%s' % plugin._load_name + if fqcn_legacy not in C.VARIABLE_PLUGINS_ENABLED: + continue has_stage = hasattr(plugin, 'get_option') and plugin.has_option('stage') diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/vars/custom_vars.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/vars/custom_vars.py index c603d72e33b..da4e776c23b 100644 --- a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/vars/custom_vars.py +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/vars/custom_vars.py @@ -39,6 +39,10 @@ from ansible.plugins.vars import BaseVarsPlugin class VarsModule(BaseVarsPlugin): + # Vars plugins in collections are only loaded when they are enabled by the user. + # If a vars plugin sets REQUIRES_ENABLED = False, a warning should occur (assuming it is loaded). + REQUIRES_ENABLED = False + def get_vars(self, loader, path, entities, cache=True): super(VarsModule, self).get_vars(loader, path, entities) return {'collection': 'collection_root_user'} diff --git a/test/integration/targets/collections/vars_plugin_tests.sh b/test/integration/targets/collections/vars_plugin_tests.sh index 9dddf321e5d..b808897412d 100755 --- a/test/integration/targets/collections/vars_plugin_tests.sh +++ b/test/integration/targets/collections/vars_plugin_tests.sh @@ -10,19 +10,21 @@ export ANSIBLE_RUN_VARS_PLUGINS=start # Test vars plugin in a playbook-adjacent collection export ANSIBLE_VARS_ENABLED=testns.content_adj.custom_adj_vars -ansible-inventory -i a.statichost.yml --list --playbook-dir=./ | tee out.txt +ansible-inventory -i a.statichost.yml --list --playbook-dir=./ 2>&1 | tee out.txt grep '"collection": "adjacent"' out.txt grep '"adj_var": "value"' out.txt +grep -v "REQUIRES_ENABLED is not supported" out.txt # Test vars plugin in a collection path export ANSIBLE_VARS_ENABLED=testns.testcoll.custom_vars export ANSIBLE_COLLECTIONS_PATH=$PWD/collection_root_user:$PWD/collection_root_sys -ansible-inventory -i a.statichost.yml --list --playbook-dir=./ | tee out.txt +ansible-inventory -i a.statichost.yml --list --playbook-dir=./ 2>&1 | tee out.txt grep '"collection": "collection_root_user"' out.txt grep -v '"adj_var": "value"' out.txt +grep "REQUIRES_ENABLED is not supported" out.txt # Test enabled vars plugins order reflects the order in which variables are merged export ANSIBLE_VARS_ENABLED=testns.content_adj.custom_adj_vars,testns.testcoll.custom_vars diff --git a/test/integration/targets/old_style_vars_plugins/aliases b/test/integration/targets/old_style_vars_plugins/aliases new file mode 100644 index 00000000000..8278ec8bcc7 --- /dev/null +++ b/test/integration/targets/old_style_vars_plugins/aliases @@ -0,0 +1,2 @@ +shippable/posix/group3 +context/controller diff --git a/test/integration/targets/old_style_vars_plugins/deprecation_warning/vars.py b/test/integration/targets/old_style_vars_plugins/deprecation_warning/vars.py new file mode 100644 index 00000000000..d5c9a422dcf --- /dev/null +++ b/test/integration/targets/old_style_vars_plugins/deprecation_warning/vars.py @@ -0,0 +1,8 @@ +from ansible.plugins.vars import BaseVarsPlugin + + +class VarsModule(BaseVarsPlugin): + REQUIRES_WHITELIST = False + + def get_vars(self, loader, path, entities): + return {} diff --git a/test/integration/targets/old_style_vars_plugins/runme.sh b/test/integration/targets/old_style_vars_plugins/runme.sh new file mode 100755 index 00000000000..4cd1916819c --- /dev/null +++ b/test/integration/targets/old_style_vars_plugins/runme.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash + +set -eux + +export ANSIBLE_VARS_PLUGINS=./vars_plugins + +# Test vars plugin without REQUIRES_ENABLED class attr and vars plugin with REQUIRES_ENABLED = False run by default +[ "$(ansible-inventory -i localhost, --list --yaml all "$@" | grep -Ec '(implicitly|explicitly)_auto_enabled')" = "2" ] + +# Test vars plugin with REQUIRES_ENABLED=True only runs when enabled +[ "$(ansible-inventory -i localhost, --list --yaml all "$@" | grep -Ec 'require_enabled')" = "0" ] +export ANSIBLE_VARS_ENABLED=require_enabled +[ "$(ansible-inventory -i localhost, --list --yaml all "$@" | grep -c 'require_enabled')" = "1" ] + +# Test the deprecated class attribute +export ANSIBLE_VARS_PLUGINS=./deprecation_warning +WARNING="The VarsModule class variable 'REQUIRES_WHITELIST' is deprecated. Use 'REQUIRES_ENABLED' instead." +ANSIBLE_DEPRECATION_WARNINGS=True ANSIBLE_NOCOLOR=True ANSIBLE_FORCE_COLOR=False \ + ansible-inventory -i localhost, --list all 2> err.txt +ansible localhost -m debug -a "msg={{ lookup('file', 'err.txt') | regex_replace('\n', '') }}" | grep "$WARNING" diff --git a/test/integration/targets/old_style_vars_plugins/vars_plugins/auto_enabled.py b/test/integration/targets/old_style_vars_plugins/vars_plugins/auto_enabled.py new file mode 100644 index 00000000000..a91d94d1ddb --- /dev/null +++ b/test/integration/targets/old_style_vars_plugins/vars_plugins/auto_enabled.py @@ -0,0 +1,8 @@ +from ansible.plugins.vars import BaseVarsPlugin + + +class VarsModule(BaseVarsPlugin): + REQUIRES_ENABLED = False + + def get_vars(self, loader, path, entities): + return {'explicitly_auto_enabled': True} diff --git a/test/integration/targets/old_style_vars_plugins/vars_plugins/implicitly_auto_enabled.py b/test/integration/targets/old_style_vars_plugins/vars_plugins/implicitly_auto_enabled.py new file mode 100644 index 00000000000..4f407b47177 --- /dev/null +++ b/test/integration/targets/old_style_vars_plugins/vars_plugins/implicitly_auto_enabled.py @@ -0,0 +1,7 @@ +from ansible.plugins.vars import BaseVarsPlugin + + +class VarsModule(BaseVarsPlugin): + + def get_vars(self, loader, path, entities): + return {'implicitly_auto_enabled': True} diff --git a/test/integration/targets/old_style_vars_plugins/vars_plugins/require_enabled.py b/test/integration/targets/old_style_vars_plugins/vars_plugins/require_enabled.py new file mode 100644 index 00000000000..a251447f0a5 --- /dev/null +++ b/test/integration/targets/old_style_vars_plugins/vars_plugins/require_enabled.py @@ -0,0 +1,8 @@ +from ansible.plugins.vars import BaseVarsPlugin + + +class VarsModule(BaseVarsPlugin): + REQUIRES_ENABLED = True + + def get_vars(self, loader, path, entities): + return {'require_enabled': True}