From e609d1b1fb113c523619164b75ee7c6bd1b249f0 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 12 Jul 2018 05:44:24 +0100 Subject: [PATCH 01/56] docs: glaring ancient typo. --- docs/ansible.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 7cadd67e..6c8bde25 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -45,7 +45,7 @@ it can only ensure the module executes as quickly as possible. * **Fewer writes to the target filesystem occur**. In typical configurations, Ansible repeatedly rewrites and extracts ZIP files to multiple temporary - directories on the target. Security issues relating to temporarily files in + directories on the target. Security issues relating to temporary files in cross-account scenarios are entirely avoided. The effect is most potent on playbooks that execute many **short-lived From 745d72bb1d0fa288a323d0eae5e7e2b3cc671815 Mon Sep 17 00:00:00 2001 From: napkindrawing Date: Thu, 12 Jul 2018 14:54:41 -0400 Subject: [PATCH 02/56] core: support for "doas" become_method --- ansible_mitogen/connection.py | 34 ++++- .../plugins/connection/mitogen_doas.py | 43 +++++++ mitogen/core.py | 1 + mitogen/doas.py | 118 ++++++++++++++++++ mitogen/parent.py | 3 + 5 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 ansible_mitogen/plugins/connection/mitogen_doas.py create mode 100644 mitogen/doas.py diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index c8b1870c..2b5c4356 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -173,6 +173,20 @@ def _connect_sudo(spec): } +def _connect_doas(spec): + return { + 'method': 'doas', + 'enable_lru': True, + 'kwargs': { + 'username': spec['become_user'], + 'password': wrap_or_none(mitogen.core.Secret, spec['become_pass']), + 'python_path': spec['python_path'], + 'doas_path': spec['become_exe'], + 'connect_timeout': spec['timeout'], + } + } + + def _connect_mitogen_su(spec): # su as a first-class proxied connection, not a become method. return { @@ -202,6 +216,20 @@ def _connect_mitogen_sudo(spec): } +def _connect_mitogen_doas(spec): + # doas as a first-class proxied connection, not a become method. + return { + 'method': 'doas', + 'kwargs': { + 'username': spec['remote_user'], + 'password': wrap_or_none(mitogen.core.Secret, spec['password']), + 'python_path': spec['python_path'], + 'doas_path': spec['become_exe'], + 'connect_timeout': spec['timeout'], + } + } + + CONNECTION_METHOD = { 'docker': _connect_docker, 'jail': _connect_jail, @@ -213,8 +241,10 @@ CONNECTION_METHOD = { 'ssh': _connect_ssh, 'su': _connect_su, 'sudo': _connect_sudo, + 'doas': _connect_doas, 'mitogen_su': _connect_mitogen_su, 'mitogen_sudo': _connect_mitogen_sudo, + 'mitogen_doas': _connect_mitogen_doas, } @@ -314,8 +344,8 @@ class Connection(ansible.plugins.connection.ConnectionBase): #: target user account. fork_context = None - #: Only sudo and su are supported for now. - become_methods = ['sudo', 'su'] + #: Only sudo, su, and doas are supported for now. + become_methods = ['sudo', 'su', 'doas'] #: Set to 'ansible_python_interpreter' by on_action_run(). python_path = None diff --git a/ansible_mitogen/plugins/connection/mitogen_doas.py b/ansible_mitogen/plugins/connection/mitogen_doas.py new file mode 100644 index 00000000..7d60b482 --- /dev/null +++ b/ansible_mitogen/plugins/connection/mitogen_doas.py @@ -0,0 +1,43 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import os.path +import sys + +try: + import ansible_mitogen.connection +except ImportError: + base_dir = os.path.dirname(__file__) + sys.path.insert(0, os.path.abspath(os.path.join(base_dir, '../../..'))) + del base_dir + +import ansible_mitogen.connection + + +class Connection(ansible_mitogen.connection.Connection): + transport = 'mitogen_doas' diff --git a/mitogen/core.py b/mitogen/core.py index 53f32bb0..6f536ec8 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -607,6 +607,7 @@ class Importer(object): self._present = {'mitogen': [ 'compat', 'debug', + 'doas', 'docker', 'fakessh', 'fork', diff --git a/mitogen/doas.py b/mitogen/doas.py new file mode 100644 index 00000000..1d9d04eb --- /dev/null +++ b/mitogen/doas.py @@ -0,0 +1,118 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import logging +import os + +import mitogen.core +import mitogen.parent +from mitogen.core import b + + +LOG = logging.getLogger(__name__) + + +class PasswordError(mitogen.core.StreamError): + pass + + +class Stream(mitogen.parent.Stream): + create_child = staticmethod(mitogen.parent.hybrid_tty_create_child) + child_is_immediate_subprocess = False + + #: Once connected, points to the corresponding TtyLogStream, allowing it to + #: be disconnected at the same time this stream is being torn down. + tty_stream = None + + username = 'root' + password = None + doas_path = 'doas' + password_prompt = b('Password:') + incorrect_prompts = ( + b('doas: authentication failed'), + ) + + def construct(self, username=None, password=None, doas_path=None, + password_prompt=None, incorrect_prompts=None, **kwargs): + super(Stream, self).construct(**kwargs) + if username is not None: + self.username = username + if password is not None: + self.password = password + if doas_path is not None: + self.doas_path = doas_path + if password_prompt is not None: + self.password_prompt = password_prompt.lower() + if incorrect_prompts is not None: + self.incorrect_prompts = map(str.lower, incorrect_prompts) + + def connect(self): + super(Stream, self).connect() + self.name = u'doas.' + mitogen.core.to_text(self.username) + + def on_disconnect(self, broker): + self.tty_stream.on_disconnect(broker) + super(Stream, self).on_disconnect(broker) + + def get_boot_command(self): + bits = [self.doas_path, '-u', self.username, '--'] + bits = bits + super(Stream, self).get_boot_command() + LOG.debug('doas command line: %r', bits) + return bits + + password_incorrect_msg = 'doas password is incorrect' + password_required_msg = 'doas password is required' + + def _connect_bootstrap(self, extra_fd): + self.tty_stream = mitogen.parent.TtyLogStream(extra_fd, self) + + password_sent = False + it = mitogen.parent.iter_read( + fds=[self.receive_side.fd, extra_fd], + deadline=self.connect_deadline, + ) + + for buf in it: + LOG.debug('%r: received %r', self, buf) + if buf.endswith(self.EC0_MARKER): + self._ec0_received() + return + if any(s in buf.lower() for s in self.incorrect_prompts): + if password_sent: + raise PasswordError(self.password_incorrect_msg) + elif self.password_prompt in buf.lower(): + if self.password is None: + raise PasswordError(self.password_required_msg) + if password_sent: + raise PasswordError(self.password_incorrect_msg) + LOG.debug('sending password') + self.tty_stream.transmit_side.write( + mitogen.core.to_text(self.password + '\n').encode('utf-8') + ) + password_sent = True + raise mitogen.core.StreamError('bootstrap failed') diff --git a/mitogen/parent.py b/mitogen/parent.py index 36d107f5..26796113 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1240,6 +1240,9 @@ class Router(mitogen.core.Router): self._context_by_id[context.context_id] = context return context + def doas(self, **kwargs): + return self.connect(u'doas', **kwargs) + def docker(self, **kwargs): return self.connect(u'docker', **kwargs) From 38799e22d251f2675383676963dd59f2992bae65 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 12 Jul 2018 20:16:44 +0100 Subject: [PATCH 03/56] examples: fix mitogen-fuse on 2.x. --- examples/mitogen-fuse.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/mitogen-fuse.py b/examples/mitogen-fuse.py index b086398d..7421a0e2 100644 --- a/examples/mitogen-fuse.py +++ b/examples/mitogen-fuse.py @@ -125,6 +125,8 @@ class Operations(fuse.Operations): # fuse.LoggingMixIn, self.host = host self.root = path self.ready = threading.Event() + if not hasattr(self, 'encoding'): + self.encoding = 'utf-8' def init(self, path): self.broker = mitogen.master.Broker(install_watcher=False) From f20274ea18d418fc6d1be308be6ed853ddd617fa Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 12 Jul 2018 21:36:33 +0100 Subject: [PATCH 04/56] docs: fix lock icon. --- docs/ansible.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 6c8bde25..3dd17c3a 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -90,7 +90,7 @@ concurrent to an equivalent run using the extension. .. raw:: html From b9c88d344b47520d929b3611f59f56c1b68e6326 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 12 Jul 2018 22:40:15 +0100 Subject: [PATCH 05/56] issue #299: ansible: fix PluginLoader.get() monkey-patch This prototype is broken for network_cli connections. --- ansible_mitogen/strategy.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible_mitogen/strategy.py b/ansible_mitogen/strategy.py index 6dd8ec04..fbe23ef7 100644 --- a/ansible_mitogen/strategy.py +++ b/ansible_mitogen/strategy.py @@ -54,7 +54,7 @@ def wrap_action_loader__get(name, *args, **kwargs): return adorned_klass(*args, **kwargs) -def wrap_connection_loader__get(name, play_context, new_stdin, **kwargs): +def wrap_connection_loader__get(name, *args, **kwargs): """ While the strategy is active, rewrite connection_loader.get() calls for some transports into requests for a compatible Mitogen transport. @@ -62,7 +62,7 @@ def wrap_connection_loader__get(name, play_context, new_stdin, **kwargs): if name in ('docker', 'jail', 'local', 'lxc', 'lxd', 'machinectl', 'setns', 'ssh'): name = 'mitogen_' + name - return connection_loader__get(name, play_context, new_stdin, **kwargs) + return connection_loader__get(name, *args, **kwargs) class StrategyMixin(object): From 184104ce9204bee9c79852b2b5f046d69881a59d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 13 Jul 2018 15:05:51 +0100 Subject: [PATCH 06/56] issue #303: add doas to the docs --- docs/ansible.rst | 34 +++++++++++++++++++++++++++++----- docs/api.rst | 34 ++++++++++++++++++++++++++++------ docs/changelog.rst | 7 +++++++ 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 3dd17c3a..a8f5df02 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -127,8 +127,8 @@ Noteworthy Differences precluding its use for installing Python on a target. This will be addressed soon. -* The ``su`` and ``sudo`` become methods are available. File bugs to register - interest in more. +* The ``doas``, ``su`` and ``sudo`` become methods are available. File bugs to + register interest in more. * The `docker `_, `jail `_, @@ -137,9 +137,9 @@ Noteworthy Differences `lxd `_, and `ssh `_ built-in connection types are supported, along with Mitogen-specific - :ref:`machinectl `, :ref:`mitogen_su `, :ref:`mitogen_sudo - `, and :ref:`setns ` types. File bugs to register interest in - others. + :ref:`machinectl `, :ref:`mitogen_doas< mitogen_doas>`, + :ref:`mitogen_su `, :ref:`mitogen_sudo `, and :ref:`setns ` + types. File bugs to register interest in others. * Local commands execute in a reuseable interpreter created identically to interpreters on targets. Presently one interpreter per ``become_user`` @@ -477,6 +477,30 @@ establishment of additional reuseable interpreters as necessary to match the configuration of each task. +.. _doas: + +Doas +~~~~ + +``doas`` can be used as a connection method that supports connection delegation, or +as a become method. + +When used as a become method: + +* ``ansible_python_interpreter`` +* ``ansible_become_exe``: path to ``doas`` binary. +* ``ansible_become_user`` (default: ``root``) +* ``ansible_become_pass`` (default: assume passwordless) +* ansible.cfg: ``timeout`` + +When used as the ``mitogen_doas`` connection method: + +* The inventory hostname has no special meaning. +* ``ansible_user``: username to use. +* ``ansible_password``: password to use. +* ``ansible_python_interpreter`` + + .. _method-docker: Docker diff --git a/docs/api.rst b/docs/api.rst index 0bb42ec2..bc88a622 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -523,6 +523,31 @@ Router Class # Use the SSH connection to create a sudo connection. remote_root = router.sudo(username='root', via=remote_machine) + .. method:: dos (username=None, password=None, su_path=None, password_prompt=None, incorrect_prompts=None, \**kwargs) + + Construct a context on the local machine over a ``su`` invocation. The + ``su`` process is started in a newly allocated pseudo-terminal, and + supports typing interactive passwords. + + Accepts all parameters accepted by :py:meth:`local`, in addition to: + + :param str username: + Username to use, defaults to ``root``. + :param str password: + The account password to use if requested. + :param str su_path: + Filename or complete path to the ``su`` binary. ``PATH`` will be + searched if given as a filename. Defaults to ``su``. + :param bytes password_prompt: + A string that indicates ``doas`` is requesting a password. Defaults + to ``Password:``. + :param list incorrect_prompts: + List of bytestrings indicating the password is incorrect. Defaults + to `(b"doas: authentication failed")`. + :raises mitogen.su.PasswordError: + A password was requested but none was provided, the supplied + password was incorrect, or the target account did not exist. + .. method:: docker (container=None, image=None, docker_path=None, \**kwargs) Construct a context on the local machine within an existing or @@ -616,12 +641,9 @@ Router Class :param str su_path: Filename or complete path to the ``su`` binary. ``PATH`` will be searched if given as a filename. Defaults to ``su``. - :param str password_prompt: - The string to wait to that signals ``su`` is requesting a password. - Defaults to ``Password:``. - :param str password_prompt: - The string that signal a request for the password. Defaults to - ``Password:``. + :param bytes password_prompt: + The string that indicates ``su`` is requesting a password. Defaults + to ``Password:``. :param str incorrect_prompts: Strings that signal the password is incorrect. Defaults to `("su: sorry", "su: authentication failure")`. diff --git a/docs/changelog.rst b/docs/changelog.rst index 9e85446e..b999ba4d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,13 @@ Release Notes +v0.2.2 (2018-07-??) +------------------- + +* `#303 `_: the ``doas`` become method + is now supported. Contributed by Mike Walker. + + v0.2.1 (2018-07-10) ------------------- From 8609fa5f445364c3a2b173a9dfde927a6ee9851c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sat, 14 Jul 2018 05:50:27 +0100 Subject: [PATCH 07/56] docs: link to PyPI release, not GitHub archive URL. Now download counts are visible via PSF BigQuery. --- docs/ansible.rst | 6 +++--- docs/conf.py | 23 ++++++++++++----------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index a8f5df02..e53ecf8e 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -60,13 +60,13 @@ Installation 1. Thoroughly review the :ref:`noteworthy_differences` and :ref:`changelog`. 2. Verify Ansible 2.3-2.5 and Python 2.6, 2.7 or 3.6 are listed in ``ansible --version`` output. -3. Download and extract https://github.com/dw/mitogen/archive/stable.zip +3. Download and extract |mitogen_url| from PyPI. 4. Modify ``ansible.cfg``: - .. code-block:: dosini + .. parsed-literal:: [defaults] - strategy_plugins = /path/to/mitogen-master/ansible_mitogen/plugins/strategy + strategy_plugins = /path/to/mitogen-|mitogen_version|/ansible_mitogen/plugins/strategy strategy = mitogen_linear The ``strategy`` key is optional. If omitted, the diff --git a/docs/conf.py b/docs/conf.py index 90c0f446..57adf597 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -2,15 +2,8 @@ import os import sys sys.path.append('..') - -def grep_version(): - path = os.path.join(os.path.dirname(__file__), '../mitogen/__init__.py') - with open(path) as fp: - for line in fp: - if line.startswith('__version__'): - _, _, s = line.partition('=') - return '.'.join(map(str, eval(s))) - +import mitogen +VERSION = '%s.%s.%s' % mitogen.__version__ author = u'David Wilson' copyright = u'2018, David Wilson' @@ -31,8 +24,16 @@ language = None master_doc = 'toc' project = u'Mitogen' pygments_style = 'sphinx' -release = grep_version() +release = VERSION source_suffix = '.rst' templates_path = ['_templates'] todo_include_todos = False -version = grep_version() +version = VERSION + +rst_epilog = """ + +.. |mitogen_version| replace:: %(VERSION)s + +.. |mitogen_url| replace:: `mitogen-%(VERSION)s.tar.gz `__ + +""" % locals() From 70d732d35beb7f496c597802de930255fcaf5e5c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sat, 14 Jul 2018 18:50:12 +0100 Subject: [PATCH 08/56] docs: add "no route" to known issues. --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index b999ba4d..0d296fc6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -94,6 +94,10 @@ Mitogen for Ansible - initech_app - y2k_fix +* When running with ``-vvv``, log messages such as *mitogen: Router(Broker(0x7f5a48921590)): no route + for Message(..., 102, ...), my ID is ...* may be visible. These are due to a + minor race while initializing logging and can be ignored. + * Performance does not scale linearly with target count. This requires significant additional work, as major bottlenecks exist in the surrounding Ansible code. Performance-related bug reports for any scenario remain From a5fae0d0844975052e7e3e73246d5fa4a08c3dd0 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 15 Jul 2018 17:10:58 +0100 Subject: [PATCH 09/56] docs: add jgadling to Contributors --- docs/contributors.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/contributors.rst b/docs/contributors.rst index 2bf2173c..9eebaa95 100644 --- a/docs/contributors.rst +++ b/docs/contributors.rst @@ -106,6 +106,7 @@ sponsorship and outstanding future-thinking of its early adopters.
  • Epartment
  • Fidy Andrianaivonever let a human do an ansible job ;)
  • rkrzr
  • +
  • jgadling
  • John F Wall — Making Ansible Great with Massive Parallelism
  • KennethC
  • Lewis Bellwood — Happy to be apart of a great project.
  • From 336e90c5e30ccabe916248078920c886819abe68 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 15 Jul 2018 20:34:43 +0100 Subject: [PATCH 10/56] parent: avoid needless quoting in Argv. This just makes debug output a little more readable. --- mitogen/parent.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 26796113..38484874 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -457,10 +457,16 @@ class Argv(object): def __init__(self, argv): self.argv = argv + must_escape = frozenset('\\$"`!') + must_escape_or_space = must_escape | frozenset(' ') + def escape(self, x): + if not self.must_escape_or_space.intersection(x): + return x + s = '"' for c in x: - if c in '\\$"`': + if c in self.must_escape: s += '\\' s += c s += '"' From 8ce51ec96cdfa4035c6bc762e3869b4d597caab6 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 17 Jul 2018 17:44:59 +0100 Subject: [PATCH 11/56] issue #307: add SSH login banner to Docker containers --- tests/build_docker_images.py | 2 ++ tests/data/docker/ssh_login_banner.txt | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/data/docker/ssh_login_banner.txt diff --git a/tests/build_docker_images.py b/tests/build_docker_images.py index 32e3384b..7f856b2b 100755 --- a/tests/build_docker_images.py +++ b/tests/build_docker_images.py @@ -45,10 +45,12 @@ RUN yum clean all && \ DOCKERFILE = r""" COPY data/001-mitogen.sudo /etc/sudoers.d/001-mitogen +COPY data/docker/ssh_login_banner.txt /etc/ssh/banner.txt RUN \ chsh -s /bin/bash && \ mkdir -p /var/run/sshd && \ echo i-am-mitogen-test-docker-image > /etc/sentinel && \ + echo "Banner /etc/ssh/banner.txt" >> /etc/ssh/sshd_config && \ groupadd mitogen__sudo_nopw && \ useradd -s /bin/bash -m mitogen__has_sudo -G SUDO_GROUP && \ useradd -s /bin/bash -m mitogen__has_sudo_pubkey -G SUDO_GROUP && \ diff --git a/tests/data/docker/ssh_login_banner.txt b/tests/data/docker/ssh_login_banner.txt new file mode 100644 index 00000000..1ae4cd03 --- /dev/null +++ b/tests/data/docker/ssh_login_banner.txt @@ -0,0 +1,21 @@ +This banner tests Mitogen's ability to differentiate the word 'password' +appearing in a login banner, and 'password' appearing in a password prompt. + +This system is for the use of authorized users only. Individuals using this +computer system without authority or in excess of their authority are subject +to having all of their activities on this system monitored and recorded by +system personnel. + +In the course of monitoring this system with regard to any unauthorized or +improper use or in the course of system maintenance the system personnel may +have insights into regular business activity. + +Anyone using this system expressly consents to such monitoring and is advised +that if such monitoring reveals possible evidence of improper activity, system +personnel may provide the evidence of such monitoring to internal Compliance +and Security Officers who will - in the case of criminal offences - relay such +incidents to law enforcement officials. + +************************************************************** +NOTE: This system is connected to DOMAIN.COM, +please use your password. From 54a93f3c46e726675009d9241c38ecabcc5a9da8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 17 Jul 2018 17:52:20 +0100 Subject: [PATCH 12/56] issue #307: suppress SSH login banner when verbosity is disabled. Login banner may include the password prompt, and there is no race-free way to detect the difference between the banner and the prompt. --- mitogen/ssh.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mitogen/ssh.py b/mitogen/ssh.py index eae70a36..5263e7f3 100644 --- a/mitogen/ssh.py +++ b/mitogen/ssh.py @@ -161,6 +161,11 @@ class Stream(mitogen.parent.Stream): bits = [self.ssh_path] if self.ssh_debug_level: bits += ['-' + ('v' * min(3, self.ssh_debug_level))] + else: + # issue #307: suppress any login banner, as it may contain the + # password prompt, and there is no robust way to tell the + # difference. + bits += ['-o', 'LogLevel ERROR'] if self.username: bits += ['-l', self.username] if self.port is not None: From 830a133ad60331d0ea439f8a2b74b1195d0c9062 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 17 Jul 2018 20:41:00 +0100 Subject: [PATCH 13/56] issue #307: require partial line when matching interactive prompt. This is a best-effort attempt to avoid SSHd banner spam from breaking our password entry loop. Closes #307. --- mitogen/ssh.py | 17 ++++++++++++++--- tests/ssh_test.py | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/mitogen/ssh.py b/mitogen/ssh.py index 5263e7f3..25928b45 100644 --- a/mitogen/ssh.py +++ b/mitogen/ssh.py @@ -61,7 +61,18 @@ def filter_debug(stream, it): This contains the mess of dealing with both line-oriented input, and partial lines such as the password prompt. + + Yields `(line, partial)` tuples, where `line` is the line, `partial` is + :data:`True` if no terminating newline character was present and no more + data exists in the read buffer. Consuming code can use this to unreliably + detect the presence of an interactive prompt. """ + # The `partial` test is unreliable, but is only problematic when verbosity + # is enabled: it's possible for a combination of SSH banner, password + # prompt, verbose output, timing and OS buffering specifics to create a + # situation where an otherwise newline-terminated line appears to not be + # terminated, due to a partial read(). If something is broken when + # ssh_debug_level>0, this is the first place to look. state = 'start_of_line' buf = b('') for chunk in it: @@ -86,7 +97,7 @@ def filter_debug(stream, it): state = 'start_of_line' elif state == 'in_plain': line, nl, buf = buf.partition(b('\n')) - yield line + nl + yield line + nl, not (nl or buf) if nl: state = 'start_of_line' @@ -237,7 +248,7 @@ class Stream(mitogen.parent.Stream): deadline=self.connect_deadline ) - for buf in filter_debug(self, it): + for buf, partial in filter_debug(self, it): LOG.debug('%r: received %r', self, buf) if buf.endswith(self.EC0_MARKER): self._router.broker.start_receive(self.tty_stream) @@ -255,7 +266,7 @@ class Stream(mitogen.parent.Stream): raise PasswordError(self.password_incorrect_msg) else: raise PasswordError(self.auth_incorrect_msg) - elif PASSWORD_PROMPT in buf.lower(): + elif partial and PASSWORD_PROMPT in buf.lower(): if self.password is None: raise PasswordError(self.password_required_msg) LOG.debug('%r: sending password', self) diff --git a/tests/ssh_test.py b/tests/ssh_test.py index 35f88cc8..a514c8ea 100644 --- a/tests/ssh_test.py +++ b/tests/ssh_test.py @@ -105,5 +105,24 @@ class SshTest(testlib.DockerMixin, unittest2.TestCase): ) +class BannerTest(testlib.DockerMixin, unittest2.TestCase): + # Verify the ability to disambiguate random spam appearing in the SSHd's + # login banner from a legitimate password prompt. + stream_class = mitogen.ssh.Stream + + def test_verbose_enabled(self): + context = self.docker_ssh( + username='mitogen__has_sudo', + password='has_sudo_password', + ssh_debug_level=3, + ) + name = 'ssh.%s:%s' % ( + self.dockerized_ssh.get_host(), + self.dockerized_ssh.port, + ) + self.assertEquals(name, context.name) + + + if __name__ == '__main__': unittest2.main() From 09d077ebd7e9c058f677430a47b4e2813c0128bc Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 17 Jul 2018 21:33:50 +0100 Subject: [PATCH 14/56] docs: update release notes --- docs/changelog.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0d296fc6..e853f6e8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,9 +18,24 @@ Release Notes v0.2.2 (2018-07-??) ------------------- +Mitogen for Ansible +~~~~~~~~~~~~~~~~~~~ + +* `#299 `_: fix the ``network_cli`` + connection type when the Mitogen strategy is active. + +Core Library +~~~~~~~~~~~~ + * `#303 `_: the ``doas`` become method is now supported. Contributed by Mike Walker. +* `#307 `_: SSH login banner output + containing the word 'password' is no longer confused for a password prompt. + +* Debug logs containing command lines are printed with the minimal quoting and + escaping required. + v0.2.1 (2018-07-10) ------------------- From 15d68b3c326d0150fbdcc5b811f6fe64e4ad4a52 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 19 Jul 2018 10:58:38 -0400 Subject: [PATCH 15/56] issue #309: fix environment cleanup regression. Closes #309. --- ansible_mitogen/runner.py | 10 ++-- docs/changelog.rst | 5 ++ tests/ansible/integration/runner/all.yml | 1 + .../runner/environment_isolation.yml | 50 +++++++++++++++++++ .../modules/custom_python_modify_environ.py | 22 ++++++++ 5 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 tests/ansible/integration/runner/environment_isolation.yml create mode 100644 tests/ansible/lib/modules/custom_python_modify_environ.py diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index a8384e21..3960cd38 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -288,9 +288,13 @@ class TemporaryEnvironment(object): os.environ[key] = str(value) def revert(self): - if self.env: - os.environ.clear() - os.environ.update(self.original) + """ + Revert changes made by the module to the process environment. This must + always run, as some modules (e.g. git.py) set variables like GIT_SSH + that must be cleared out between runs. + """ + os.environ.clear() + os.environ.update(self.original) class TemporaryArgv(object): diff --git a/docs/changelog.rst b/docs/changelog.rst index e853f6e8..d0d054c6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,11 @@ Mitogen for Ansible * `#299 `_: fix the ``network_cli`` connection type when the Mitogen strategy is active. +* `#309 `_: fix a regression to process + environment cleanup, caused by the change in v0.2.1 to run local tasks with + the correct environment. + + Core Library ~~~~~~~~~~~~ diff --git a/tests/ansible/integration/runner/all.yml b/tests/ansible/integration/runner/all.yml index a5da1c87..eaeb46d5 100644 --- a/tests/ansible/integration/runner/all.yml +++ b/tests/ansible/integration/runner/all.yml @@ -11,4 +11,5 @@ - import_playbook: custom_python_new_style_module.yml - import_playbook: custom_python_want_json_module.yml - import_playbook: custom_script_interpreter.yml +- import_playbook: environment_isolation.yml - import_playbook: forking_behaviour.yml diff --git a/tests/ansible/integration/runner/environment_isolation.yml b/tests/ansible/integration/runner/environment_isolation.yml new file mode 100644 index 00000000..08f0924f --- /dev/null +++ b/tests/ansible/integration/runner/environment_isolation.yml @@ -0,0 +1,50 @@ +# issue #309: ensure process environment is restored after a module runs. + + +- name: integration/runner/environment_isolation.yml + hosts: test-targets + any_errors_fatal: true + gather_facts: true + tasks: + + # --- + # Verify custom env setting is cleared out. + # --- + + # Verify sane state first. + - custom_python_detect_environment: + register: out + - assert: + that: not out.env.evil_key is defined + + - shell: echo 'hi' + environment: + evil_key: evil + + # Verify environment was cleaned up. + - custom_python_detect_environment: + register: out + - assert: + that: not out.env.evil_key is defined + + + # --- + # Verify non-explicit module env mutations are cleared out. + # --- + + # Verify sane state first. + - custom_python_detect_environment: + register: out + - assert: + that: not out.env.evil_key is defined + + - custom_python_modify_environ: + key: evil_key + val: evil + + # Verify environment was cleaned up. + - custom_python_detect_environment: + register: out + - assert: + that: not out.env.evil_key is defined + diff --git a/tests/ansible/lib/modules/custom_python_modify_environ.py b/tests/ansible/lib/modules/custom_python_modify_environ.py new file mode 100644 index 00000000..8cdd3bde --- /dev/null +++ b/tests/ansible/lib/modules/custom_python_modify_environ.py @@ -0,0 +1,22 @@ +#!/usr/bin/python +# I am an Ansible new-style Python module. I modify the process environment and +# don't clean up after myself. + +from ansible.module_utils.basic import AnsibleModule + +import os +import pwd +import socket +import sys + + +def main(): + module = AnsibleModule(argument_spec={ + 'key': {'type': str}, + 'val': {'type': str} + }) + os.environ[module.params['key']] = module.params['val'] + module.exit_json(msg='Muahahaha!') + +if __name__ == '__main__': + main() From 5d3152cdc1c2bb64750d7b0684babe3f78102dbb Mon Sep 17 00:00:00 2001 From: Daniel Quackenbush Date: Sat, 21 Jul 2018 11:16:54 -0400 Subject: [PATCH 16/56] Updated Travis Config to use Ansible 2.6.1, Added build status in readme --- .travis.yml | 56 ++++++++++++++++++++++++++++------ .travis/ansible_tests.sh | 2 +- .travis/debops_common_tests.sh | 2 +- README.md | 6 +++- dev_requirements.txt | 2 +- 5 files changed, 55 insertions(+), 13 deletions(-) diff --git a/.travis.yml b/.travis.yml index 85bbc1e4..4aed8d0a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,10 +47,10 @@ matrix: env: MODE=debops_common VER=2.4.3.0 # 2.5.5; 2.7 -> 2.7 - python: "2.7" - env: MODE=debops_common VER=2.5.5 + env: MODE=debops_common VER=2.6.1 # 2.5.5; 3.6 -> 2.7 - python: "3.6" - env: MODE=debops_common VER=2.5.5 + env: MODE=debops_common VER=2.6.1 # ansible_mitogen tests. # 2.4.3.0; Debian; 2.7 -> 2.7 @@ -59,22 +59,60 @@ matrix: # 2.5.5; Debian; 2.7 -> 2.7 - python: "2.7" env: MODE=ansible VER=2.5.5 DISTRO=debian - # 2.5.5; CentOS; 2.7 -> 2.7 + # 2.6.0; Debian; 2.7 -> 2.7 - python: "2.7" - env: MODE=ansible VER=2.5.5 DISTRO=centos7 - # 2.5.5; CentOS; 2.7 -> 2.6 + env: MODE=ansible VER=2.6.0 DISTRO=debian + # 2.6.1; Debian; 2.7 -> 2.7 - python: "2.7" - env: MODE=ansible VER=2.5.5 DISTRO=centos6 - # 2.5.5; CentOS; 2.6 -> 2.7 + env: MODE=ansible VER=2.6.1 DISTRO=debian + + # Centos 7 Python2 + # Latest - python: "2.6" + env: MODE=ansible VER=2.6.1 DISTRO=centos7 + # Backward Compatiability + - python: "2.7" + env: MODE=ansible VER=2.5.5 DISTRO=centos7 + - python: "2.7" + env: MODE=ansible VER=2.6.0 DISTRO=centos7 + - python: "2.7" + env: MODE=ansible VER=2.6.1 DISTRO=centos7 + + # Centos 7 Python3 + - python: "3.6" env: MODE=ansible VER=2.5.5 DISTRO=centos7 - # 2.5.5; CentOS; 2.6 -> 2.6 + - python: "3.6" + env: MODE=ansible VER=2.6.0 DISTRO=centos7 + - python: "3.6" + env: MODE=ansible VER=2.6.1 DISTRO=centos7 + + + # Centos 6 Python2 + # Latest + - python: "2.6" + env: MODE=ansible VER=2.6.1 DISTRO=centos6 + # Backward Compatiability - python: "2.6" env: MODE=ansible VER=2.5.5 DISTRO=centos6 - # 2.5.5; Debian; 3.6 -> 2.7 + - python: "2.6" + env: MODE=ansible VER=2.6.0 DISTRO=centos6 + - python: "2.7" + env: MODE=ansible VER=2.6.1 DISTRO=centos6 + + # Centos 6 Python3 - python: "3.6" env: MODE=ansible VER=2.5.5 DISTRO=centos6 + - python: "3.6" + env: MODE=ansible VER=2.6.0 DISTRO=centos6 + - python: "3.6" + env: MODE=ansible VER=2.6.1 DISTRO=centos6 # Sanity check our tests against vanilla Ansible, they should pass. - python: "2.7" env: MODE=ansible VER=2.5.5 DISTRO=debian STRATEGY=linear + - python: "2.7" + env: MODE=ansible VER=2.6.0 DISTRO=debian STRATEGY=linear + - python: "2.7" + env: MODE=ansible VER=2.6.1 DISTRO=debian STRATEGY=linear + + diff --git a/.travis/ansible_tests.sh b/.travis/ansible_tests.sh index 5c236521..a61ed836 100755 --- a/.travis/ansible_tests.sh +++ b/.travis/ansible_tests.sh @@ -3,7 +3,7 @@ TRAVIS_BUILD_DIR="${TRAVIS_BUILD_DIR:-`pwd`}" TMPDIR="/tmp/ansible-tests-$$" -ANSIBLE_VERSION="${VER:-2.5.5}" +ANSIBLE_VERSION="${VER:-2.6.1}" export ANSIBLE_STRATEGY="${STRATEGY:-mitogen_linear}" DISTRO="${DISTRO:-debian}" diff --git a/.travis/debops_common_tests.sh b/.travis/debops_common_tests.sh index f1909c10..ec4c1481 100755 --- a/.travis/debops_common_tests.sh +++ b/.travis/debops_common_tests.sh @@ -4,7 +4,7 @@ TMPDIR="/tmp/debops-$$" TRAVIS_BUILD_DIR="${TRAVIS_BUILD_DIR:-`pwd`}" TARGET_COUNT="${TARGET_COUNT:-2}" -ANSIBLE_VERSION="${VER:-2.5.5}" +ANSIBLE_VERSION="${VER:-2.6.1}" DISTRO=debian # Naturally DebOps only supports Debian. export PYTHONPATH="${PYTHONPATH}:${TRAVIS_BUILD_DIR}" diff --git a/README.md b/README.md index 4fb3a588..2250f1b2 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,8 @@ # Mitogen - +<<<<<<< HEAD +[![Build Status](https://travis-ci.org/dw/mitogen.png?branch=master)](https://travis-ci.org/{ORG-or-USERNAME}/{REPO-NAME}) +======= +[![Build Status](https://travis-ci.org/{ORG-or-USERNAME}/{REPO-NAME}.png?branch=c%2B%2B11)](https://travis-ci.org/{ORG-or-USERNAME}/{REPO-NAME}) +>>>>>>> fcc10e644ee217743480940e4184a5bba1955487 Please see the documentation. diff --git a/dev_requirements.txt b/dev_requirements.txt index faa7dab9..7527504e 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,5 +1,5 @@ -r docs/docs-requirements.txt -ansible==2.5.5 +ansible==2.6.1 coverage==4.5.1 Django==1.6.11 # Last version supporting 2.6. mock==2.0.0 From 3297552f650a9ff097c569e28256872b476078d0 Mon Sep 17 00:00:00 2001 From: Daniel Quackenbush Date: Sat, 21 Jul 2018 11:30:41 -0400 Subject: [PATCH 17/56] Updated readme with build status, updated docs --- README.md | 6 +----- docs/ansible.rst | 26 +++++++++++++------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 2250f1b2..43075a2e 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,4 @@ # Mitogen -<<<<<<< HEAD -[![Build Status](https://travis-ci.org/dw/mitogen.png?branch=master)](https://travis-ci.org/{ORG-or-USERNAME}/{REPO-NAME}) -======= -[![Build Status](https://travis-ci.org/{ORG-or-USERNAME}/{REPO-NAME}.png?branch=c%2B%2B11)](https://travis-ci.org/{ORG-or-USERNAME}/{REPO-NAME}) ->>>>>>> fcc10e644ee217743480940e4184a5bba1955487 +[![Build Status](https://travis-ci.org/dw/mitogen.png?branch=master)](https://travis-ci.org/dw/mitogen}) Please see the documentation. diff --git a/docs/ansible.rst b/docs/ansible.rst index e53ecf8e..b667fd37 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -58,7 +58,7 @@ Installation ------------ 1. Thoroughly review the :ref:`noteworthy_differences` and :ref:`changelog`. -2. Verify Ansible 2.3-2.5 and Python 2.6, 2.7 or 3.6 are listed in ``ansible +2. Verify Ansible 2.3-2.6.1 and Python 2.6, 2.7 or 3.6 are listed in ``ansible --version`` output. 3. Download and extract |mitogen_url| from PyPI. 4. Modify ``ansible.cfg``: @@ -130,12 +130,12 @@ Noteworthy Differences * The ``doas``, ``su`` and ``sudo`` become methods are available. File bugs to register interest in more. -* The `docker `_, - `jail `_, - `local `_, - `lxc `_, - `lxd `_, - and `ssh `_ +* The `docker `_, + `jail `_, + `local `_, + `lxc `_, + `lxd `_, + and `ssh `_ built-in connection types are supported, along with Mitogen-specific :ref:`machinectl `, :ref:`mitogen_doas< mitogen_doas>`, :ref:`mitogen_su `, :ref:`mitogen_sudo `, and :ref:`setns ` @@ -507,7 +507,7 @@ Docker ~~~~~~ Like `docker -`_ except +`_ except connection delegation is supported. * ``ansible_host``: Name of Docker container (default: inventory hostname). @@ -534,7 +534,7 @@ FreeBSD Jail ~~~~~~~~~~~~ Like `jail -`_ except +`_ except connection delegation is supported. * ``ansible_host``: Name of jail (default: inventory hostname). @@ -545,7 +545,7 @@ Local ~~~~~ Like `local -`_ except +`_ except connection delegation is supported. * ``ansible_python_interpreter`` @@ -556,8 +556,8 @@ connection delegation is supported. LXC ~~~ -Like `lxc `_ -and `lxd `_ +Like `lxc `_ +and `lxd `_ except connection delegation is supported, and ``lxc-attach`` is always used rather than the LXC Python bindings, as is usual with ``lxc``. @@ -646,7 +646,7 @@ When used as the ``mitogen_sudo`` connection method: SSH ~~~ -Like `ssh `_ +Like `ssh `_ except connection delegation is supported. * ``ansible_ssh_timeout`` From 7a4f6e74285d3866305347b1cb640333a4b051e2 Mon Sep 17 00:00:00 2001 From: Daniel Quackenbush Date: Sat, 21 Jul 2018 13:15:30 -0400 Subject: [PATCH 18/56] commented out travis badge --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 43075a2e..979afc66 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ # Mitogen -[![Build Status](https://travis-ci.org/dw/mitogen.png?branch=master)](https://travis-ci.org/dw/mitogen}) + Please see the documentation. From f977be2868b14a4028149a78603c208377689504 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 17 Jul 2018 23:16:54 +0100 Subject: [PATCH 19/56] issue #291: permit supplying a full Python argv. --- docs/api.rst | 10 +++++++--- mitogen/parent.py | 17 +++++++++++++++-- tests/local_test.py | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index bc88a622..6efca6dd 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -483,9 +483,13 @@ Router Class determine its installation prefix. This is required to support virtualenv. - :param str python_path: - Path to the Python interpreter to use for bootstrap. Defaults to - :data:`sys.executable`. For SSH, defaults to ``python``. + :param str|list python_path: + String or list path to the Python interpreter to use for bootstrap. + Defaults to :data:`sys.executable` for local connections, and + ``python`` for remote connections. + + It is possible to pass a list to invoke Python wrapped using + another tool, such as ``["/usr/bin/env", "python"]``. :param bool debug: If :data:`True`, arrange for debug logging (:py:meth:`enable_debug`) to diff --git a/mitogen/parent.py b/mitogen/parent.py index 38484874..21332454 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -871,6 +871,19 @@ class Stream(mitogen.core.Stream): fp.close() os.write(1,'MITO001\n'.encode()) + def get_python_argv(self): + """ + Return the initial argument vector elements necessary to invoke Python, + by returning a 1-element list containing :attr:`python_path` if it is a + string, or simply returning it if it is already a list. + + This allows emulation of existing tools where the Python invocation may + be set to e.g. `['/usr/bin/env', 'python']`. + """ + if isinstance(self.python_path, list): + return self.python_path + return [self.python_path] + def get_boot_command(self): source = inspect.getsource(self._first_stage) source = textwrap.dedent('\n'.join(source.strip().split('\n')[2:])) @@ -886,8 +899,8 @@ class Stream(mitogen.core.Stream): # codecs.decode() requires a bytes object. Since we must be compatible # with 2.4 (no bytes literal), an extra .encode() either returns the # same str (2.x) or an equivalent bytes (3.x). - return [ - self.python_path, '-c', + return self.get_python_argv() + [ + '-c', 'import codecs,os,sys;_=codecs.decode;' 'exec(_(_("%s".encode(),"base64"),"zip"))' % (encoded.decode(),) ] diff --git a/tests/local_test.py b/tests/local_test.py index 8ba248da..fbf5c1c8 100644 --- a/tests/local_test.py +++ b/tests/local_test.py @@ -1,5 +1,6 @@ import os +import sys import unittest2 @@ -11,6 +12,14 @@ import testlib import plain_old_module +def get_sys_executable(): + return sys.executable + + +def get_os_environ(): + return dict(os.environ) + + class LocalTest(testlib.RouterMixin, unittest2.TestCase): stream_class = mitogen.ssh.Stream @@ -20,5 +29,35 @@ class LocalTest(testlib.RouterMixin, unittest2.TestCase): self.assertEquals('local.%d' % (pid,), context.name) +class PythonPathTest(testlib.RouterMixin, unittest2.TestCase): + stream_class = mitogen.ssh.Stream + + def test_inherited(self): + context = self.router.local() + self.assertEquals(sys.executable, context.call(get_sys_executable)) + + def test_string(self): + os.environ['PYTHON'] = sys.executable + context = self.router.local( + python_path=testlib.data_path('env_wrapper.sh'), + ) + self.assertEquals(sys.executable, context.call(get_sys_executable)) + env = context.call(get_os_environ) + self.assertEquals('1', env['EXECUTED_VIA_ENV_WRAPPER']) + + def test_list(self): + context = self.router.local( + python_path=[ + testlib.data_path('env_wrapper.sh'), + "magic_first_arg", + sys.executable + ] + ) + self.assertEquals(sys.executable, context.call(get_sys_executable)) + env = context.call(get_os_environ) + self.assertEquals('magic_first_arg', env['ENV_WRAPPER_FIRST_ARG']) + self.assertEquals('1', env['EXECUTED_VIA_ENV_WRAPPER']) + + if __name__ == '__main__': unittest2.main() From e39c602fd32510ae9cebdf5caa720b7c1f193234 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 17 Jul 2018 23:45:28 +0100 Subject: [PATCH 20/56] issue #291: support UNIX hashbang syntax for ansible_*_interpreter. Closes #291. --- ansible_mitogen/connection.py | 21 ++++- ansible_mitogen/parsing.py | 84 +++++++++++++++++++ ansible_mitogen/planner.py | 31 +------ tests/ansible/integration/runner/all.yml | 1 + .../runner/custom_bash_hashbang_argument.yml | 19 +++++ .../modules/custom_bash_old_style_module.sh | 1 + 6 files changed, 125 insertions(+), 32 deletions(-) create mode 100644 ansible_mitogen/parsing.py create mode 100644 tests/ansible/integration/runner/custom_bash_hashbang_argument.yml diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 2b5c4356..464a9d81 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -44,9 +44,10 @@ import ansible.utils.shlex import mitogen.unix import mitogen.utils -import ansible_mitogen.target +import ansible_mitogen.parsing import ansible_mitogen.process import ansible_mitogen.services +import ansible_mitogen.target LOG = logging.getLogger(__name__) @@ -248,6 +249,20 @@ CONNECTION_METHOD = { } +def parse_python_path(s): + """ + Given the string set for ansible_python_interpeter, parse it as hashbang + syntax and return an appropriate argument vector. + """ + if not s: + return None + + interpreter, arg = ansible_mitogen.parsing.parse_script_interpreter(s) + if arg: + return [interpreter, arg] + return [interpreter] + + def config_from_play_context(transport, inventory_name, connection): """ Return a dict representing all important connection configuration, allowing @@ -265,7 +280,7 @@ def config_from_play_context(transport, inventory_name, connection): 'become_pass': connection._play_context.become_pass, 'password': connection._play_context.password, 'port': connection._play_context.port, - 'python_path': connection.python_path, + 'python_path': parse_python_path(connection.python_path), 'private_key_file': connection._play_context.private_key_file, 'ssh_executable': connection._play_context.ssh_executable, 'timeout': connection._play_context.timeout, @@ -314,7 +329,7 @@ def config_from_hostvars(transport, inventory_name, connection, 'password': (hostvars.get('ansible_ssh_pass') or hostvars.get('ansible_password')), 'port': hostvars.get('ansible_port'), - 'python_path': hostvars.get('ansible_python_interpreter'), + 'python_path': parse_python_path(hostvars.get('ansible_python_interpreter')), 'private_key_file': (hostvars.get('ansible_ssh_private_key_file') or hostvars.get('ansible_private_key_file')), 'mitogen_via': hostvars.get('mitogen_via'), diff --git a/ansible_mitogen/parsing.py b/ansible_mitogen/parsing.py new file mode 100644 index 00000000..fa79282a --- /dev/null +++ b/ansible_mitogen/parsing.py @@ -0,0 +1,84 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +""" +Classes to detect each case from [0] and prepare arguments necessary for the +corresponding Runner class within the target, including preloading requisite +files/modules known missing. + +[0] "Ansible Module Architecture", developing_program_flow_modules.html +""" + +from __future__ import absolute_import +from __future__ import unicode_literals + +import mitogen.core + + +def parse_script_interpreter(source): + """ + Parse the script interpreter portion of a UNIX hashbang using the rules + Linux uses. + + :param str source: String like "/usr/bin/env python". + + :returns: + Tuple of `(interpreter, arg)`, where `intepreter` is the script + interpreter and `arg` is its sole argument if present, otherwise + :py:data:`None`. + """ + # Find terminating newline. Assume last byte of binprm_buf if absent. + nl = source.find(b'\n', 0, 128) + if nl == -1: + nl = min(128, len(source)) + + # Split once on the first run of whitespace. If no whitespace exists, + # bits just contains the interpreter filename. + bits = source[0:nl].strip().split(None, 1) + if len(bits) == 1: + return mitogen.core.to_text(bits[0]), None + return mitogen.core.to_text(bits[0]), mitogen.core.to_text(bits[1]) + + +def parse_hashbang(source): + """ + Parse a UNIX "hashbang line" using the syntax supported by Linux. + + :param str source: String like "#!/usr/bin/env python". + + :returns: + Tuple of `(interpreter, arg)`, where `intepreter` is the script + interpreter and `arg` is its sole argument if present, otherwise + :py:data:`None`. + """ + # Linux requires first 2 bytes with no whitespace, pretty sure it's the + # same everywhere. See binfmt_script.c. + if not source.startswith(b'#!'): + return None, None + + return parse_script_interpreter(source[2:]) diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index 3542756b..c5636773 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -48,6 +48,7 @@ import ansible.module_utils import mitogen.core import ansible_mitogen.loaders +import ansible_mitogen.parsing import ansible_mitogen.target @@ -56,34 +57,6 @@ NO_METHOD_MSG = 'Mitogen: no invocation method found for: ' NO_INTERPRETER_MSG = 'module (%s) is missing interpreter line' -def parse_script_interpreter(source): - """ - Extract the script interpreter and its sole argument from the module - source code. - - :returns: - Tuple of `(interpreter, arg)`, where `intepreter` is the script - interpreter and `arg` is its sole argument if present, otherwise - :py:data:`None`. - """ - # Linux requires first 2 bytes with no whitespace, pretty sure it's the - # same everywhere. See binfmt_script.c. - if not source.startswith(b'#!'): - return None, None - - # Find terminating newline. Assume last byte of binprm_buf if absent. - nl = source.find(b'\n', 0, 128) - if nl == -1: - nl = min(128, len(source)) - - # Split once on the first run of whitespace. If no whitespace exists, - # bits just contains the interpreter filename. - bits = source[2:nl].strip().split(None, 1) - if len(bits) == 1: - return mitogen.core.to_text(bits[0]), None - return mitogen.core.to_text(bits[0]), mitogen.core.to_text(bits[1]) - - class Invocation(object): """ Collect up a module's execution environment then use it to invoke @@ -215,7 +188,7 @@ class ScriptPlanner(BinaryPlanner): detection and rewrite. """ def _get_interpreter(self): - interpreter, arg = parse_script_interpreter( + interpreter, arg = ansible_mitogen.parsing.parse_hashbang( self._inv.module_source ) if interpreter is None: diff --git a/tests/ansible/integration/runner/all.yml b/tests/ansible/integration/runner/all.yml index eaeb46d5..5242a405 100644 --- a/tests/ansible/integration/runner/all.yml +++ b/tests/ansible/integration/runner/all.yml @@ -1,6 +1,7 @@ - import_playbook: builtin_command_module.yml - import_playbook: custom_bash_old_style_module.yml - import_playbook: custom_bash_want_json_module.yml +- import_playbook: custom_bash_hashbang_argument.yml - import_playbook: custom_binary_producing_json.yml - import_playbook: custom_binary_producing_junk.yml - import_playbook: custom_binary_single_null.yml diff --git a/tests/ansible/integration/runner/custom_bash_hashbang_argument.yml b/tests/ansible/integration/runner/custom_bash_hashbang_argument.yml new file mode 100644 index 00000000..f02b8419 --- /dev/null +++ b/tests/ansible/integration/runner/custom_bash_hashbang_argument.yml @@ -0,0 +1,19 @@ +# https://github.com/dw/mitogen/issues/291 +- name: integration/runner/custom_bash_hashbang_argument.yml + hosts: test-targets + any_errors_fatal: true + tasks: + + - custom_bash_old_style_module: + foo: true + with_sequence: start=1 end={{end|default(1)}} + register: out + vars: + ansible_bash_interpreter: "/usr/bin/env RUN_VIA_ENV=yes bash" + + - assert: + that: | + (not out.changed) and + (not out.results[0].changed) and + out.results[0].msg == 'Here is my input' and + out.results[0].run_via_env == "yes" diff --git a/tests/ansible/lib/modules/custom_bash_old_style_module.sh b/tests/ansible/lib/modules/custom_bash_old_style_module.sh index 9f80dc28..04f1bcd9 100755 --- a/tests/ansible/lib/modules/custom_bash_old_style_module.sh +++ b/tests/ansible/lib/modules/custom_bash_old_style_module.sh @@ -16,5 +16,6 @@ echo "{" echo " \"changed\": false," echo " \"msg\": \"Here is my input\"," echo " \"filename\": \"$INPUT\"," +echo " \"run_via_env\": \"$RUN_VIA_ENV\"," echo " \"input\": [\"$(cat $INPUT | tr \" \' )\"]" echo "}" From b5e7e97c620dde0d52a7868075003b2efb2e417d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 17 Jul 2018 23:50:21 +0100 Subject: [PATCH 21/56] issue #291: update changelog. --- docs/changelog.rst | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d0d054c6..0756a371 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,9 +21,18 @@ v0.2.2 (2018-07-??) Mitogen for Ansible ~~~~~~~~~~~~~~~~~~~ -* `#299 `_: fix the ``network_cli`` +* `#291 `_: compatibility: + ``ansible_*_interpreter`` variables are parsed using UNIX hashbang syntax, + i.e. with support for a single space-separated argument. This supports a + common idiom where ``ansible_python_interpreter`` is set to ``/usr/bin/env + python``. + +* `#299 `_: fix the ``network_cli`` connection type when the Mitogen strategy is active. +* `#303 `_: the ``doas`` become method + is now supported. Contributed by Mike Walker. + * `#309 `_: fix a regression to process environment cleanup, caused by the change in v0.2.1 to run local tasks with the correct environment. @@ -32,10 +41,14 @@ Mitogen for Ansible Core Library ~~~~~~~~~~~~ +* `#291 `_: the ``python_path`` + paramater may specify an argument vector prefix rather than a single string + program path. + * `#303 `_: the ``doas`` become method is now supported. Contributed by Mike Walker. -* `#307 `_: SSH login banner output +* `#307 `_: SSH login banner output containing the word 'password' is no longer confused for a password prompt. * Debug logs containing command lines are printed with the minimal quoting and From 1a74938ee09252fbc48ae4cf201985349e9c8bf8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 18 Jul 2018 00:05:04 +0100 Subject: [PATCH 22/56] issue #291: missing env_wrapper.sh test script. --- tests/data/env_wrapper.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100755 tests/data/env_wrapper.sh diff --git a/tests/data/env_wrapper.sh b/tests/data/env_wrapper.sh new file mode 100755 index 00000000..afb523f0 --- /dev/null +++ b/tests/data/env_wrapper.sh @@ -0,0 +1,12 @@ +#!/bin/bash +# This script exists to test the behavior of Stream.python_path being set to a +# list. It sets an environmnt variable that we can detect, then executes any +# arguments passed to it. +export EXECUTED_VIA_ENV_WRAPPER=1 +if [ "${1:0:1}" == "-" ]; then + exec "$PYTHON" "$@" +else + export ENV_WRAPPER_FIRST_ARG="$1" + shift + exec "$@" +fi From 6b79db2ecd9ccaf04e7b9b5bf990040bf1689197 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 19 Jul 2018 12:06:42 -0400 Subject: [PATCH 23/56] docs: document local connection process model difference. --- docs/ansible.rst | 51 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index b667fd37..d179a6ac 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -137,7 +137,7 @@ Noteworthy Differences `lxd `_, and `ssh `_ built-in connection types are supported, along with Mitogen-specific - :ref:`machinectl `, :ref:`mitogen_doas< mitogen_doas>`, + :ref:`machinectl `, :ref:`mitogen_doas `, :ref:`mitogen_su `, :ref:`mitogen_sudo `, and :ref:`setns ` types. File bugs to register interest in others. @@ -514,22 +514,6 @@ connection delegation is supported. * ``ansible_user``: Name of user within the container to execute as. -.. _machinectl: - -Machinectl -~~~~~~~~~~ - -Like the `machinectl third party plugin -`_ except -connection delegation is supported. This is a light wrapper around the -:ref:`setns ` method. - -* ``ansible_host``: Name of Docker container (default: inventory hostname). -* ``ansible_user``: Name of user within the container to execute as. -* ``mitogen_machinectl_path``: path to ``machinectl`` command if not available - as ``/bin/machinectl``. - - FreeBSD Jail ~~~~~~~~~~~~ @@ -551,6 +535,23 @@ connection delegation is supported. * ``ansible_python_interpreter`` +Process Model +^^^^^^^^^^^^^ + +Ansible usually executes local connection commands as a transient subprocess of +the forked worker executing a task. With the extension, the local connection +exists as a persistent subprocess of the connection multiplexer. + +This means that global state mutations made to the top-level Ansible process +that are normally visible to newly forked subprocesses, such as vars plug-ins +that modify the environment, will not be reflected when executing local +commands without additional effort. + +During execution the extension presently mimics the working directory and +process environment inheritence of regular Ansible, however it is possible some +additional differences exist that may break existing playbooks. + + .. _method-lxc: LXC @@ -567,6 +568,22 @@ The ``lxc-attach`` command must be available on the host machine. * ``ansible_host``: Name of LXC container (default: inventory hostname). +.. _machinectl: + +Machinectl +~~~~~~~~~~ + +Like the `machinectl third party plugin +`_ except +connection delegation is supported. This is a light wrapper around the +:ref:`setns ` method. + +* ``ansible_host``: Name of Docker container (default: inventory hostname). +* ``ansible_user``: Name of user within the container to execute as. +* ``mitogen_machinectl_path``: path to ``machinectl`` command if not available + as ``/bin/machinectl``. + + .. _setns: Setns From f7e288fa2599a93d33e605cb044b71cb0eecd27f Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 19 Jul 2018 15:12:54 -0400 Subject: [PATCH 24/56] core: fd 0/1 were accidently made non-blocking. This breaks regular code. Triggered by a huge pprint() in the child to stdout. --- mitogen/core.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 6f536ec8..c0b864f7 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -851,7 +851,7 @@ class LogHandler(logging.Handler): class Side(object): _fork_refs = weakref.WeakValueDictionary() - def __init__(self, stream, fd, cloexec=True, keep_alive=True): + def __init__(self, stream, fd, cloexec=True, keep_alive=True, blocking=False): self.stream = stream self.fd = fd self.closed = False @@ -859,7 +859,8 @@ class Side(object): self._fork_refs[id(self)] = self if cloexec: set_cloexec(fd) - set_nonblock(fd) + if not blocking: + set_nonblock(fd) def __repr__(self): return '' % (self.stream, self.fd) @@ -1520,7 +1521,7 @@ class IoLogger(BasicStream): set_cloexec(self._wsock.fileno()) self.receive_side = Side(self, self._rsock.fileno()) - self.transmit_side = Side(self, dest_fd, cloexec=False) + self.transmit_side = Side(self, dest_fd, cloexec=False, blocking=True) self._broker.start_receive(self) def __repr__(self): From d26fe5b993393679069914eb2f632159555640a8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 19 Jul 2018 15:17:09 -0400 Subject: [PATCH 25/56] issue #310: fix negative imports on Python 3.x. On 3.x, Importer() can still have its methods called even if load_module() raises ImportError. Closes #310. --- mitogen/core.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mitogen/core.py b/mitogen/core.py index c0b864f7..dd706311 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -815,10 +815,21 @@ class Importer(object): def get_filename(self, fullname): if fullname in self._cache: + path = self._cache[fullname][2] + if path is None: + # If find_loader() returns self but a subsequent master RPC + # reveals the module can't be loaded, and so load_module() + # throws ImportError, on Python 3.x it is still possible for + # the loader to be called to fetch metadata. + raise ImportError('master cannot serve %r' % (fullname,)) return u'master:' + self._cache[fullname][2] def get_source(self, fullname): if fullname in self._cache: + compressed = self._cache[fullname][3] + if compressed is None: + raise ImportError('master cannot serve %r' % (fullname,)) + source = zlib.decompress(self._cache[fullname][3]) if PY3: return to_text(source) From 417e02bd6d96fd50b8bb63415ec56cdc1051c7b1 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 20 Jul 2018 11:27:02 -0400 Subject: [PATCH 26/56] ansible: copy Ansible's method for finding the display. --- ansible_mitogen/logging.py | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/ansible_mitogen/logging.py b/ansible_mitogen/logging.py index d0fa13c6..cb472336 100644 --- a/ansible_mitogen/logging.py +++ b/ansible_mitogen/logging.py @@ -34,15 +34,20 @@ import sys import mitogen.core import mitogen.utils +try: + from __main__ import display +except ImportError: + from ansible.utils.display import Display + display = Display() + class Handler(logging.Handler): """ Use Mitogen's log format, but send the result to a Display method. """ - def __init__(self, display, normal_method): + def __init__(self, normal_method): logging.Handler.__init__(self) self.formatter = mitogen.utils.log_get_formatter() - self.display = display self.normal_method = normal_method #: Set of target loggers that produce warnings and errors that spam the @@ -62,36 +67,21 @@ class Handler(logging.Handler): s = '[pid %d] %s' % (os.getpid(), self.format(record)) if record.levelno >= logging.ERROR: - self.display.error(s, wrap_text=False) + display.error(s, wrap_text=False) elif record.levelno >= logging.WARNING: - self.display.warning(s, formatted=True) + display.warning(s, formatted=True) else: self.normal_method(s) -def find_display(): - """ - Find the CLI tool's display variable somewhere up the stack. Why god why, - right? Because it's the the simplest way to get access to the verbosity - configured on the command line. - """ - f = sys._getframe() - while f: - if 'display' in f.f_locals: - return f.f_locals['display'] - f = f.f_back - - def setup(): """ Install a handler for Mitogen's logger to redirect it into the Ansible display framework, and prevent propagation to the root logger. """ - display = find_display() - - logging.getLogger('ansible_mitogen').handlers = [Handler(display, display.vvv)] - mitogen.core.LOG.handlers = [Handler(display, display.vvv)] - mitogen.core.IOLOG.handlers = [Handler(display, display.vvvv)] + logging.getLogger('ansible_mitogen').handlers = [Handler(display.vvv)] + mitogen.core.LOG.handlers = [Handler(display.vvv)] + mitogen.core.IOLOG.handlers = [Handler(display.vvvv)] mitogen.core.IOLOG.propagate = False if display.verbosity > 2: From b6d6468c92e4dcd41ea1636c1022bf36ee5c5bbc Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 22 Jul 2018 16:09:04 -0400 Subject: [PATCH 27/56] issue #301: support expandvars() for remote_tmp only. Vanilla Ansible support expandvars-like expansions widely in a variety of places. Prefer to whitelist those we need, rather than sprinkling hellish semantics everywhere. --- ansible_mitogen/target.py | 3 +++ tests/ansible/ansible.cfg | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ansible_mitogen/target.py b/ansible_mitogen/target.py index c9be4bfb..22c5cb02 100644 --- a/ansible_mitogen/target.py +++ b/ansible_mitogen/target.py @@ -407,6 +407,9 @@ def make_temp_directory(base_dir): :returns: Newly created temporary directory. """ + # issue #301: remote_tmp may contain $vars. + base_dir = os.path.expandvars(base_dir) + if not os.path.exists(base_dir): os.makedirs(base_dir, mode=int('0700', 8)) return tempfile.mkdtemp( diff --git a/tests/ansible/ansible.cfg b/tests/ansible/ansible.cfg index e00cc974..7bf849d5 100644 --- a/tests/ansible/ansible.cfg +++ b/tests/ansible/ansible.cfg @@ -17,8 +17,9 @@ timeout = 10 # On Travis, paramiko check fails due to host key checking enabled. host_key_checking = False -# Required by integration/runner__remote_tmp.yml -remote_tmp = ~/.ansible/mitogen-tests/ +# "mitogen-tests" required by integration/runner/remote_tmp.yml +# "$HOME" required by integration/action/make_tmp_path.yml +remote_tmp = $HOME/.ansible/mitogen-tests/ [ssh_connection] ssh_args = -o ForwardAgent=yes -o ControlMaster=auto -o ControlPersist=60s From 17dda781c0a043755698bbcd9930eada7d097b8b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 13:33:08 -0700 Subject: [PATCH 28/56] issue #317: ansible: fix log filtering in several cases * mitogen/ansible_mitogen should only generate ERROR-level logs in log_path unless -vvv is enabled. * Targets were accidentally configured to always have DEBUG set, causing many log messages to be sent on the wire even though they would be filtered in the master. Closes #317. --- ansible_mitogen/logging.py | 10 +++++++++- ansible_mitogen/services.py | 3 ++- ansible_mitogen/target.py | 11 ++++++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/ansible_mitogen/logging.py b/ansible_mitogen/logging.py index cb472336..37e309e2 100644 --- a/ansible_mitogen/logging.py +++ b/ansible_mitogen/logging.py @@ -85,8 +85,16 @@ def setup(): mitogen.core.IOLOG.propagate = False if display.verbosity > 2: - logging.getLogger('ansible_mitogen').setLevel(logging.DEBUG) mitogen.core.LOG.setLevel(logging.DEBUG) + logging.getLogger('ansible_mitogen').setLevel(logging.DEBUG) + else: + # Mitogen copies the active log level into new children, allowing them + # to filter tiny messages before they hit the network, and therefore + # before they wake the IO loop. Explicitly setting INFO saves ~4% + # running against just the local machine. + mitogen.core.LOG.setLevel(logging.ERROR) + logging.getLogger('ansible_mitogen').setLevel(logging.ERROR) if display.verbosity > 3: mitogen.core.IOLOG.setLevel(logging.DEBUG) + logging.getLogger('ansible_mitogen').setLevel(logging.DEBUG) diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index ad28ce48..4f4779fe 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -297,7 +297,8 @@ class ContextService(mitogen.service.Service): lambda: self._on_stream_disconnect(stream)) self._send_module_forwards(context) - init_child_result = context.call(ansible_mitogen.target.init_child) + init_child_result = context.call(ansible_mitogen.target.init_child, + log_level=LOG.getEffectiveLevel()) if os.environ.get('MITOGEN_DUMP_THREAD_STACKS'): from mitogen import debug diff --git a/ansible_mitogen/target.py b/ansible_mitogen/target.py index 22c5cb02..e5365dd4 100644 --- a/ansible_mitogen/target.py +++ b/ansible_mitogen/target.py @@ -202,7 +202,7 @@ def reset_temp_dir(econtext): @mitogen.core.takes_econtext -def init_child(econtext): +def init_child(econtext, log_level): """ Called by ContextService immediately after connection; arranges for the (presently) spotless Python interpreter to be forked, where the newly @@ -213,6 +213,9 @@ def init_child(econtext): polluting the global interpreter state in a way that effects explicitly isolated modules. + :param int log_level: + Logging package level active in the master. + :returns: Dict like:: @@ -230,6 +233,12 @@ def init_child(econtext): _fork_parent = econtext.router.fork() reset_temp_dir(econtext) + # Copying the master's log level causes log messages to be filtered before + # they reach LogForwarder, thus reducing an influx of tiny messges waking + # the connection multiplexer process in the master. + LOG.setLevel(log_level) + logging.getLogger('ansible_mitogen').setLevel(log_level) + return { 'fork_context': _fork_parent, 'home_dir': mitogen.core.to_text(os.path.expanduser('~')), From 11c73baa9c622332cb78af535dc50d2055ff00cc Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 14:01:27 -0700 Subject: [PATCH 29/56] docs: update Changelog. --- docs/ansible.rst | 7 +++++++ docs/changelog.rst | 29 +++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index d179a6ac..8750aec1 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -158,6 +158,13 @@ Noteworthy Differences may be established in parallel by default, this can be modified by setting the ``MITOGEN_POOL_SIZE`` environment variable. +* The ``ansible_python_interpreter`` variable is parsed using a restrictive + :mod:`shell-like ` syntax, permitting values such as ``/usr/bin/env + FOO=bar python``, which occur in practice. Ansible `documents this + `_ + as an absolute path, however the implementation passes it unquoted through + the shell, permitting arbitrary code to be injected. + * Performance does not scale linearly with target count. This will improve over time. diff --git a/docs/changelog.rst b/docs/changelog.rst index 0756a371..dae6a92a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,14 +28,30 @@ Mitogen for Ansible python``. * `#299 `_: fix the ``network_cli`` - connection type when the Mitogen strategy is active. + connection type when the Mitogen strategy is active. Mitogen does not help + connections to network devices, but it should still be possible to use such + connections when Mitogen is active. + +* `#301 `_: compatibility: the presence + of variables such as ``$HOME`` in the ``remote_tmp`` setting are expanded. * `#303 `_: the ``doas`` become method - is now supported. Contributed by Mike Walker. + is now supported. Contributed by `Mike Walker + `_. + +* `#309 `_: fix a regression to + process environment cleanup, caused by the change in v0.2.1 to run local + tasks with the correct environment. + +* `#315 `_: the extension is now + supported under Ansible 2.6. Contributed by `Dan Quackenbush + `_. -* `#309 `_: fix a regression to process - environment cleanup, caused by the change in v0.2.1 to run local tasks with - the correct environment. +* `#317 `_: respect the verbosity + setting when writing to to Ansible's ``log_path`` log file, if it is enabled. + Child log filtering was also incorrect, causing the master to needlessly wake + many times. This nets a 3.5% runtime improvement running against the local + machine. Core Library @@ -46,7 +62,8 @@ Core Library program path. * `#303 `_: the ``doas`` become method - is now supported. Contributed by Mike Walker. + is now supported. Contributed by `Mike Walker + `_. * `#307 `_: SSH login banner output containing the word 'password' is no longer confused for a password prompt. From 9410903f20b8e32d1fcc61e324917a471d8507ab Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 14:52:18 -0700 Subject: [PATCH 30/56] issue #301: add related test. --- tests/ansible/integration/action/all.yml | 1 + .../integration/action/remote_expand_user.yml | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tests/ansible/integration/action/remote_expand_user.yml diff --git a/tests/ansible/integration/action/all.yml b/tests/ansible/integration/action/all.yml index d7f2b583..ebbff26a 100644 --- a/tests/ansible/integration/action/all.yml +++ b/tests/ansible/integration/action/all.yml @@ -1,4 +1,5 @@ - import_playbook: remote_file_exists.yml +- import_playbook: remote_expand_user.yml - import_playbook: low_level_execute_command.yml - import_playbook: make_tmp_path.yml - import_playbook: transfer_data.yml diff --git a/tests/ansible/integration/action/remote_expand_user.yml b/tests/ansible/integration/action/remote_expand_user.yml new file mode 100644 index 00000000..632e9492 --- /dev/null +++ b/tests/ansible/integration/action/remote_expand_user.yml @@ -0,0 +1,32 @@ +# related to issue 301. Essentially ensure remote_expand_user does not support +# $HOME expansion. + +- name: integration/action/remote_expand_user.yml + hosts: test-targets + any_errors_fatal: true + gather_facts: true + tasks: + + # Expand ~/foo + - action_passthrough: + method: _remote_expand_user + args: ['~/foo'] + register: out + - assert: + that: out.result == '{{ansible_user_dir}}/foo' + + # Expand ~user/foo + - action_passthrough: + method: _remote_expand_user + args: ['~{{ansible_user_id}}/foo'] + register: out + - assert: + that: out.result == '{{ansible_user_dir}}/foo' + + # Expanding $HOME/foo has no effect. + - action_passthrough: + method: _remote_expand_user + args: ['$HOME/foo'] + register: out + - assert: + that: out.result == '$HOME/foo' From 2c74eac19ab766b2db9bc20e9c548391ef7264da Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 17:40:17 -0700 Subject: [PATCH 31/56] issue #291: more Ansible-compatible script invocation When running any kind of script, rewrite the hashbang like Ansible does, but subsequently ignore it and explicitly use a fragment of shell from the ansible_*_interpreter variable to call the interpreter, just like Ansible does. This fixes hashbangs containing '/usr/bin/env A=1 bash' on Linux, where putting that into a hashbang line results in an infinite loop. --- ansible_mitogen/connection.py | 7 ++-- ansible_mitogen/planner.py | 46 ++++++++++++++++++------ ansible_mitogen/runner.py | 66 +++++++++++++++++++++-------------- docs/changelog.rst | 7 ++-- 4 files changed, 80 insertions(+), 46 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 464a9d81..a33e1ed0 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -251,16 +251,13 @@ CONNECTION_METHOD = { def parse_python_path(s): """ - Given the string set for ansible_python_interpeter, parse it as hashbang + Given the string set for ansible_python_interpeter, parse it using shell syntax and return an appropriate argument vector. """ if not s: return None - interpreter, arg = ansible_mitogen.parsing.parse_script_interpreter(s) - if arg: - return [interpreter, arg] - return [interpreter] + return shlex.split(s) def config_from_play_context(transport, inventory_name, connection): diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index c5636773..c297ad8f 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -187,27 +187,51 @@ class ScriptPlanner(BinaryPlanner): Common functionality for script module planners -- handle interpreter detection and rewrite. """ + def _rewrite_interpreter(self, path): + """ + Given the original interpreter binary extracted from the script's + interpreter line, look up the associated `ansible_*_interpreter` + variable, render it and return it. + + :param str path: + Absolute UNIX path to original interpreter. + + :returns: + Shell fragment prefix used to execute the script via "/bin/sh -c". + While `ansible_*_interpreter` documentation suggests shell isn't + involved here, the vanilla implementation uses it and that use is + exploited in common playbooks. + """ + try: + key = u'ansible_%s_interpreter' % os.path.basename(path).strip() + template = self._inv.task_vars[key] + except KeyError: + return path + + return mitogen.utils.cast( + self._inv.templar.template(self._inv.task_vars[key]) + ) + def _get_interpreter(self): - interpreter, arg = ansible_mitogen.parsing.parse_hashbang( + path, arg = ansible_mitogen.parsing.parse_hashbang( self._inv.module_source ) - if interpreter is None: + if path is None: raise ansible.errors.AnsibleError(NO_INTERPRETER_MSG % ( self._inv.module_name, )) - key = u'ansible_%s_interpreter' % os.path.basename(interpreter).strip() - try: - template = self._inv.task_vars[key].strip() - return self._inv.templar.template(template), arg - except KeyError: - return interpreter, arg + fragment = self._rewrite_interpreter(path) + if arg: + fragment += ' ' + arg + + return fragment, path.startswith('python') def get_kwargs(self, **kwargs): - interpreter, arg = self._get_interpreter() + interpreter_fragment, is_python = self._get_interpreter() return super(ScriptPlanner, self).get_kwargs( - interpreter_arg=arg, - interpreter=interpreter, + interpreter_fragment=interpreter_fragment, + is_python=is_python, **kwargs ) diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index 3960cd38..89dccb81 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -381,11 +381,10 @@ class ProgramRunner(Runner): ) def _get_program_args(self): - return [ - self.args['_ansible_shell_executable'], - '-c', - self.program_fp.name - ] + """ + Return any arguments to pass to the program. + """ + return [] def revert(self): """ @@ -395,14 +394,20 @@ class ProgramRunner(Runner): self.program_fp.close() super(ProgramRunner, self).revert() + def _get_argv(self): + """ + Return the final argument vector used to execute the program. + """ + return [self.program_fp.name] + self._get_program_args() + def _run(self): try: rc, stdout, stderr = ansible_mitogen.target.exec_args( - args=self._get_program_args(), + args=self._get_argv(), emulate_tty=self.emulate_tty, ) except Exception as e: - LOG.exception('While running %s', self._get_program_args()) + LOG.exception('While running %s', self._get_argv()) return { 'rc': 1, 'stdout': '', @@ -442,11 +447,7 @@ class ArgsFileRunner(Runner): return json.dumps(self.args) def _get_program_args(self): - return [ - self.args['_ansible_shell_executable'], - '-c', - "%s %s" % (self.program_fp.name, self.args_fp.name), - ] + return [self.args_fp.name] def revert(self): """ @@ -461,10 +462,10 @@ class BinaryRunner(ArgsFileRunner, ProgramRunner): class ScriptRunner(ProgramRunner): - def __init__(self, interpreter, interpreter_arg, **kwargs): + def __init__(self, interpreter_fragment, is_python, **kwargs): super(ScriptRunner, self).__init__(**kwargs) - self.interpreter = interpreter - self.interpreter_arg = interpreter_arg + self.interpreter_fragment = interpreter_fragment + self.is_python = is_python b_ENCODING_STRING = b'# -*- coding: utf-8 -*-' @@ -473,21 +474,34 @@ class ScriptRunner(ProgramRunner): super(ScriptRunner, self)._get_program() ) + def _get_argv(self): + return [ + self.args['_ansible_shell_executable'], + '-c', + self._get_shell_fragment(), + ] + + def _get_shell_fragment(self): + """ + Scripts are eligible for having their hashbang line rewritten, and to + be executed via /bin/sh using the ansible_*_interpreter value used as a + shell fragment prefixing to the invocation. + """ + return "%s %s %s" % ( + self.interpreter_fragment, + shlex_quote(self.program_fp.name), + ' '.join(map(shlex_quote, self._get_program_args())), + ) + def _rewrite_source(self, s): """ Mutate the source according to the per-task parameters. """ - # Couldn't find shebang, so let shell run it, because shell assumes - # executables like this are just shell scripts. - if not self.interpreter: - return s - - shebang = b'#!' + utf8(self.interpreter) - if self.interpreter_arg: - shebang += b' ' + utf8(self.interpreter_arg) - - new = [shebang] - if os.path.basename(self.interpreter).startswith('python'): + # While Ansible rewrites the #! using ansible_*_interpreter, it is + # never actually used to execute the script, instead it is a shell + # fragment consumed by shell/__init__.py::build_module_command(). + new = [b'#!' + utf8(self.interpreter_fragment)] + if self.is_python: new.append(self.b_ENCODING_STRING) _, _, rest = s.partition(b'\n') diff --git a/docs/changelog.rst b/docs/changelog.rst index dae6a92a..6e2d6328 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,10 +22,9 @@ Mitogen for Ansible ~~~~~~~~~~~~~~~~~~~ * `#291 `_: compatibility: - ``ansible_*_interpreter`` variables are parsed using UNIX hashbang syntax, - i.e. with support for a single space-separated argument. This supports a - common idiom where ``ansible_python_interpreter`` is set to ``/usr/bin/env - python``. + ``ansible_*_interpreter`` variables are parsed using a restrictive shell-like + syntax, supporting a common idiom where ``ansible_python_interpreter`` is set + to ``/usr/bin/env python``. * `#299 `_: fix the ``network_cli`` connection type when the Mitogen strategy is active. Mitogen does not help From c5ea7c45a168a3fee3c2635be9c63b75f64c761f Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 19:11:23 -0700 Subject: [PATCH 32/56] comments/docs: correct mitogen.master.Context -> mitogen.parent.Context. --- ansible_mitogen/connection.py | 10 +++++----- ansible_mitogen/services.py | 2 +- docs/getting_started.rst | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index a33e1ed0..21836cae 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -344,15 +344,15 @@ class Connection(ansible.plugins.connection.ConnectionBase): #: mitogen.master.Router for this worker. router = None - #: mitogen.master.Context representing the parent Context, which is + #: mitogen.parent.Context representing the parent Context, which is #: presently always the connection multiplexer process. parent = None - #: mitogen.master.Context connected to the target user account on the - #: target machine (i.e. via sudo). + #: mitogen.parent.Context connected to the target user account on the + #: target machine (i.e. possibly via sudo). context = None - #: mitogen.master.Context connected to the fork parent process in the + #: mitogen.parent.Context connected to the fork parent process in the #: target user account. fork_context = None @@ -492,7 +492,7 @@ class Connection(ansible.plugins.connection.ConnectionBase): """ Establish a connection to the master process's UNIX listener socket, constructing a mitogen.master.Router to communicate with the master, - and a mitogen.master.Context to represent it. + and a mitogen.parent.Context to represent it. Depending on the original transport we should emulate, trigger one of the _connect_*() service calls defined above to cause the master diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index 4f4779fe..8bdb9b9d 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -358,7 +358,7 @@ class ContextService(mitogen.service.Service): Subsequent elements are proxied via the previous. :returns dict: - * context: mitogen.master.Context or None. + * context: mitogen.parent.Context or None. * init_child_result: Result of :func:`init_child`. * msg: StreamError exception text or None. * method_name: string failing method name. diff --git a/docs/getting_started.rst b/docs/getting_started.rst index 77defd1c..436b34a1 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -163,7 +163,7 @@ Logging Records Messages received from a child context via :class:`mitogen.master.LogForwarder` receive extra attributes: -* `mitogen_context`: :class:`mitogen.master.Context` referring to the message +* `mitogen_context`: :class:`mitogen.parent.Context` referring to the message source. * `mitogen_name`: original logger name in the source context. * `mitogen_msg`: original message in the source context. From de0d526487110390bd1d83d68b901fb57732265b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 21:41:34 -0700 Subject: [PATCH 33/56] issue #291: restore behaviour of invoking binaries via /bin/sh This ensures failed task output matches vanilla Ansible exactly (as it did before starting #291). --- ansible_mitogen/runner.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index 89dccb81..ca3928b3 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -398,7 +398,17 @@ class ProgramRunner(Runner): """ Return the final argument vector used to execute the program. """ - return [self.program_fp.name] + self._get_program_args() + return [ + self.args['_ansible_shell_executable'], + '-c', + self._get_shell_fragment(), + ] + + def _get_shell_fragment(self): + return "%s %s" % ( + shlex_quote(self.program_fp.name), + ' '.join(map(shlex_quote, self._get_program_args())), + ) def _run(self): try: From a8e4dcc98ddc13eb316936578f3d1276e0742dfb Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 21:43:06 -0700 Subject: [PATCH 34/56] issue #301: correct remote_tmp evaluation context. Vanilla Ansible expands remote_tmp variables in the context of the login account, not any become_user account. --- ansible_mitogen/connection.py | 24 +++++++++-- ansible_mitogen/mixins.py | 3 +- ansible_mitogen/services.py | 2 + .../integration/action/make_tmp_path.yml | 42 ++++++++++++++++++- .../integration/action/remote_expand_user.yml | 37 ++++++++++++---- 5 files changed, 93 insertions(+), 15 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 21836cae..ced67ac0 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -348,10 +348,14 @@ class Connection(ansible.plugins.connection.ConnectionBase): #: presently always the connection multiplexer process. parent = None - #: mitogen.parent.Context connected to the target user account on the - #: target machine (i.e. possibly via sudo). + #: mitogen.parent.Context for the target account on the target, possibly + #: reached via become. context = None + #: mitogen.parent.Context for the login account on the target. This is + #: always the login account, even when become=True. + login_context = None + #: mitogen.parent.Context connected to the fork parent process in the #: target user account. fork_context = None @@ -529,6 +533,11 @@ class Connection(ansible.plugins.connection.ConnectionBase): raise ansible.errors.AnsibleConnectionFailure(dct['msg']) self.context = dct['context'] + if self._play_context.become: + self.login_context = dct['via'] + else: + self.login_context = self.context + self.fork_context = dct['init_child_result']['fork_context'] self.home_dir = dct['init_child_result']['home_dir'] @@ -546,6 +555,8 @@ class Connection(ansible.plugins.connection.ConnectionBase): ) self.context = None + self.fork_context = None + self.login_context = None if self.broker and not new_task: self.broker.shutdown() self.broker.join() @@ -556,11 +567,18 @@ class Connection(ansible.plugins.connection.ConnectionBase): """ Start a function call to the target. + :param bool use_login_context: + If present and :data:`True`, send the call to the login account + context rather than the optional become user context. :returns: mitogen.core.Receiver that receives the function call result. """ self._connect() - return self.context.call_async(func, *args, **kwargs) + if kwargs.pop('use_login_context', None): + call_context = self.login_context + else: + call_context = self.context + return call_context.call_async(func, *args, **kwargs) def call(self, func, *args, **kwargs): """ diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index 86b9fdd5..5b8b90dd 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -200,9 +200,10 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): # _make_tmp_path() is basically a global stashed away as Shell.tmpdir. # The copy action plugin violates layering and grabs this attribute # directly. - self._connection._shell.tmpdir = self.call( + self._connection._shell.tmpdir = self._connection.call( ansible_mitogen.target.make_temp_directory, base_dir=self._get_remote_tmp(), + use_login_context=True, ) LOG.debug('Temporary directory: %r', self._connection._shell.tmpdir) self._cleanup_remote_tmp = True diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index 8bdb9b9d..a7bb7db1 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -267,6 +267,7 @@ class ContextService(mitogen.service.Service): { 'context': mitogen.core.Context or None, + 'via': mitogen.core.Context or None, 'init_child_result': { 'fork_context': mitogen.core.Context, 'home_dir': str or None, @@ -308,6 +309,7 @@ class ContextService(mitogen.service.Service): self._refs_by_context[context] = 0 return { 'context': context, + 'via': via, 'init_child_result': init_child_result, 'msg': None, } diff --git a/tests/ansible/integration/action/make_tmp_path.yml b/tests/ansible/integration/action/make_tmp_path.yml index 603ebd09..83261208 100644 --- a/tests/ansible/integration/action/make_tmp_path.yml +++ b/tests/ansible/integration/action/make_tmp_path.yml @@ -2,16 +2,54 @@ - name: integration/action/make_tmp_path.yml hosts: test-targets any_errors_fatal: true - gather_facts: true tasks: + - name: "Find out root's homedir." + # Runs first because it blats regular Ansible facts with junk, so + # non-become run fixes that up. + setup: gather_subset=min + become: true + register: root_facts + + - name: "Find regular homedir" + setup: gather_subset=min + register: user_facts + + # + # non-become + # + + - action_passthrough: + method: _make_tmp_path + register: out + + - assert: + # This string must match ansible.cfg::remote_tmp + that: out.result.startswith("{{user_facts.ansible_facts.ansible_user_dir}}/.ansible/mitogen-tests/") + + - stat: + path: "{{out.result}}" + register: st + + - assert: + that: st.stat.exists and st.stat.isdir and st.stat.mode == "0700" + + - file: + path: "{{out.result}}" + state: absent + + # + # become. make_tmp_path() must evaluate HOME in the context of the SSH + # user, not the become user. + # - action_passthrough: method: _make_tmp_path register: out + become: true - assert: # This string must match ansible.cfg::remote_tmp - that: out.result.startswith("{{ansible_user_dir}}/.ansible/mitogen-tests/") + that: out.result.startswith("{{user_facts.ansible_facts.ansible_user_dir}}/.ansible/mitogen-tests/") - stat: path: "{{out.result}}" diff --git a/tests/ansible/integration/action/remote_expand_user.yml b/tests/ansible/integration/action/remote_expand_user.yml index 632e9492..813fbace 100644 --- a/tests/ansible/integration/action/remote_expand_user.yml +++ b/tests/ansible/integration/action/remote_expand_user.yml @@ -4,27 +4,46 @@ - name: integration/action/remote_expand_user.yml hosts: test-targets any_errors_fatal: true - gather_facts: true tasks: + - name: "Find out root's homedir." + # Runs first because it blats regular Ansible facts with junk, so + # non-become run fixes that up. + setup: gather_subset=min + become: true + register: root_facts - # Expand ~/foo - - action_passthrough: + - name: "Find regular homedir" + setup: gather_subset=min + register: user_facts + + - name: "Expand ~/foo" + action_passthrough: method: _remote_expand_user args: ['~/foo'] register: out - assert: - that: out.result == '{{ansible_user_dir}}/foo' + that: out.result == '{{user_facts.ansible_facts.ansible_user_dir}}/foo' + + - name: "Expand ~/foo with become active. ~ is become_user's home." + action_passthrough: + method: _remote_expand_user + args: ['~/foo'] + register: out + become: true + + - assert: + that: out.result == '{{root_facts.ansible_facts.ansible_user_dir}}/foo' - # Expand ~user/foo - - action_passthrough: + - name: "Expand ~user/foo" + action_passthrough: method: _remote_expand_user args: ['~{{ansible_user_id}}/foo'] register: out - assert: - that: out.result == '{{ansible_user_dir}}/foo' + that: out.result == '{{user_facts.ansible_facts.ansible_user_dir}}/foo' - # Expanding $HOME/foo has no effect. - - action_passthrough: + - name: "Expanding $HOME/foo has no effect." + action_passthrough: method: _remote_expand_user args: ['$HOME/foo'] register: out From 50670430ec23b36ccd2e6bcef52062e8d8ecf0e6 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 23 Jul 2018 22:13:13 -0700 Subject: [PATCH 35/56] docs: add thanks to release notes --- docs/changelog.rst | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6e2d6328..59fb2a5e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,28 +21,28 @@ v0.2.2 (2018-07-??) Mitogen for Ansible ~~~~~~~~~~~~~~~~~~~ -* `#291 `_: compatibility: - ``ansible_*_interpreter`` variables are parsed using a restrictive shell-like - syntax, supporting a common idiom where ``ansible_python_interpreter`` is set - to ``/usr/bin/env python``. +* `#291 `_: ``ansible_*_interpreter`` + variables are parsed using a restrictive shell-like syntax, supporting a + common idiom where ``ansible_python_interpreter`` is set to ``/usr/bin/env + python``. * `#299 `_: fix the ``network_cli`` - connection type when the Mitogen strategy is active. Mitogen does not help - connections to network devices, but it should still be possible to use such - connections when Mitogen is active. + connection type when the Mitogen strategy is active. Mitogen cannot help + network device connections, however it should still be possible to use device + connections while Mitogen is active. -* `#301 `_: compatibility: the presence - of variables such as ``$HOME`` in the ``remote_tmp`` setting are expanded. +* `#301 `_: variables like ``$HOME`` in + the ``remote_tmp`` setting are evaluated correctly. * `#303 `_: the ``doas`` become method - is now supported. Contributed by `Mike Walker + is supported. Contributed by `Mike Walker `_. * `#309 `_: fix a regression to process environment cleanup, caused by the change in v0.2.1 to run local tasks with the correct environment. -* `#315 `_: the extension is now +* `#315 `_: Mitogen for Ansible is supported under Ansible 2.6. Contributed by `Dan Quackenbush `_. @@ -71,6 +71,21 @@ Core Library escaping required. +Thanks! +~~~~~~~ + +Mitogen would not be possible without the support of users. A huge thanks for +the bug reports and pull requests in this release contributed by +`Frances Albanese `_, +`Mark Janssen `_, +`Ayaz Ahmed Khan `_, +`Colin McCarthy `_, +`Dan Quackenbush `_, +`Alex Russu `_, +`Josh Smift `_, and +`Mike Walker `_. + + v0.2.1 (2018-07-10) ------------------- From 662b2d0668fd67685d60a2b3f83e44da1f676fa2 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 24 Jul 2018 08:10:12 -0700 Subject: [PATCH 36/56] docs: whups, add missing contributors entry --- docs/contributors.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/contributors.rst b/docs/contributors.rst index 9eebaa95..ee5c3132 100644 --- a/docs/contributors.rst +++ b/docs/contributors.rst @@ -101,6 +101,7 @@ sponsorship and outstanding future-thinking of its early adopters.
    • Alex Willmer
    • Dan Dorman — - When I truly understand my enemy … then in that very moment I also love him.
    • +
    • Daniel Foerster
    • DepsPrivate Maven Repository Hosting for Java, Scala, Groovy, Clojure
    • Edward WilsonTo efficiency and beyond! I wish Mitogen and all who sail in her the best of luck.
    • Epartment
    • From 76caf7d41d532c619b679c8afbe34868db00387e Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 24 Jul 2018 16:04:14 -0700 Subject: [PATCH 37/56] issue #320: ignore kqueue events marked KQ_EV_ERROR. Since BasicStream.close() invokes _stop_transmit() followed by os.close(), and KqueuePoller._stop_transmit() defers the unsubscription until the IO loop resumes, kqueue generates an error event for the associated FD, even though the changelist includes an unsubscription command for the FD. We could fix this by deferring close() until after the IO loop has run once (simply by calling .defer()), but that generates extra wakeups for no real reason. Instead simply notice the error event and log it, rather than treating it as a legitimate event. Another approach to fixing this would be to process _stop_receive()/_stop_transmit() eagerly, however that entails making more syscalls. Closes #320. --- mitogen/parent.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index 38484874..50d78efd 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -566,7 +566,10 @@ class KqueuePoller(mitogen.core.Poller): changelist, 32, timeout) for event in events: fd = event.ident - if event.filter == select.KQ_FILTER_READ and fd in self._rfds: + if event.flags & select.KQ_EV_ERROR: + LOG.debug('ignoring stale event for fd %r: errno=%d: %s', + fd, event.data, errno.errorcode.get(event.data)) + elif event.filter == select.KQ_FILTER_READ and fd in self._rfds: # Events can still be read for an already-discarded fd. mitogen.core._vv and IOLOG.debug('%r: POLLIN: %r', self, fd) yield self._rfds[fd] From eae531210abac5a7dadf54ad2f14453fa5a4e5b9 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 25 Jul 2018 11:42:23 -0700 Subject: [PATCH 38/56] tests: allow passing -vvv to debops_common --- .travis/debops_common_tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis/debops_common_tests.sh b/.travis/debops_common_tests.sh index ec4c1481..50e67ada 100755 --- a/.travis/debops_common_tests.sh +++ b/.travis/debops_common_tests.sh @@ -81,10 +81,10 @@ echo travis_fold:end:job_setup echo travis_fold:start:first_run -/usr/bin/time debops common +/usr/bin/time debops common "$@" echo travis_fold:end:first_run echo travis_fold:start:second_run -/usr/bin/time debops common +/usr/bin/time debops common "$@" echo travis_fold:end:second_run From b44b823c4a3073be44351b7c430331e045a08666 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 25 Jul 2018 12:13:27 -0700 Subject: [PATCH 39/56] ansible: make _remote_expand_user() pay attention to sudoable=.. --- ansible_mitogen/mixins.py | 29 +++++---- .../integration/action/remote_expand_user.yml | 65 +++++++++++++++++-- 2 files changed, 77 insertions(+), 17 deletions(-) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index 5b8b90dd..2a9fdac8 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -188,12 +188,13 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): except AttributeError: s = ansible.constants.DEFAULT_REMOTE_TMP # <=2.4.x - return self._remote_expand_user(s) + return self._remote_expand_user(s, sudoable=False) def _make_tmp_path(self, remote_user=None): """ Replace the base implementation's use of shell to implement mkdtemp() - with an actual call to mkdtemp(). + with an actual call to mkdtemp(). Like vanilla, the directory is always + created in the login account context. """ LOG.debug('_make_tmp_path(remote_user=%r)', remote_user) @@ -281,20 +282,26 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): """ Replace the base implementation's attempt to emulate os.path.expanduser() with an actual call to os.path.expanduser(). + + :param bool sudoable: + If :data:`True`, indicate unqualified tilde ("~" with no username) + should be evaluated in the context of the login account, not any + become_user. """ LOG.debug('_remote_expand_user(%r, sudoable=%r)', path, sudoable) if not path.startswith('~'): # /home/foo -> /home/foo return path - if path == '~': - # ~ -> /home/dmw - return self._connection.homedir - if path.startswith('~/'): - # ~/.ansible -> /home/dmw/.ansible - return os.path.join(self._connection.homedir, path[2:]) - if path.startswith('~'): - # ~root/.ansible -> /root/.ansible - return self.call(os.path.expanduser, mitogen.utils.cast(path)) + if sudoable or not self._play_context.become: + if path == '~': + # ~ -> /home/dmw + return self._connection.homedir + if path.startswith('~/'): + # ~/.ansible -> /home/dmw/.ansible + return os.path.join(self._connection.homedir, path[2:]) + # ~root/.ansible -> /root/.ansible + return self.call(os.path.expanduser, mitogen.utils.cast(path), + use_login_context=not sudoable) def get_task_timeout_secs(self): """ diff --git a/tests/ansible/integration/action/remote_expand_user.yml b/tests/ansible/integration/action/remote_expand_user.yml index 813fbace..85990264 100644 --- a/tests/ansible/integration/action/remote_expand_user.yml +++ b/tests/ansible/integration/action/remote_expand_user.yml @@ -16,10 +16,14 @@ setup: gather_subset=min register: user_facts + # ------------------------ + - name: "Expand ~/foo" action_passthrough: method: _remote_expand_user - args: ['~/foo'] + kwargs: + path: '~/foo' + sudoable: false register: out - assert: that: out.result == '{{user_facts.ansible_facts.ansible_user_dir}}/foo' @@ -27,17 +31,20 @@ - name: "Expand ~/foo with become active. ~ is become_user's home." action_passthrough: method: _remote_expand_user - args: ['~/foo'] + kwargs: + path: '~/foo' + sudoable: false register: out become: true - - assert: - that: out.result == '{{root_facts.ansible_facts.ansible_user_dir}}/foo' + that: out.result == '{{user_facts.ansible_facts.ansible_user_dir}}/foo' - name: "Expand ~user/foo" action_passthrough: method: _remote_expand_user - args: ['~{{ansible_user_id}}/foo'] + kwargs: + path: '~{{ansible_user_id}}/foo' + sudoable: false register: out - assert: that: out.result == '{{user_facts.ansible_facts.ansible_user_dir}}/foo' @@ -45,7 +52,53 @@ - name: "Expanding $HOME/foo has no effect." action_passthrough: method: _remote_expand_user - args: ['$HOME/foo'] + kwargs: + path: '$HOME/foo' + sudoable: false + register: out + - assert: + that: out.result == '$HOME/foo' + + # ------------------------ + + - name: "sudoable; Expand ~/foo" + action_passthrough: + method: _remote_expand_user + kwargs: + path: '~/foo' + sudoable: true + register: out + - assert: + that: out.result == '{{user_facts.ansible_facts.ansible_user_dir}}/foo' + + - name: "sudoable; Expand ~/foo with become active. ~ is become_user's home." + action_passthrough: + method: _remote_expand_user + kwargs: + path: '~/foo' + sudoable: true + register: out + become: true + + - assert: + that: out.result == '{{root_facts.ansible_facts.ansible_user_dir}}/foo' + + - name: "sudoable; Expand ~user/foo" + action_passthrough: + method: _remote_expand_user + kwargs: + path: '~{{ansible_user_id}}/foo' + sudoable: true + register: out + - assert: + that: out.result == '{{user_facts.ansible_facts.ansible_user_dir}}/foo' + + - name: "sudoable; Expanding $HOME/foo has no effect." + action_passthrough: + method: _remote_expand_user + kwargs: + path: '$HOME/foo' + sudoable: true register: out - assert: that: out.result == '$HOME/foo' From 71b4294888c340a64900941de7b2796dd012cd63 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 25 Jul 2018 13:25:17 -0700 Subject: [PATCH 40/56] issue #291: 2.6 compat fix. --- ansible_mitogen/connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index ced67ac0..3be63bd4 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -257,7 +257,7 @@ def parse_python_path(s): if not s: return None - return shlex.split(s) + return ansible.utils.shlex.shlex_split(s) def config_from_play_context(transport, inventory_name, connection): From a29a883dfc4dafd530d47e83396e4606527a7d1a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 25 Jul 2018 14:03:26 -0700 Subject: [PATCH 41/56] issue #311: docs: comment out Ansible 2.6 for now. --- docs/ansible.rst | 2 +- docs/changelog.rst | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 8750aec1..0d0948f6 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -58,7 +58,7 @@ Installation ------------ 1. Thoroughly review the :ref:`noteworthy_differences` and :ref:`changelog`. -2. Verify Ansible 2.3-2.6.1 and Python 2.6, 2.7 or 3.6 are listed in ``ansible +2. Verify Ansible 2.3-2.5 and Python 2.6, 2.7 or 3.6 are listed in ``ansible --version`` output. 3. Download and extract |mitogen_url| from PyPI. 4. Modify ``ansible.cfg``: diff --git a/docs/changelog.rst b/docs/changelog.rst index 59fb2a5e..8d9a17ed 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,16 @@ Release Notes +.. comment + + v0.2.3 (2018-07-??) + ------------------- + + * `#315 `_: Mitogen for Ansible is + supported under Ansible 2.6. Contributed by `Dan Quackenbush + `_. + + v0.2.2 (2018-07-??) ------------------- @@ -42,10 +52,6 @@ Mitogen for Ansible process environment cleanup, caused by the change in v0.2.1 to run local tasks with the correct environment. -* `#315 `_: Mitogen for Ansible is - supported under Ansible 2.6. Contributed by `Dan Quackenbush - `_. - * `#317 `_: respect the verbosity setting when writing to to Ansible's ``log_path`` log file, if it is enabled. Child log filtering was also incorrect, causing the master to needlessly wake From bfe9f81d0b8cb789744d84473e6a322dbb671063 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 25 Jul 2018 16:37:49 -0700 Subject: [PATCH 42/56] ansible: fix RPC time logging. Rendering call arguemtns was broken for non-positional arguments. --- ansible_mitogen/connection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 3be63bd4..c45a8aa7 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -593,8 +593,8 @@ class Connection(ansible.plugins.connection.ConnectionBase): try: return self.call_async(func, *args, **kwargs).get().unpickle() finally: - LOG.debug('Call took %d ms: %s%r', 1000 * (time.time() - t0), - func.__name__, args) + LOG.debug('Call took %d ms: %r', 1000 * (time.time() - t0), + mitogen.parent.CallSpec(func, args, kwargs)) def create_fork_child(self): """ From 3138982ef4e1a17bbc548a3d4830448f76e53c43 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 25 Jul 2018 22:11:43 -0700 Subject: [PATCH 43/56] docs: link mitogen-announce mailing list. --- docs/ansible.rst | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 0d0948f6..37dfde1c 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -57,11 +57,9 @@ write files. Installation ------------ -1. Thoroughly review the :ref:`noteworthy_differences` and :ref:`changelog`. -2. Verify Ansible 2.3-2.5 and Python 2.6, 2.7 or 3.6 are listed in ``ansible - --version`` output. -3. Download and extract |mitogen_url| from PyPI. -4. Modify ``ansible.cfg``: +1. Thoroughly review :ref:`noteworthy_differences` and :ref:`changelog`. +2. Download and extract |mitogen_url|. +3. Modify ``ansible.cfg``: .. parsed-literal:: @@ -74,12 +72,16 @@ Installation per-run basis. Like ``mitogen_linear``, the ``mitogen_free`` strategy exists to mimic the ``free`` strategy. -5. If targets have a restrictive ``sudoers`` file, add a rule like: +4. If targets have a restrictive ``sudoers`` file, add a rule like: -:: + :: deploy = (ALL) NOPASSWD:/usr/bin/python -c* +5. Subscribe to the `mitogen-announce mailing list + `_ in order to stay up to + date with new releases and important bug fixes. + Demo ~~~~ @@ -123,6 +125,10 @@ Testimonials Noteworthy Differences ---------------------- +* Ansible 2.3-2.5 are supported along with Python 2.6, 2.7 or 3.6. Verify your + installation is running one of these versions by checking ``ansible + --version`` output. + * The Ansible ``raw`` action executes as a regular Mitogen connection, precluding its use for installing Python on a target. This will be addressed soon. From 50a1bf6f2257f68700ebebb05cce4c9920e0942a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 09:30:12 -0700 Subject: [PATCH 44/56] issue #300: temporary workaround for shutdown issue. Closes #300. --- mitogen/parent.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index de8d893d..a62128eb 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -531,7 +531,15 @@ class KqueuePoller(mitogen.core.Poller): def _control(self, fd, filters, flags): mitogen.core._vv and IOLOG.debug( '%r._control(%r, %r, %r)', self, fd, filters, flags) - self._changelist.append(select.kevent(fd, filters, flags)) + # TODO: at shutdown it is currently possible for KQ_EV_ADD/KQ_EV_DEL + # pairs to be pending after the associated file descriptor has already + # been closed. Fixing this requires maintaining extra state, or perhaps + # making fd closure the poller's responsibility. In the meantime, + # simply apply changes immediately. + # self._changelist.append(select.kevent(fd, filters, flags)) + changelist = [select.kevent(fd, filters, flags)] + events, _ = mitogen.core.io_op(self._kqueue.control, changelist, 0, 0) + assert not events def start_receive(self, fd, data=None): mitogen.core._vv and IOLOG.debug('%r.start_receive(%r, %r)', From 17c5bd26e1373a0fda89b6b1957a81479c2de32d Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 11:43:17 -0700 Subject: [PATCH 45/56] Update changelog. --- docs/changelog.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8d9a17ed..f55ab6d8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -41,6 +41,12 @@ Mitogen for Ansible network device connections, however it should still be possible to use device connections while Mitogen is active. +* `#300 `_: an exception could appear + on OS X during shutdown due to scheduling pending Kevent filter changes for + file descriptors that have already been closed before the IO loop resumes. As + a temporary workaround, Mitogen does not make use of Kevent's bulk change + feature. + * `#301 `_: variables like ``$HOME`` in the ``remote_tmp`` setting are evaluated correctly. From 441c60a3ca9a97681813a814a3e24608c55a6746 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 11:55:04 -0700 Subject: [PATCH 46/56] Fix GitHub vulnerability notification. --- dev_requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev_requirements.txt b/dev_requirements.txt index 7527504e..f093721b 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -4,7 +4,7 @@ coverage==4.5.1 Django==1.6.11 # Last version supporting 2.6. mock==2.0.0 pytz==2012d # Last 2.6-compat version. -paramiko==2.3.1 # Last 2.6-compat version. +paramiko==2.3.2 # Last 2.6-compat version. pytest-catchlog==1.2.2 pytest==3.1.2 PyYAML==3.11; python_version < '2.7' From 56943d3141c95a25b376d4dcfe01741d22f78bdf Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 18:17:27 -0700 Subject: [PATCH 47/56] issue #319: have cfsetraw() generate sensible flags. Attempting to fix issue on WSL. Closes #71 --- mitogen/parent.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index a62128eb..54412717 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -102,26 +102,23 @@ def is_immediate_child(msg, stream): def flags(names): """Return the result of ORing a set of (space separated) :py:mod:`termios` module constants together.""" - return sum(getattr(termios, name) for name in names.split()) + return sum(getattr(termios, name, 0) + for name in names.split()) def cfmakeraw(tflags): """Given a list returned by :py:func:`termios.tcgetattr`, return a list - that has been modified in the same manner as the `cfmakeraw()` C library - function.""" + modified in a manner similar to the `cfmakeraw()` C library function, but + additionally disabling local echo.""" + # BSD: https://github.com/freebsd/freebsd/blob/master/lib/libc/gen/termios.c#L162 + # Linux: https://github.com/lattera/glibc/blob/master/termios/cfmakeraw.c#L20 iflag, oflag, cflag, lflag, ispeed, ospeed, cc = tflags - iflag &= ~flags('IGNBRK BRKINT PARMRK ISTRIP INLCR IGNCR ICRNL IXON') - oflag &= ~flags('OPOST IXOFF') - lflag &= ~flags('ECHO ECHOE ECHONL ICANON ISIG IEXTEN') + iflag &= ~flags('IMAXBEL IXOFF INPCK BRKINT PARMRK ISTRIP INLCR ICRNL IXON IGNPAR') + iflag &= ~flags('IGNBRK BRKINT PARMRK') + oflag &= ~flags('OPOST') + lflag &= ~flags('ECHO ECHOE ECHOK ECHONL ICANON ISIG IEXTEN NOFLSH TOSTOP PENDIN') cflag &= ~flags('CSIZE PARENB') - cflag |= flags('CS8') - - # TODO: one or more of the above bit twiddles sets or omits a necessary - # flag. Forcing these fields to zero, as shown below, gets us what we want - # on Linux/OS X, but it is possibly broken on some other OS. - iflag = 0 - oflag = 0 - lflag = 0 + cflag |= flags('CS8 CREAD') return [iflag, oflag, cflag, lflag, ispeed, ospeed, cc] From 22bab87821a02ed8cb6b3eb4b52c766a8f5cfee7 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 27 Jul 2018 02:31:34 +0100 Subject: [PATCH 48/56] issue #319: avoid TCSAFLUSH flag on WSL. Closes #319. --- docs/changelog.rst | 4 ++++ mitogen/parent.py | 11 +++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index f55ab6d8..6726aa9f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -79,6 +79,10 @@ Core Library * `#307 `_: SSH login banner output containing the word 'password' is no longer confused for a password prompt. +* `#319 `_: SSH connections would + fail immediately on Windows Subsystem for Linux, due to use of `TCSAFLUSH` + with :func:`termios.tcsetattr`. The flag is omitted if WSL is detected. + * Debug logs containing command lines are printed with the minimal quoting and escaping required. diff --git a/mitogen/parent.py b/mitogen/parent.py index 54412717..4299d3cd 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -69,6 +69,8 @@ from mitogen.core import LOG from mitogen.core import IOLOG +IS_WSL = 'Microsoft' in os.uname()[2] + if mitogen.core.PY3: xrange = range @@ -125,10 +127,11 @@ def cfmakeraw(tflags): def disable_echo(fd): old = termios.tcgetattr(fd) new = cfmakeraw(old) - flags = ( - termios.TCSAFLUSH | - getattr(termios, 'TCSASOFT', 0) - ) + flags = getattr(termios, 'TCSASOFT', 0) + if not IS_WSL: + # issue #319: Windows Subsystem for Linux as of July 2018 throws EINVAL + # if TCSAFLUSH is specified. + flags |= termios.TCSAFLUSH termios.tcsetattr(fd, flags, new) From 3c55571fe2ade7268a7ae338f43ce33d07cff858 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 18:39:22 -0700 Subject: [PATCH 49/56] docs: update changelog --- docs/changelog.rst | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6726aa9f..fae40798 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -41,16 +41,10 @@ Mitogen for Ansible network device connections, however it should still be possible to use device connections while Mitogen is active. -* `#300 `_: an exception could appear - on OS X during shutdown due to scheduling pending Kevent filter changes for - file descriptors that have already been closed before the IO loop resumes. As - a temporary workaround, Mitogen does not make use of Kevent's bulk change - feature. - * `#301 `_: variables like ``$HOME`` in the ``remote_tmp`` setting are evaluated correctly. -* `#303 `_: the ``doas`` become method +* `#303 `_: the :ref:`doas` become method is supported. Contributed by `Mike Walker `_. @@ -69,10 +63,16 @@ Core Library ~~~~~~~~~~~~ * `#291 `_: the ``python_path`` - paramater may specify an argument vector prefix rather than a single string - program path. + parameter may specify an argument vector prefix rather than a string program + path. + +* `#300 `_: the broker could crash on + OS X during shutdown due to scheduled `kqueue + `_ filter changes for + descriptors that were closed before the IO loop resumes. As a temporary + workaround, kqueue's bulk change feature is not used. -* `#303 `_: the ``doas`` become method +* `#303 `_: the :ref:`doas` become method is now supported. Contributed by `Mike Walker `_. @@ -92,14 +92,18 @@ Thanks! Mitogen would not be possible without the support of users. A huge thanks for the bug reports and pull requests in this release contributed by -`Frances Albanese `_, -`Mark Janssen `_, +`Alex Russu `_, `Ayaz Ahmed Khan `_, `Colin McCarthy `_, `Dan Quackenbush `_, -`Alex Russu `_, -`Josh Smift `_, and -`Mike Walker `_. +`Duane Zamrok `_, +`Frances Albanese `_, +`Gonzalo Servat `_, +`Josh Smift `_, +`Mark Janssen `_, +`Mike Walker `_, +`Tawana Musewe `_, and +`Zach Swanson `_. v0.2.1 (2018-07-10) From 5d67ce77469af97a8d9144ba9e44fa4e83bb0e1c Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 18:39:38 -0700 Subject: [PATCH 50/56] service: service pool threads should respect _profile_hook. --- docs/changelog.rst | 3 +++ mitogen/service.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index fae40798..39e43ccf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -86,6 +86,9 @@ Core Library * Debug logs containing command lines are printed with the minimal quoting and escaping required. +* When :func:`mitogen.core.enable_profiling` is active, :mod:`mitogen.service` + threads are run under the profiling hook just like other threads. + Thanks! ~~~~~~~ diff --git a/mitogen/service.py b/mitogen/service.py index 8af02d0e..923ec04a 100644 --- a/mitogen/service.py +++ b/mitogen/service.py @@ -444,9 +444,11 @@ class Pool(object): self.add(service) self._threads = [] for x in range(size): + name = 'mitogen.service.Pool.%x.worker-%d' % (id(self), x,) thread = threading.Thread( - name='mitogen.service.Pool.%x.worker-%d' % (id(self), x,), - target=self._worker_main, + name=name, + target=mitogen.core._profile_hook, + args=(name, self._worker_main), ) thread.start() self._threads.append(thread) From 998762ab4ff83631dc58cf536ce3d97a4204270f Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 18:58:49 -0700 Subject: [PATCH 51/56] docs: update changelog. --- docs/changelog.rst | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 39e43ccf..b432cdc1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -53,9 +53,9 @@ Mitogen for Ansible tasks with the correct environment. * `#317 `_: respect the verbosity - setting when writing to to Ansible's ``log_path`` log file, if it is enabled. - Child log filtering was also incorrect, causing the master to needlessly wake - many times. This nets a 3.5% runtime improvement running against the local + setting when writing to Ansible's ``log_path``, if it is enabled. Child log + filtering was also incorrect, causing the master to needlessly wake many + times. This nets a 3.5% runtime improvement running against the local machine. @@ -83,11 +83,20 @@ Core Library fail immediately on Windows Subsystem for Linux, due to use of `TCSAFLUSH` with :func:`termios.tcsetattr`. The flag is omitted if WSL is detected. +* `#320 `_: The OS X poller + could spuriously wake up due to ignoring an error bit set on events returned + by the kernel, manifesting as a failure to read from an unrelated descriptor. + * Debug logs containing command lines are printed with the minimal quoting and escaping required. +* Standard IO forwarding accidentally configured the replacement ``stdout`` and + ``stderr`` write descriptors as non-blocking, causing subprocesses that + generate more output than kernel buffer space existed to throw errors. The + write ends are now configured as blocking. + * When :func:`mitogen.core.enable_profiling` is active, :mod:`mitogen.service` - threads are run under the profiling hook just like other threads. + threads are profiled just like other threads. Thanks! @@ -96,12 +105,14 @@ Thanks! Mitogen would not be possible without the support of users. A huge thanks for the bug reports and pull requests in this release contributed by `Alex Russu `_, +`Andy Freeland `_, `Ayaz Ahmed Khan `_, `Colin McCarthy `_, `Dan Quackenbush `_, `Duane Zamrok `_, `Frances Albanese `_, `Gonzalo Servat `_, +`Guy Knights `_, `Josh Smift `_, `Mark Janssen `_, `Mike Walker `_, From 0f512688757bf4f52046fcc2b4e31be632ab66f8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 19:14:56 -0700 Subject: [PATCH 52/56] Add GitHub issue template. --- .github/ISSUE_TEMPLATE.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE.md diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md new file mode 100644 index 00000000..b73eae64 --- /dev/null +++ b/.github/ISSUE_TEMPLATE.md @@ -0,0 +1,15 @@ + +Feel free to write an issue in your preferred format, however if in doubt, use +the following checklist as a guide for what to include. + +* Have you tried the latest master version from Git? +* Mention your host and target OS and versions +* Mention your host and target Python versions +* If reporting a performance issue, mention the number of targets and a rough + description of your workload (lots of copies, lots of tiny file edits, etc.) +* If reporting a crash or hang in Ansible, please rerun with -vvvv and include + the last 200 lines of output, along with a full copy of any traceback or + error text in the log. Beware "-vvvv" may include secret data! Edit as + necessary before posting. +* If reporting any kind of problem with Ansible, please include the Ansible + version along with output of "ansible-config dump --only-changed". From 21eda90a6761f6050bb3e874e050adbeebc12598 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 19:15:57 -0700 Subject: [PATCH 53/56] docs: reorder changelog --- docs/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b432cdc1..ab5444f8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -87,9 +87,6 @@ Core Library could spuriously wake up due to ignoring an error bit set on events returned by the kernel, manifesting as a failure to read from an unrelated descriptor. -* Debug logs containing command lines are printed with the minimal quoting and - escaping required. - * Standard IO forwarding accidentally configured the replacement ``stdout`` and ``stderr`` write descriptors as non-blocking, causing subprocesses that generate more output than kernel buffer space existed to throw errors. The @@ -98,6 +95,9 @@ Core Library * When :func:`mitogen.core.enable_profiling` is active, :mod:`mitogen.service` threads are profiled just like other threads. +* Debug logs containing command lines are printed with the minimal quoting and + escaping required. + Thanks! ~~~~~~~ From ebf721411c5633966b35878d9ade919febefd643 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 22:30:46 -0700 Subject: [PATCH 54/56] docs: add known issue --- docs/changelog.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ab5444f8..c6eeca92 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -196,6 +196,12 @@ Mitogen for Ansible for Message(..., 102, ...), my ID is ...* may be visible. These are due to a minor race while initializing logging and can be ignored. +* When running with ``-vvv``, log messages will be printed to the console + *after* the Ansible run completes, as connection multiplexer shutdown only + begins after Ansible exits. This is due to a lack of suitable shutdown hook + in Ansible, and is fairly harmless, albeit cosmetically annoying. A future + release may include a solution. + * Performance does not scale linearly with target count. This requires significant additional work, as major bottlenecks exist in the surrounding Ansible code. Performance-related bug reports for any scenario remain From 6813443d094cf00f2da098c9b331b56ac4ab4d90 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 23:36:41 -0700 Subject: [PATCH 55/56] docs: minor tweaks. --- docs/ansible.rst | 9 +++++---- docs/changelog.rst | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 37dfde1c..6ab6626a 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -79,8 +79,8 @@ Installation deploy = (ALL) NOPASSWD:/usr/bin/python -c* 5. Subscribe to the `mitogen-announce mailing list - `_ in order to stay up to - date with new releases and important bug fixes. + `_ to stay updated with new + releases and important bug fixes. Demo @@ -702,7 +702,7 @@ controller with ``-vvvv`` or higher. Although use of standard IO and the logging package on the target is forwarded to the controller, it is not possible to receive IO activity logs, as the -processs of receiving those logs would would itself generate IO activity. To +process of receiving those logs would would itself generate IO activity. To receive a complete trace of every process on every machine, file-based logging is necessary. File-based logging can be enabled by setting ``MITOGEN_ROUTER_DEBUG=1`` in your environment. @@ -711,7 +711,8 @@ When file-based logging is enabled, one file per context will be created on the local machine and every target machine, as ``/tmp/mitogen..log``. If you are experiencing a hang, ``MITOGEN_DUMP_THREAD_STACKS=1`` causes every -process to dump every thread stack into the logging framework every 5 seconds. +process on every machine to dump every thread stack into the logging framework +every 5 seconds. Getting Help diff --git a/docs/changelog.rst b/docs/changelog.rst index c6eeca92..d167ef14 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -110,7 +110,7 @@ the bug reports and pull requests in this release contributed by `Colin McCarthy `_, `Dan Quackenbush `_, `Duane Zamrok `_, -`Frances Albanese `_, +`falbanese `_, `Gonzalo Servat `_, `Guy Knights `_, `Josh Smift `_, From d64729e0418815038ecb56a68237a8d0527f49b6 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 26 Jul 2018 23:37:12 -0700 Subject: [PATCH 56/56] Bump version for release. --- docs/changelog.rst | 2 +- mitogen/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index d167ef14..2efdb035 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,7 +25,7 @@ Release Notes `_. -v0.2.2 (2018-07-??) +v0.2.2 (2018-07-26) ------------------- Mitogen for Ansible diff --git a/mitogen/__init__.py b/mitogen/__init__.py index 7044cdd5..3fc02433 100644 --- a/mitogen/__init__.py +++ b/mitogen/__init__.py @@ -33,7 +33,7 @@ be expected. On the slave, it is built dynamically during startup. #: Library version as a tuple. -__version__ = (0, 2, 1) +__version__ = (0, 2, 2) #: This is :data:`False` in slave contexts. Previously it was used to prevent