From a612137098f4f0ad375194f70c0d47c25fe79719 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 14 Feb 2018 18:44:58 -0800 Subject: [PATCH] Final fix for #35666 Previous PR (#36143) was merged prematurely. --- lib/ansible/plugins/shell/__init__.py | 33 ++++++++++++------- lib/ansible/plugins/shell/powershell.py | 7 ---- .../module_docs_fragments/shell_common.py | 2 +- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/ansible/plugins/shell/__init__.py b/lib/ansible/plugins/shell/__init__.py index 0e505983fe..80d90a34ae 100644 --- a/lib/ansible/plugins/shell/__init__.py +++ b/lib/ansible/plugins/shell/__init__.py @@ -47,6 +47,20 @@ class ShellBase(AnsiblePlugin): self.tempdir = None + def _normalize_system_temps(self): + # Normalize the temp directory strings. We don't use expanduser/expandvars because those + # can vary between remote user and become user. Therefore the safest practice will be for + # this to always be specified as full paths) + normalized_paths = [d.rstrip('/') for d in self.get_option('system_temps')] + + # Make sure all system_temps are absolute otherwise they'd be relative to the login dir + # which is almost certainly going to fail in a cornercase. + if not all(os.path.isabs(d) for d in normalized_paths): + raise AnsibleError('The configured system_temps contains a relative path: {0}. All' + ' system_temps must be absolute'.format(to_native(normalized_paths))) + + self.set_option('system_temps', normalized_paths) + def set_options(self, task_keys=None, var_options=None, direct=None): super(ShellBase, self).set_options(task_keys=task_keys, var_options=var_options, direct=direct) @@ -54,18 +68,13 @@ class ShellBase(AnsiblePlugin): # set env self.env.update(self.get_option('environment')) - # Normalize the temp directory strings. We don't use expanduser/expandvars because those - # can vary between remote user and become user. Therefore the safest practice will be for - # this to always be specified as full paths) - normalized_system_temps = [d.rstrip('/') for d in self.get_option('system_temps')] - - # Make sure all system_temps are absolute otherwise they'd be relative to the login dir - # which is almost certainly going to fail in a cornercase. - if not all(os.path.isabs(d) for d in normalized_system_temps): - raise AnsibleError('The configured system_temps contains a relative path: {0}. All' - ' system_temps must be absolute'.format(to_native(normalized_system_temps))) - - self.set_option('system_temps', normalized_system_temps) + # We can remove the try: except in the future when we make ShellBase a proper subset of + # *all* shells. Right now powershell and third party shells which do not use the + # shell_common documentation fragment (and so do not have system_temps) will fail + try: + self._normalize_system_temps() + except AnsibleError: + pass def env_prefix(self, **kwargs): return ' '.join(['%s=%s' % (k, shlex_quote(text_type(v))) for k, v in kwargs.items()]) diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index 9613ce9edd..22b9016d0a 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -1823,13 +1823,6 @@ class ShellModule(ShellBase): # TODO: add binary module support - def set_options(self, task_keys=None, var_options=None, direct=None): - - super(ShellModule, self).set_options(task_keys=task_keys, var_options=var_options, direct=direct) - - # set env - self.env.update(self.get_option('environment')) - def assert_safe_env_key(self, key): if not self.safe_envkey.match(key): raise AnsibleError("Invalid PowerShell environment key: %s" % key) diff --git a/lib/ansible/utils/module_docs_fragments/shell_common.py b/lib/ansible/utils/module_docs_fragments/shell_common.py index 250d6c06d9..19d5c727fc 100644 --- a/lib/ansible/utils/module_docs_fragments/shell_common.py +++ b/lib/ansible/utils/module_docs_fragments/shell_common.py @@ -19,7 +19,7 @@ options: - name: ansible_remote_tmp system_temps: description: - - List of valid system temporary directories for Ansible to choose when it cannot use ``remote_temp``, normally due to permission issues. + - List of valid system temporary directories for Ansible to choose when it cannot use ``remote_temp``, normally due to permission issues. These must be world readable, writable, and executable. default: [ /var/tmp, /tmp ] type: list env: [{name: ANSIBLE_SYSTEM_TMPS}]