diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index aaac4882..26cce71d 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -79,24 +79,112 @@ for symbol in 'res_init', '__res_init': except AttributeError: pass -# For tasks running on Linux machines, with vanilla Ansible, edits to -# /etc/environment and ~/.pam_environment are reflected if become:true, due to -# sudo reinvoking pam_env. If multiplexing is disabled, then edits are also -# reflected with become:false. Rather than emulate existing semantics, simply -# always ensure edits are reflects for the next task. -try: - etc_env_st = os.stat('/etc/environment') -except OSError: - etc_env_st = None +iteritems = getattr(dict, 'iteritems', dict.items) +LOG = logging.getLogger(__name__) -try: - pam_env_st = os.stat(os.path.expanduser('~/.pam_environment')) -except OSError: - pam_env_st = None +class EnvironmentFileWatcher(object): + """ + Usually Ansible edits to /etc/environment and ~/.pam_environment are + reflected in subsequent tasks if become:true or SSH multiplexing is + disabled, due to sudo and/or SSH reinvoking pam_env. Rather than emulate + existing semantics, do our best to ensure edits are always reflected. -iteritems = getattr(dict, 'iteritems', dict.items) -LOG = logging.getLogger(__name__) + This can't perfectly replicate the existing behaviour, but it can safely + update and remove keys that appear to originate in `path`, and that do not + conflict with any existing environment key inherited from elsewhere. + + A more robust future approach may simply be to arrange for the persistent + interpreter to restart when a change is detected. + """ + def __init__(self, path): + self.path = os.path.expanduser(path) + #: Inode data at time of last check. + self._st = self._stat() + #: List of inherited keys appearing to originated from this file. + self._keys = [key for key, value in self._load() + if value == os.environ.get(key)] + LOG.debug('%r installed; existing keys: %r', self, self._keys) + + def __repr__(self): + return 'EnvironmentFileWatcher(%r)' % (self.path,) + + def _stat(self): + try: + return os.stat(self.path) + except OSError: + return None + + def _load(self): + try: + with open(self.path, 'r') as fp: + return list(self._parse(fp)) + except IOError: + return [] + + def _parse(self, fp): + """ + linux-pam-1.3.1/modules/pam_env/pam_env.c#L207 + """ + for line in fp: + # ' #export foo=some var ' -> ['#export', 'foo=some var '] + bits = shlex.split(line, comments=True) + if (not bits) or bits[0].startswith('#'): + continue + + if bits[0] == 'export': + bits.pop(0) + + key, sep, value = (' '.join(bits)).partition('=') + if key and sep: + yield key, value + + def _on_file_changed(self): + LOG.debug('%r: file changed, reloading', self) + for key, value in self._load(): + if key in os.environ: + LOG.debug('%r: existing key %r=%r exists, not setting %r', + self, key, os.environ[key], value) + else: + LOG.debug('%r: setting key %r to %r', self, key, value) + self._keys.append(key) + os.environ[key] = value + + def _remove_existing(self): + """ + When a change is detected, remove keys that existed in the old file. + """ + for key in self._keys: + if key in os.environ: + LOG.debug('%r: removing old key %r', self, key) + del os.environ[key] + self._keys = [] + + def check(self): + """ + Compare the :func:`os.stat` for the pam_env style environmnt file + `path` with the previous result `old_st`, which may be :data:`None` if + the previous stat attempt failed. Reload its contents if the file has + changed or appeared since last attempt. + + :returns: + New :func:`os.stat` result. The new call to :func:`reload_env` should + pass it as the value of `old_st`. + """ + st = self._stat() + if self._st == st: + return + + self._st = st + self._remove_existing() + + if st is None: + LOG.debug('%r: file has disappeared', self) + else: + self._on_file_changed() + +_pam_env_watcher = EnvironmentFileWatcher('~/.pam_environment') +_etc_env_watcher = EnvironmentFileWatcher('/etc/environment') def utf8(s): @@ -121,54 +209,6 @@ def reopen_readonly(fp): os.close(fd) -def parse_env(fp): - """ - Parse /etc/environ using roughly the same syntax as pam_env. - """ - # https://github.com/linux-pam/linux-pam/blob/v1.3.1/modules/pam_env/pam_env.c#L207 - for line in fp: - # ' #export foo=some var ' -> ['#export', 'foo=some var '] - bits = shlex.split(line, comments=True) - if not bits: - continue - - if bits[0] == 'export': - bits.pop(0) - - key, sep, value = (' '.join(bits)).partition('=') - if sep: - os.environ[key] = value - - -def reload_env(old_st, path): - """ - Compare the :func:`os.stat` for the pam_env style environmnt file `path` - with the previous result `old_st`, which may be :data:`None` if the - previous stat attempt failed. Reload its contents if the file has changed - or appeared since last attempt. - - :returns: - New :func:`os.stat` result. The new call to :func:`reload_env` should - pass it as the value of `old_st`. - """ - try: - path = os.path.expanduser(path) - st = os.stat(path) - except OSError: - return None - - if old_st == st: - return old_st - if st is None: - LOG.debug('reload_env(%r): file has disappeared', path) - return st - - LOG.debug('reload_env(%r): file has changed or appeared, reloading', path) - with open(path) as fp: - parse_env(fp) - return st - - class Runner(object): """ Ansible module runner. After instantiation (with kwargs supplied by the @@ -219,30 +259,29 @@ class Runner(object): from the parent, as :meth:`run` may detach prior to beginning execution. The base implementation simply prepares the environment. """ + self._setup_cwd() + self._setup_environ() + + def _setup_cwd(self): + """ + For situations like sudo to a non-privileged account, CWD could be + $HOME of the old account, which could have mode go=, which means it is + impossible to restore the old directory, so don't even try. + """ if self.cwd: - # For situations like sudo to another non-privileged account, the - # CWD could be $HOME of the old account, which could have mode go=, - # which means it is impossible to restore the old directory, so - # don't even bother. os.chdir(self.cwd) - env = dict(self.extra_env or {}) - if self.env: - env.update(self.env) - self._setup_environ() - self._env = TemporaryEnvironment(env) def _setup_environ(self): """ - Ensure /etc/environment and ~/.pam_environment are reloaded if their - content appears to differ since execution of the previous task. This - must happen before TemporaryEnvironment is installed, to ensure changes - persist across tasks. + Apply changes from /etc/environment files before creating a + TemporaryEnvironment to snapshot environment state prior to module run. """ - global etc_env_st - etc_env_st = reload_env(etc_env_st, '/etc/environment') - - global pam_env_st - pam_env_st = reload_env(pam_env_st, '~/.pam_environment') + _pam_env_watcher.check() + _etc_env_watcher.check() + env = dict(self.extra_env or {}) + if self.env: + env.update(self.env) + self._env = TemporaryEnvironment(env) def revert(self): """ diff --git a/docs/ansible.rst b/docs/ansible.rst index a6130873..d66743f4 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -401,6 +401,61 @@ this precisely, to avoid breaking playbooks that expect text to appear in specific variables with a particular linefeed style. +.. _ansible_process_env: + +Process Environment Emulation +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Since Ansible discards processes after each module invocation, follow-up tasks +often (but not always) receive a new environment that will usually include +changes made by previous tasks. As such modifications are common, for +compatibility the extension emulates the existing behaviour as closely as +possible. + +Some scenarios exist where emulation is impossible, for example, applying +``nsswitch.conf`` changes when ``nscd`` is not in use. If future scenarios +appear that cannot be solved through emulation, the extension will be updated +to automatically restart affected interpreters instead. + + +DNS Resolution +^^^^^^^^^^^^^^ + +Modifications to ``/etc/resolv.conf`` cause the glibc resolver configuration to +be reloaded via `res_init(3) `_. This +isn't necessary on some Linux distributions carrying glibc patches to +automatically check ``/etc/resolv.conf`` periodically, however it is necessary +on at least Debian and the BSD derivatives. + + +``/etc/environment`` +^^^^^^^^^^^^^^^^^^^^ + +When ``become: true`` is active or SSH multiplexing is disabled, modifications +by previous tasks to ``/etc/environment`` and ``$HOME/.pam_environment`` are +reflected, since the content of those files is reapplied by `PAM +`_ via `pam_env` +on each authentication of ``sudo`` or ``sshd``. + +Both files are monitored for changes, and changes are applied where it appears +safe to do so: + +* New keys are added if they did not otherwise exist in the inherited + environment, or previously had the same value as found in the file before it + changed. + +* Given a key (such as ``http_proxy``) added to the file where no such key + exists in the environment, the key will be added. + +* Given a key (such as ``PATH``) where an existing environment key exists with + a different value, the update or deletion will be ignored, as it is likely + the key was overridden elsewhere after `pam_env` ran, such as by + ``/etc/profile``. + +* Given a key removed from the file that had the same value as the existing + environment key, the key will be removed. + + How Modules Execute ~~~~~~~~~~~~~~~~~~~ diff --git a/docs/changelog.rst b/docs/changelog.rst index 9a78790b..0c1df179 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -56,11 +56,10 @@ Mitogen for Ansible * `#332 `_: support a new :data:`sys.excepthook`-based module exit mechanism added in Ansible 2.6. -* `#338 `_: compatibility: due to - Ansible's implementation, changes to ``/etc/environment`` made by a task are - reflected in the runtime environment of subsequent tasks, but only if those - tasks set ``become: true``, or if SSH multiplexing is disabled. Changes to - ``/etc/environment`` are now monitored and always reflected. +* `#338 `_: compatibility: changes to + ``/etc/environment`` and ``~/.pam_environment`` made by a task are reflected + in the runtime environment of subsequent tasks. See + :ref:`ansible_process_env` for a complete description. * Runs with many targets executed the module dependency scanner redundantly due to missing synchronization, causing significant wasted computation in the diff --git a/tests/ansible/integration/runner/etc_environment.yml b/tests/ansible/integration/runner/etc_environment.yml index 68ec980a..0037698a 100644 --- a/tests/ansible/integration/runner/etc_environment.yml +++ b/tests/ansible/integration/runner/etc_environment.yml @@ -13,7 +13,7 @@ path: ~/.pam_environment state: absent - - shell: echo $MAGIC_NEW_ENV + - shell: echo $MAGIC_PAM_ENV register: echo - assert: @@ -22,9 +22,9 @@ - copy: dest: ~/.pam_environment content: | - MAGIC_NEW_ENV=321 + MAGIC_PAM_ENV=321 - - shell: echo $MAGIC_NEW_ENV + - shell: echo $MAGIC_PAM_ENV register: echo - assert: @@ -34,6 +34,12 @@ path: ~/.pam_environment state: absent + - shell: echo $MAGIC_PAM_ENV + register: echo + + - assert: + that: echo.stdout == "" + # /etc/environment - meta: end_play @@ -56,7 +62,7 @@ MAGIC_ETC_ENV=555 become: true - - shell: echo $MAGIC_ENV_ENV + - shell: echo $MAGIC_ETC_ENV register: echo - assert: @@ -66,3 +72,9 @@ path: /etc/environment state: absent become: true + + - shell: echo $MAGIC_ETC_ENV + register: echo + + - assert: + that: echo.stdout == ""