diff --git a/changelogs/fragments/interpreter_discovery.yaml b/changelogs/fragments/interpreter_discovery.yaml new file mode 100644 index 0000000000..66ff39d847 --- /dev/null +++ b/changelogs/fragments/interpreter_discovery.yaml @@ -0,0 +1,5 @@ +major_changes: +- Python interpreter discovery - The first time a Python module runs on a target, Ansible will attempt to discover the + proper default Python interpreter to use for the target platform/version (instead of immediately defaulting to + ``/usr/bin/python``). You can override this behavior by setting ``ansible_python_interpreter`` or via config. (see + https://github.com/ansible/ansible/pull/50163) diff --git a/docs/docsite/rst/index.rst b/docs/docsite/rst/index.rst index 778d2d83ca..cd3418743b 100644 --- a/docs/docsite/rst/index.rst +++ b/docs/docsite/rst/index.rst @@ -75,6 +75,7 @@ Ansible releases a new major release of Ansible approximately three to four time reference_appendices/config reference_appendices/YAMLSyntax reference_appendices/python_3_support + reference_appendices/interpreter_discovery reference_appendices/release_and_maintenance reference_appendices/test_strategies dev_guide/testing/sanity/index diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst index 344748cdec..89c467991e 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst @@ -62,11 +62,85 @@ Beginning in version 2.8, Ansible will warn if a module expects a string, but a This behavior can be changed to be an error or to be ignored by setting the ``ANSIBLE_STRING_CONVERSION_ACTION`` environment variable, or by setting the ``string_conversion_action`` configuration in the ``defaults`` section of ``ansible.cfg``. + Command line facts ------------------ ``cmdline`` facts returned in system will be deprecated in favor of ``proc_cmdline``. This change handles special case where Kernel command line parameter contains multiple values with the same key. + +Python Interpreter Discovery +============================ + +In Ansible 2.7 and earlier, Ansible defaulted to ``usr/bin/python`` as the +setting for ``ansible_python_interpreter``. If you ran Ansible against a system +that installed Python with a different name or a different path, your playbooks +would fail with ``/usr/bin/python: bad interpreter: No such file or directory`` +unless you either set ``ansible_python_interpreter`` to the correct value for +that system or added a Python interpreter and any necessary dependencies at +``usr/bin/python``. + +Starting in Ansible 2.8, Ansible searches for the correct path and executable +name for Python on each target system, first in a lookup table of default +Python interpreters for common distros, then in an ordered fallback list of +possible Python interpreter names/paths. + +It's risky to rely on a Python interpreter set from the fallback list, because +the interpreter may change on future runs. If an interpreter from +higher in the fallback list gets installed (for example, as a side-effect of +installing other packages), your original interpreter and its dependencies will +no longer be used. For this reason, Ansible warns you when it uses a Python +interpreter discovered from the fallback list. If you see this warning, the +best solution is to explicitly set ``ansible_python_interpreter`` to the path +of the correct interpreter for those target systems. + +You can still set ``ansible_python_interpreter`` to a specific path at any +variable level (as a host variable, in vars files, in playbooks, etc.). +If you prefer to use the Python interpreter discovery behavior, use +one of the four new values for ``ansible_python_interpreter`` introduced in +Ansible 2.8: + ++---------------------------+-----------------------------------------------+ +| New value | Behavior | ++===========================+===============================================+ +| | auto | | If a Python interpreter is discovered, | +| | (future default) | | Ansible uses the discovered Python, even if | +| | | | ``/usr/bin/python`` is also present. Warns | +| | | | when using the fallback list. | ++---------------------------+-----------------------------------------------+ +| | **auto_legacy** | | If a Python interpreter is discovered, and | +| | (Ansible 2.8 default) | | ``/usr/bin/python`` is absent, Ansible | +| | | | uses the discovered Python. Warns when | +| | | | using the fallback list. | +| | | | | +| | | | If a Python interpreter is discovered, and | +| | | | ``/usr/bin/python`` is present, Ansible | +| | | | uses ``/usr/bin/python`` and prints a | +| | | | deprecation warning about future default | +| | | | behavior. Warns when using the fallback | +| | | | list. | ++---------------------------+-----------------------------------------------+ +| | auto_legacy_silent | | Behaves like ``auto_legacy`` but suppresses | +| | | | the deprecation and fallback-list warnings. | ++---------------------------+-----------------------------------------------+ +| | auto_silent | | Behaves like ``auto`` but suppresses the | +| | | | fallback-list warning. | ++---------------------------+-----------------------------------------------+ + +Starting with Ansible 2.12, Ansible will use the discovered Python interpreter +by default, whether or not ``/usr/bin/python`` is also present. Until then, +the default ``auto_legacy`` setting provides compatibility with +previous versions of Ansible that always defaulted to ``/usr/bin/python``. + +If you installed Python and dependencies (``boto``, etc.) to +``/usr/bin/python`` as a workaround on distros with a different default Python +interpreter (for example, Ubuntu 16.04+, RHEL8, Fedora 23+), you have two +options: + + #. Move existing dependencies over to the default Python for each platform/distribution/version. + #. Use ``auto_legacy``. This setting lets Ansible find and use the workaround Python on hosts that have it, while also finding the correct default Python on newer hosts. But remember, the default will change in 4 releases. + + Command Line ============ diff --git a/docs/docsite/rst/reference_appendices/interpreter_discovery.rst b/docs/docsite/rst/reference_appendices/interpreter_discovery.rst new file mode 100644 index 0000000000..0a09abbb66 --- /dev/null +++ b/docs/docsite/rst/reference_appendices/interpreter_discovery.rst @@ -0,0 +1,51 @@ +.. _interpreter_discovery: + +Interpreter Discovery +===================== + +Most Ansible modules that execute under a POSIX environment require a Python +interpreter on the target host. Unless configured otherwise, Ansible will +attempt to discover a suitable Python interpreter on each target host +the first time a Python module is executed for that host. + +To control the discovery behavior: + +* for individual hosts and groups, use the ``ansible_python_interpreter`` inventory variable +* globally, use the ``interpreter_python`` key in the ``[defaults]`` section of ``ansible.cfg`` + +Use one of the following values: + +auto_legacy : (default in 2.8) + Detects the target OS platform, distribution, and version, then consults a + table listing the correct Python interpreter and path for each + platform/distribution/version. If an entry is found, and ``/usr/bin/python`` is absent, uses the discovered interpreter (and path). If an entry + is found, and ``/usr/bin/python`` is present, uses ``/usr/bin/python`` + and issues a warning. + This exception provides temporary compatibility with previous versions of + Ansible that always defaulted to ``/usr/bin/python``, so if you have + installed Python and other dependencies at ``usr/bin/python`` on some hosts, + Ansible will find and use them with this setting. + If no entry is found, or the listed Python is not present on the + target host, searches a list of common Python interpreter + paths and uses the first one found; also issues a warning that future + installation of another Python interpreter could alter the one chosen. + +auto : (future default in 2.12) + Detects the target OS platform, distribution, and version, then consults a + table listing the correct Python interpreter and path for each + platform/distribution/version. If an entry is found, uses the discovered + interpreter. + If no entry is found, or the listed Python is not present on the + target host, searches a list of common Python interpreter + paths and uses the first one found; also issues a warning that future + installation of another Python interpreter could alter the one chosen. + +auto_legacy_silent + Same as ``auto_legacy``, but does not issue warnings. + +auto_silent + Same as ``auto``, but does not issue warnings. + +You can still set ``ansible_python_interpreter`` to a specific path at any +variable level (for example, in host_vars, in vars files, in playbooks, etc.). +Setting a specific path completely disables automatic interpreter discovery; Ansible always uses the path specified. diff --git a/docs/docsite/rst/reference_appendices/python_3_support.rst b/docs/docsite/rst/reference_appendices/python_3_support.rst index 3eca64a540..35714a5f58 100644 --- a/docs/docsite/rst/reference_appendices/python_3_support.rst +++ b/docs/docsite/rst/reference_appendices/python_3_support.rst @@ -39,9 +39,12 @@ command via ``python3``. For example: Using Python 3 on the managed machines with commands and playbooks ------------------------------------------------------------------ -* Set the ``ansible_python_interpreter`` configuration option to :command:`/usr/bin/python3`. The - ``ansible_python_interpreter`` configuration option is usually set as an inventory - variable associated with a host or group of hosts: +* Ansible will automatically detect and use Python 3 on many platforms that ship with it. To explicitly configure a + Python 3 interpreter, set the ``ansible_python_interpreter`` inventory variable at a group or host level to the + location of a Python 3 interpreter, such as :command:`/usr/bin/python3`. The default interpreter path may also be + set in ``ansible.cfg``. + +.. seealso:: :ref:`interpreter_discovery` for more information. .. code-block:: ini diff --git a/docs/docsite/rst/user_guide/modules.rst b/docs/docsite/rst/user_guide/modules.rst index 063917b07d..917180888d 100644 --- a/docs/docsite/rst/user_guide/modules.rst +++ b/docs/docsite/rst/user_guide/modules.rst @@ -29,6 +29,8 @@ like services, packages, or files (anything really), or handle executing system How to write your own modules :doc:`../dev_guide/developing_api` Examples of using modules with the Python API + :doc:`../reference_appendices/interpreter_discovery` + Configuring the right Python interpreter on target hosts `Mailing List `_ Questions? Help? Ideas? Stop by the list on Google Groups `irc.freenode.net `_ diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index a31b7e20cf..0f8c779527 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -1308,6 +1308,14 @@ DISPLAY_SKIPPED_HOSTS: ini: - {key: display_skipped_hosts, section: defaults} type: boolean +DOCSITE_ROOT_URL: + name: Root docsite URL + default: https://docs.ansible.com/ansible/ + description: Root docsite URL used to generate docs URLs in warning/error text; + must be an absolute URL with valid scheme and trailing slash. + ini: + - {key: docsite_root_url, section: defaults} + version_added: "2.8" ERROR_ON_MISSING_HANDLER: name: Missing handler error default: True @@ -1382,6 +1390,55 @@ HOST_PATTERN_MISMATCH: - {key: host_pattern_mismatch, section: inventory} choices: ['warning', 'error', 'ignore'] version_added: "2.8" +INTERPRETER_PYTHON: + name: Python interpreter path (or automatic discovery behavior) used for module execution + default: auto_legacy + env: [{name: ANSIBLE_PYTHON_INTERPRETER}] + ini: + - {key: interpreter_python, section: defaults} + vars: + - {name: ansible_python_interpreter} + version_added: "2.8" + description: + - Path to the Python interpreter to be used for module execution on remote targets, or an automatic discovery mode. + Supported discovery modes are ``auto``, ``auto_silent``, and ``auto_legacy`` (the default). All discovery modes + employ a lookup table to use the included system Python (on distributions known to include one), falling back to a + fixed ordered list of well-known Python interpreter locations if a platform-specific default is not available. The + fallback behavior will issue a warning that the interpreter should be set explicitly (since interpreters installed + later may change which one is used). This warning behavior can be disabled by setting ``auto_silent``. The default + value of ``auto_legacy`` provides all the same behavior, but for backwards-compatibility with older Ansible releases + that always defaulted to ``/usr/bin/python``, will use that interpreter if present (and issue a warning that the + default behavior will change to that of ``auto`` in a future Ansible release. +INTERPRETER_PYTHON_DISTRO_MAP: + name: Mapping of known included platform pythons for various Linux distros + default: + centos: &rhelish + '6': /usr/bin/python + '8': /usr/libexec/platform-python + fedora: + '23': /usr/bin/python3 + redhat: *rhelish + rhel: *rhelish + ubuntu: + '14': /usr/bin/python + '16': /usr/bin/python3 + version_added: "2.8" + # FUTURE: add inventory override once we're sure it can't be abused by a rogue target + # FUTURE: add a platform layer to the map so we could use for, eg, freebsd/macos/etc? +INTERPRETER_PYTHON_FALLBACK: + name: Ordered list of Python interpreters to check for in discovery + default: + - /usr/bin/python + - python3.7 + - python3.6 + - python3.5 + - python2.7 + - python2.6 + - /usr/libexec/platform-python + - /usr/bin/python3 + - python + # FUTURE: add inventory override once we're sure it can't be abused by a rogue target + version_added: "2.8" INVALID_TASK_ATTRIBUTE_FAILED: name: Controls whether invalid attributes for a task result in errors instead of warnings default: True diff --git a/lib/ansible/executor/discovery/__init__.py b/lib/ansible/executor/discovery/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lib/ansible/executor/discovery/python_target.py b/lib/ansible/executor/discovery/python_target.py new file mode 100644 index 0000000000..71377332a8 --- /dev/null +++ b/lib/ansible/executor/discovery/python_target.py @@ -0,0 +1,48 @@ +# Copyright: (c) 2018 Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# FUTURE: this could be swapped out for our bundled version of distro to move more complete platform +# logic to the targets, so long as we maintain Py2.6 compat and don't need to do any kind of script assembly + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import json +import platform +import io +import os + + +def read_utf8_file(path, encoding='utf-8'): + if not os.access(path, os.R_OK): + return None + with io.open(path, 'r', encoding=encoding) as fd: + content = fd.read() + + return content + + +def get_platform_info(): + result = dict(platform_dist_result=[]) + + if hasattr(platform, 'dist'): + result['platform_dist_result'] = platform.dist() + + osrelease_content = read_utf8_file('/etc/os-release') + # try to fall back to /usr/lib/os-release + if not osrelease_content: + osrelease_content = read_utf8_file('/usr/lib/os-release') + + result['osrelease_content'] = osrelease_content + + return result + + +def main(): + info = get_platform_info() + + print(json.dumps(info)) + + +if __name__ == '__main__': + main() diff --git a/lib/ansible/executor/interpreter_discovery.py b/lib/ansible/executor/interpreter_discovery.py new file mode 100644 index 0000000000..aafb473aba --- /dev/null +++ b/lib/ansible/executor/interpreter_discovery.py @@ -0,0 +1,203 @@ +# Copyright: (c) 2018 Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import bisect +import json +import pkgutil +import re + +from ansible import constants as C +from ansible.module_utils._text import to_text +from ansible.module_utils.distro import LinuxDistribution +from ansible.utils.display import Display +from ansible.utils.plugin_docs import get_versioned_doclink +from distutils.version import LooseVersion +from traceback import format_exc + +display = Display() +foundre = re.compile(r'(?s)PLATFORM[\r\n]+(.*)FOUND(.*)ENDFOUND') + + +class InterpreterDiscoveryRequiredError(Exception): + def __init__(self, message, interpreter_name, discovery_mode): + super(InterpreterDiscoveryRequiredError, self).__init__(message) + self.interpreter_name = interpreter_name + self.discovery_mode = discovery_mode + + def __str__(self): + return self.message + + def __repr__(self): + # TODO: proper repr impl + return self.message + + +def discover_interpreter(action, interpreter_name, discovery_mode, task_vars): + # interpreter discovery is a 2-step process with the target. First, we use a simple shell-agnostic bootstrap to + # get the system type from uname, and find any random Python that can get us the info we need. For supported + # target OS types, we'll dispatch a Python script that calls plaform.dist() (for older platforms, where available) + # and brings back /etc/os-release (if present). The proper Python path is looked up in a table of known + # distros/versions with included Pythons; if nothing is found, depending on the discovery mode, either the + # default fallback of /usr/bin/python is used (if we know it's there), or discovery fails. + + # FUTURE: add logical equivalence for "python3" in the case of py3-only modules? + if interpreter_name != 'python': + raise ValueError('Interpreter discovery not supported for {0}'.format(interpreter_name)) + + host = task_vars.get('inventory_hostname', 'unknown') + res = None + platform_type = 'unknown' + found_interpreters = ['/usr/bin/python'] # fallback value + is_auto_legacy = discovery_mode.startswith('auto_legacy') + is_silent = discovery_mode.endswith('_silent') + + try: + platform_python_map = C.config.get_config_value('INTERPRETER_PYTHON_DISTRO_MAP', variables=task_vars) + bootstrap_python_list = C.config.get_config_value('INTERPRETER_PYTHON_FALLBACK', variables=task_vars) + + display.vvv(msg="Attempting {0} interpreter discovery".format(interpreter_name), host=host) + + # not all command -v impls accept a list of commands, so we have to call it once per python + command_list = ["command -v '%s'" % py for py in bootstrap_python_list] + shell_bootstrap = "echo PLATFORM; uname; echo FOUND; {0}; echo ENDFOUND".format('; '.join(command_list)) + + # FUTURE: in most cases we probably don't want to use become, but maybe sometimes we do? + res = action._low_level_execute_command(shell_bootstrap, sudoable=False) + + raw_stdout = res.get('stdout', '') + + match = foundre.match(raw_stdout) + + if not match: + display.debug('raw interpreter discovery output: {0}'.format(raw_stdout), host=host) + raise ValueError('unexpected output from Python interpreter discovery') + + platform_type = match.groups()[0].lower().strip() + + found_interpreters = [interp.strip() for interp in match.groups()[1].splitlines() if interp.startswith('/')] + + display.debug("found interpreters: {0}".format(found_interpreters), host=host) + + if not found_interpreters: + action._discovery_warnings.append('No python interpreters found for host {0} (tried {1})'.format(host, bootstrap_python_list)) + # this is lame, but returning None or throwing an exception is uglier + return '/usr/bin/python' + + if platform_type != 'linux': + raise NotImplementedError('unsupported platform for extended discovery: {0}'.format(platform_type)) + + platform_script = pkgutil.get_data('ansible.executor.discovery', 'python_target.py') + + # FUTURE: respect pipelining setting instead of just if the connection supports it? + if action._connection.has_pipelining: + res = action._low_level_execute_command(found_interpreters[0], sudoable=False, in_data=platform_script) + else: + # FUTURE: implement on-disk case (via script action or ?) + raise NotImplementedError('pipelining support required for extended interpreter discovery') + + platform_info = json.loads(res.get('stdout')) + + distro, version = _get_linux_distro(platform_info) + + if not distro or not version: + raise NotImplementedError('unable to get Linux distribution/version info') + + version_map = platform_python_map.get(distro.lower().strip()) + if not version_map: + raise NotImplementedError('unsupported Linux distribution: {0}'.format(distro)) + + platform_interpreter = _version_fuzzy_match(version, version_map) + + # provide a transition period for hosts that were using /usr/bin/python previously (but shouldn't have been) + if is_auto_legacy: + if platform_interpreter != '/usr/bin/python' and '/usr/bin/python' in found_interpreters: + # FIXME: support comments in sivel's deprecation scanner so we can get reminded on this + if not is_silent: + action._discovery_deprecation_warnings.append(dict( + msg="Distribution {0} {1} should use {2}, but is using " + "/usr/bin/python for backward compatibility with prior Ansible releases. " + "A future Ansible release will default to using the discovered platform " + "python for this host. See {3} for more information" + .format(distro, version, platform_interpreter, + get_versioned_doclink('reference_appendices/interpreter_discovery.html')), + version='2.12')) + return '/usr/bin/python' + + if platform_interpreter not in found_interpreters: + if platform_interpreter not in bootstrap_python_list: + # sanity check to make sure we looked for it + if not is_silent: + action._discovery_warnings \ + .append("Platform interpreter {0} is missing from bootstrap list" + .format(platform_interpreter)) + + if not is_silent: + action._discovery_warnings \ + .append("Distribution {0} {1} should use {2}, but is using {3}, since the " + "discovered platform python interpreter was not present. See {4} " + "for more information." + .format(distro, version, platform_interpreter, found_interpreters[0], + get_versioned_doclink('reference_appendices/interpreter_discovery.html'))) + return found_interpreters[0] + + return platform_interpreter + except NotImplementedError as ex: + display.vvv(msg='Python interpreter discovery fallback ({0})'.format(to_text(ex)), host=host) + except Exception as ex: + if not is_silent: + display.warning(msg='Unhandled error in Python interpreter discovery for host {0}: {1}'.format(host, to_text(ex))) + display.debug(msg='Interpreter discovery traceback:\n{0}'.format(to_text(format_exc())), host=host) + if res and res.get('stderr'): + display.vvv(msg='Interpreter discovery remote stderr:\n{0}'.format(to_text(res.get('stderr'))), host=host) + + if not is_silent: + action._discovery_warnings \ + .append("Platform {0} is using the discovered Python interpreter at {1}, but future installation of " + "another Python interpreter could change this. See {2} " + "for more information." + .format(platform_type, found_interpreters[0], + get_versioned_doclink('reference_appendices/interpreter_discovery.html'))) + return found_interpreters[0] + + +def _get_linux_distro(platform_info): + dist_result = platform_info.get('platform_dist_result', []) + + if len(dist_result) == 3 and any(dist_result): + return dist_result[0], dist_result[1] + + osrelease_content = platform_info.get('osrelease_content') + + if not osrelease_content: + return '', '' + + osr = LinuxDistribution._parse_os_release_content(osrelease_content) + + return osr.get('id', ''), osr.get('version_id', '') + + +def _version_fuzzy_match(version, version_map): + # try exact match first + res = version_map.get(version) + if res: + return res + + sorted_looseversions = sorted([LooseVersion(v) for v in version_map.keys()]) + + find_looseversion = LooseVersion(version) + + # slot match; return nearest previous version we're newer than + kpos = bisect.bisect(sorted_looseversions, find_looseversion) + + if kpos == 0: + # older than everything in the list, return the oldest version + # TODO: warning-worthy? + return version_map.get(sorted_looseversions[0].vstring) + + # TODO: is "past the end of the list" warning-worthy too (at least if it's not a major version match)? + + # return the next-oldest entry that we're newer than... + return version_map.get(sorted_looseversions[kpos - 1].vstring) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index b6c369efa0..d4266464fa 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -34,6 +34,7 @@ from io import BytesIO from ansible.release import __version__, __author__ from ansible import constants as C from ansible.errors import AnsibleError +from ansible.executor.interpreter_discovery import InterpreterDiscoveryRequiredError from ansible.executor.powershell import module_manifest as ps_manifest from ansible.module_utils._text import to_bytes, to_text, to_native from ansible.plugins.loader import module_utils_loader @@ -459,18 +460,46 @@ def _get_shebang(interpreter, task_vars, templar, args=tuple()): file rather than trust that we reformatted what they already have correctly. """ - interpreter_config = u'ansible_%s_interpreter' % os.path.basename(interpreter).strip() + interpreter_name = os.path.basename(interpreter).strip() - if interpreter_config not in task_vars: - return (None, interpreter) + # FUTURE: add logical equivalence for python3 in the case of py3-only modules - interpreter = templar.template(task_vars[interpreter_config].strip()) - shebang = u'#!' + interpreter + # check for first-class interpreter config + interpreter_config_key = "INTERPRETER_%s" % interpreter_name.upper() + + if C.config.get_configuration_definitions().get(interpreter_config_key): + # a config def exists for this interpreter type; consult config for the value + interpreter_out = C.config.get_config_value(interpreter_config_key, variables=task_vars) + discovered_interpreter_config = u'discovered_interpreter_%s' % interpreter_name + + interpreter_out = templar.template(interpreter_out.strip()) + + facts_from_task_vars = task_vars.get('ansible_facts', {}) + + # handle interpreter discovery if requested + if interpreter_out in ['auto', 'auto_legacy', 'auto_silent', 'auto_legacy_silent']: + if discovered_interpreter_config not in facts_from_task_vars: + # interpreter discovery is desired, but has not been run for this host + raise InterpreterDiscoveryRequiredError("interpreter discovery needed", + interpreter_name=interpreter_name, + discovery_mode=interpreter_out) + else: + interpreter_out = facts_from_task_vars[discovered_interpreter_config] + else: + # a config def does not exist for this interpreter type; consult vars for a possible direct override + interpreter_config = u'ansible_%s_interpreter' % interpreter_name + + if interpreter_config not in task_vars: + return None, interpreter + + interpreter_out = templar.template(task_vars[interpreter_config].strip()) + + shebang = u'#!' + interpreter_out if args: shebang = shebang + u' ' + u' '.join(args) - return (shebang, interpreter) + return shebang, interpreter_out def recursive_finder(name, data, py_module_names, py_module_cache, zf): diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 59e7c85e8a..831e4d9979 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -20,6 +20,7 @@ from abc import ABCMeta, abstractmethod from ansible import constants as C from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleActionSkip, AnsibleActionFail from ansible.executor.module_common import modify_module +from ansible.executor.interpreter_discovery import discover_interpreter, InterpreterDiscoveryRequiredError from ansible.module_utils.json_utils import _filter_non_json_lines from ansible.module_utils.six import binary_type, string_types, text_type, iteritems, with_metaclass from ansible.module_utils.six.moves import shlex_quote @@ -30,7 +31,6 @@ from ansible.utils.display import Display from ansible.utils.unsafe_proxy import wrap_var from ansible.vars.clean import remove_internal_keys - display = Display() @@ -58,6 +58,12 @@ class ActionBase(with_metaclass(ABCMeta, object)): self._supports_check_mode = True self._supports_async = False + # interpreter discovery state + self._discovered_interpreter_key = None + self._discovered_interpreter = False + self._discovery_deprecation_warnings = [] + self._discovery_warnings = [] + # Backwards compat: self._display isn't really needed, just import the global display and use that. self._display = display @@ -181,16 +187,36 @@ class ActionBase(with_metaclass(ABCMeta, object)): final_environment = dict() self._compute_environment_string(final_environment) - (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, self._templar, - task_vars=task_vars, - module_compression=self._play_context.module_compression, - async_timeout=self._task.async_val, - become=self._play_context.become, - become_method=self._play_context.become_method, - become_user=self._play_context.become_user, - become_password=self._play_context.become_pass, - become_flags=self._play_context.become_flags, - environment=final_environment) + # modify_module will exit early if interpreter discovery is required; re-run after if necessary + for dummy in (1, 2): + try: + (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, self._templar, + task_vars=task_vars, + module_compression=self._play_context.module_compression, + async_timeout=self._task.async_val, + become=self._play_context.become, + become_method=self._play_context.become_method, + become_user=self._play_context.become_user, + become_password=self._play_context.become_pass, + become_flags=self._play_context.become_flags, + environment=final_environment) + break + except InterpreterDiscoveryRequiredError as idre: + self._discovered_interpreter = discover_interpreter( + action=self, + interpreter_name=idre.interpreter_name, + discovery_mode=idre.discovery_mode, + task_vars=task_vars) + + # update the local task_vars with the discovered interpreter (which might be None); + # we'll propagate back to the controller in the task result + discovered_key = 'discovered_interpreter_%s' % idre.interpreter_name + # store in local task_vars facts collection for the retry and any other usages in this worker + if task_vars.get('ansible_facts') is None: + task_vars['ansible_facts'] = {} + task_vars['ansible_facts'][discovered_key] = self._discovered_interpreter + # preserve this so _execute_module can propagate back to controller as a fact + self._discovered_interpreter_key = discovered_key return (module_style, module_shebang, module_data, module_path) @@ -904,6 +930,23 @@ class ActionBase(with_metaclass(ABCMeta, object)): txt = data.get('stderr', None) or u'' data['stderr_lines'] = txt.splitlines() + # propagate interpreter discovery results back to the controller + if self._discovered_interpreter_key: + if data.get('ansible_facts') is None: + data['ansible_facts'] = {} + + data['ansible_facts'][self._discovered_interpreter_key] = self._discovered_interpreter + + if self._discovery_warnings: + if data.get('warnings') is None: + data['warnings'] = [] + data['warnings'].extend(self._discovery_warnings) + + if self._discovery_deprecation_warnings: + if data.get('deprecations') is None: + data['deprecations'] = [] + data['deprecations'].extend(self._discovery_deprecation_warnings) + display.debug("done with _execute_module (%s, %s)" % (module_name, module_args)) return data diff --git a/lib/ansible/utils/plugin_docs.py b/lib/ansible/utils/plugin_docs.py index 1735a6c271..d9cc413cdf 100644 --- a/lib/ansible/utils/plugin_docs.py +++ b/lib/ansible/utils/plugin_docs.py @@ -4,6 +4,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +from ansible import constants as C +from ansible.release import __version__ as ansible_version from ansible.errors import AnsibleError, AnsibleAssertionError from ansible.module_utils.six import string_types from ansible.module_utils._text import to_native @@ -107,3 +109,30 @@ def get_docstring(filename, fragment_loader, verbose=False, ignore_errors=False) add_fragments(data['doc'], filename, fragment_loader=fragment_loader) return data['doc'], data['plainexamples'], data['returndocs'], data['metadata'] + + +def get_versioned_doclink(path): + """ + returns a versioned documentation link for the current Ansible major.minor version; used to generate + in-product warning/error links to the configured DOCSITE_ROOT_URL + (eg, https://docs.ansible.com/ansible/2.8/somepath/doc.html) + + :param path: relative path to a document under docs/docsite/rst; + :return: absolute URL to the specified doc for the current version of Ansible + """ + path = to_native(path) + try: + base_url = C.config.get_config_value('DOCSITE_ROOT_URL') + if not base_url.endswith('/'): + base_url += '/' + if path.startswith('/'): + path = path[1:] + split_ver = ansible_version.split('.') + if len(split_ver) < 2: + raise RuntimeError('invalid version ({0})'.format(ansible_version)) + + major_minor = '{0}.{1}'.format(split_ver[0], split_ver[1]) + + return '{0}{1}/{2}'.format(base_url, major_minor, path) + except Exception as ex: + return '(unable to create versioned doc link for path {0}: {1})'.format(path, to_native(ex)) diff --git a/lib/ansible/vars/clean.py b/lib/ansible/vars/clean.py index f98d3b77d5..04d5de33ac 100644 --- a/lib/ansible/vars/clean.py +++ b/lib/ansible/vars/clean.py @@ -96,6 +96,11 @@ def remove_internal_keys(data): if key in data and not data[key]: del data[key] + # cleanse fact values that are allowed from actions but not modules + for key in list(data.get('ansible_facts', {}).keys()): + if key.startswith('discovered_interpreter_') or key.startswith('ansible_discovered_interpreter_'): + del data['ansible_facts'][key] + def clean_facts(facts): ''' remove facts that can override internal keys or otherwise deemed unsafe ''' diff --git a/test/integration/targets/interpreter_discovery_python/aliases b/test/integration/targets/interpreter_discovery_python/aliases new file mode 100644 index 0000000000..740ed1a57f --- /dev/null +++ b/test/integration/targets/interpreter_discovery_python/aliases @@ -0,0 +1,2 @@ +shippable/posix/group1 +non_local # workaround to allow override of ansible_python_interpreter; disables coverage on this integration target diff --git a/test/integration/targets/interpreter_discovery_python/library/test_echo_module.py b/test/integration/targets/interpreter_discovery_python/library/test_echo_module.py new file mode 100644 index 0000000000..73179211f7 --- /dev/null +++ b/test/integration/targets/interpreter_discovery_python/library/test_echo_module.py @@ -0,0 +1,29 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- + +# (c) 2012, Michael DeHaan +# (c) 2016, Toshio Kuratomi +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import sys +from ansible.module_utils.basic import AnsibleModule + + +def main(): + result = dict(changed=False) + + module = AnsibleModule(argument_spec=dict( + facts=dict(type=dict, default={}) + )) + + result['ansible_facts'] = module.params['facts'] + result['running_python_interpreter'] = sys.executable + + module.exit_json(**result) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/interpreter_discovery_python/tasks/main.yml b/test/integration/targets/interpreter_discovery_python/tasks/main.yml new file mode 100644 index 0000000000..50e99a26df --- /dev/null +++ b/test/integration/targets/interpreter_discovery_python/tasks/main.yml @@ -0,0 +1,145 @@ +- name: ensure we can override ansible_python_interpreter + vars: + ansible_python_interpreter: overriddenpython + assert: + that: + - ansible_python_interpreter == 'overriddenpython' + fail_msg: "'ansible_python_interpreter' appears to be set at a high precedence to {{ ansible_python_interpreter }}, + which breaks this test." + +- name: snag some facts to validate for later + set_fact: + distro: '{{ ansible_distribution | default("unknown") | lower }}' + distro_version: '{{ ansible_distribution_version | default("unknown") }}' + os_family: '{{ ansible_os_family | default("unknown") }}' + +- name: test that python discovery is working and that fact persistence makes it only run once + block: + - name: clear facts to force interpreter discovery to run + meta: clear_facts + + - name: trigger discovery with auto + vars: + ansible_python_interpreter: auto + ping: + register: auto_out + + - name: get the interpreter being used on the target to execute modules + vars: + # keep this set so we can verify we didn't repeat discovery + ansible_python_interpreter: auto + test_echo_module: + register: echoout + + - assert: + that: + - auto_out.ansible_facts.discovered_interpreter_python is defined + - echoout.running_python_interpreter == auto_out.ansible_facts.discovered_interpreter_python + # verify that discovery didn't run again (if it did, we'd have the fact in the result) + - echoout.ansible_facts is not defined or echoout.ansible_facts.discovered_interpreter_python is not defined + + +- name: test that auto_legacy gives a dep warning when /usr/bin/python present but != auto result + block: + - name: clear facts to force interpreter discovery to run + meta: clear_facts + + - name: trigger discovery with auto_legacy + vars: + ansible_python_interpreter: auto_legacy + ping: + register: legacy + + - name: check for dep warning (only on platforms where auto result is not /usr/bin/python and legacy is) + assert: + that: + - legacy.deprecations | default([]) | length > 0 + # only check for a dep warning if legacy returned /usr/bin/python and auto didn't + when: legacy.ansible_facts.discovered_interpreter_python == '/usr/bin/python' and + auto_out.ansible_facts.discovered_interpreter_python != '/usr/bin/python' + + +- name: test that auto_silent never warns and got the same answer as auto + block: + - name: clear facts to force interpreter discovery to run + meta: clear_facts + + - name: initial task to trigger discovery + vars: + ansible_python_interpreter: auto_silent + ping: + register: auto_silent_out + + - assert: + that: + - auto_silent_out.warnings is not defined + - auto_silent_out.ansible_facts.discovered_interpreter_python == auto_out.ansible_facts.discovered_interpreter_python + + +- name: test that auto_legacy_silent never warns and got the same answer as auto_legacy + block: + - name: clear facts to force interpreter discovery to run + meta: clear_facts + + - name: trigger discovery with auto_legacy_silent + vars: + ansible_python_interpreter: auto_legacy_silent + ping: + register: legacy_silent + + - assert: + that: + - legacy_silent.warnings is not defined + - legacy_silent.ansible_facts.discovered_interpreter_python == legacy.ansible_facts.discovered_interpreter_python + +- name: ensure modules can't set discovered_interpreter_X or ansible_X_interpreter + block: + - test_echo_module: + facts: + ansible_discovered_interpreter_bogus: from module + discovered_interpreter_bogus: from_module + ansible_bogus_interpreter: from_module + test_fact: from_module + register: echoout + + - assert: + that: + - test_fact == 'from_module' + - discovered_interpreter_bogus | default('nope') == 'nope' + - ansible_bogus_interpreter | default('nope') == 'nope' + # this one will exist in facts, but with its prefix removed + - ansible_facts['ansible_bogus_interpreter'] | default('nope') == 'nope' + - ansible_facts['discovered_interpreter_bogus'] | default('nope') == 'nope' + + - name: fedora assertions + assert: + that: + - auto_out.ansible_facts.discovered_interpreter_python == '/usr/bin/python3' + when: distro == 'fedora' and distro_version is version('23', '>=') + + - name: rhel assertions + assert: + that: + # rhel 6/7 + - (auto_out.ansible_facts.discovered_interpreter_python == '/usr/bin/python' and distro_version is version('8','<')) or distro_version is version('8','>=') + # rhel 8+ + - (auto_out.ansible_facts.discovered_interpreter_python == '/usr/libexec/platform-python' and distro_version is version('8','>=')) or distro_version is version('8','<') + when: distro == 'redhat' + + - name: ubuntu assertions + assert: + that: + # ubuntu < 16 + - (auto_out.ansible_facts.discovered_interpreter_python == '/usr/bin/python' and distro_version is version('16.04','<')) or distro_version is version('16.04','>=') + # ubuntu >= 16 + - (auto_out.ansible_facts.discovered_interpreter_python == '/usr/bin/python3' and distro_version is version('16.04','>=')) or distro_version is version('16.04','<') + when: distro == 'ubuntu' + + - name: mac assertions + assert: + that: + - auto_out.ansible_facts.discovered_interpreter_python == '/usr/bin/python' + when: os_family == 'darwin' + + always: + - meta: clear_facts diff --git a/test/units/executor/module_common/test_modify_module.py b/test/units/executor/module_common/test_modify_module.py index 147eda37fd..dceef76336 100644 --- a/test/units/executor/module_common/test_modify_module.py +++ b/test/units/executor/module_common/test_modify_module.py @@ -27,10 +27,11 @@ def fake_old_module_open(mocker): else: mocker.patch('builtins.open', m) - -def test_shebang(fake_old_module_open, templar): - (data, style, shebang) = modify_module('fake_module', 'fake_path', {}, templar) - assert shebang == '#!/usr/bin/python' +# this test no longer makes sense, since a Python module will always either have interpreter discovery run or +# an explicit interpreter passed (so we'll never default to the module shebang) +# def test_shebang(fake_old_module_open, templar): +# (data, style, shebang) = modify_module('fake_module', 'fake_path', {}, templar) +# assert shebang == '#!/usr/bin/python' def test_shebang_task_vars(fake_old_module_open, templar): diff --git a/test/units/executor/module_common/test_module_common.py b/test/units/executor/module_common/test_module_common.py index 69c6b30c55..10b4f0cf55 100644 --- a/test/units/executor/module_common/test_module_common.py +++ b/test/units/executor/module_common/test_module_common.py @@ -24,6 +24,7 @@ import pytest import ansible.errors from ansible.executor import module_common as amc +from ansible.executor.interpreter_discovery import InterpreterDiscoveryRequiredError from ansible.module_utils.six import PY2 @@ -105,7 +106,10 @@ def templar(): class TestGetShebang(object): """Note: We may want to change the API of this function in the future. It isn't a great API""" def test_no_interpreter_set(self, templar): - assert amc._get_shebang(u'/usr/bin/python', {}, templar) == (None, u'/usr/bin/python') + # normally this would return /usr/bin/python, but so long as we're defaulting to auto python discovery, we'll get + # an InterpreterDiscoveryRequiredError here instead + with pytest.raises(InterpreterDiscoveryRequiredError): + amc._get_shebang(u'/usr/bin/python', {}, templar) def test_non_python_interpreter(self, templar): assert amc._get_shebang(u'/usr/bin/ruby', {}, templar) == (None, u'/usr/bin/ruby') diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index b01b0e703a..17e9e457c2 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -133,7 +133,6 @@ class TestActionBase(unittest.TestCase): mock_module_loader.find_plugin.side_effect = mock_find_plugin mock_shared_obj_loader = MagicMock() mock_shared_obj_loader.module_loader = mock_module_loader - mock_templar = MagicMock() # we're using a real play context here play_context = PlayContext() @@ -144,7 +143,7 @@ class TestActionBase(unittest.TestCase): connection=mock_connection, play_context=play_context, loader=fake_loader, - templar=mock_templar, + templar=Templar(loader=fake_loader), shared_loader_obj=mock_shared_obj_loader, ) @@ -153,7 +152,8 @@ class TestActionBase(unittest.TestCase): with patch.object(os, 'rename'): mock_task.args = dict(a=1, foo='fö〩') mock_connection.module_implementation_preferences = ('',) - (style, shebang, data, path) = action_base._configure_module(mock_task.action, mock_task.args) + (style, shebang, data, path) = action_base._configure_module(mock_task.action, mock_task.args, + task_vars=dict(ansible_python_interpreter='/usr/bin/python')) self.assertEqual(style, "new") self.assertEqual(shebang, u"#!/usr/bin/python")