ansible_shell_executable is not used by default by module_utils/basic.py AnsibleModule run_command #24169
Comments
ansibot
added
affects_2.3
|
We don't pass the executable information to modules, not all of them execute commands. For those that do, most use run_command w/o shell, the fact gathering module is one of the few exceptions that relies on shell semantics, but it does not explicitly set it, it relies on python's |
|
Hi,
On 01/05/17 18:27, Brian Coca wrote:
We don't pass the executable information to modules, not all of them
execute commands.
Correct and understood, but the question is if it's the right decision.
For those that do, most use run_command w/o shell, the fact gathering
module is one of the few exceptions that relies on shell semantics, but
it does not explicitly set it, it relies on python's |Popen| and passes
|shell=True| which leads to the execution of |/bin/sh| (which is
expected to be present on all POSIX platforms).
But there is a reason for the ansible_shell_executable parameter? And if
it's not passed everywhere, it's kind of a moot parameter, or what
remains as use case: either it's needed everywhere or it's needed nowhere?
Thanks, Eric
|
|
I was explaining the current state, I did not close this issue as we might want to revisit the current state of affairs. The parameter is used mostly for 'controller side' construction of commands, we remotely create/copy/execute/remove files (which includes modules). Looking at the Popen code it actually hard codes it I'm thinking the simplest solution is for run_command to use |
|
Hi, I like the idea of using the underlying SHELL environment variable, it makes a lot of sense and should work. I haven't yet tested anything but I think that your solution keeps people from calling run_command with the - def run_command(self, args, check_rc=False, close_fds=True, executable=None, data=None, binary_data=False, path_prefix=None, cwd=None,
+ def run_command(self, args, check_rc=False, close_fds=True, executable=os.environ.get('SHELL'), data=None, binary_data=False, path_prefix=None, cwd=None,
[...]
- :kw executable: See documentation for subprocess.Popen(). Default None
+ :kw executable: See documentation for subprocess.Popen(). Default content of $SHELL or NoneAs said, not tested, and my python-skills are limited, but I think you get the idea. |
|
udpated the PR to account for existing |
|
We had to revert this fix for 2.4.1 as it was changing the value of output, thus breaking existing user's playbooks. We're looking into a new fix for 2.5.x: #31361 |
ansibot
added
the
support:core
ISSUE TYPE
COMPONENT NAME
core ?
ANSIBLE VERSION
CONFIGURATION
OS / ENVIRONMENT
N/A
SUMMARY
In order to manage "exotic" platforms (embedded Linux, Android, etc), one might need to set android_shell_executable in the inventory. This inventory variable isn't used as default by all modules relying on shell (setup, shell, and possibly others), so that they fail by default with an unclear 'file not found' error.
This is due to module_utils/basic.py AnsibleModule.run_command having the parameter
executableempty by default, defaulting to/bin/sh, whereas it should IMHO default to the shell set by the user.One can work around this issue where modules like
shelloffer the possibility to explicitly overwrite the shell used, but not with modules likesetup, which don't offer this possibility.STEPS TO REPRODUCE
/bin/shfrom your managed hostansible -i my.inv -m setup managedhostoransible -i my.inv -m shell -a id managedhostand they fail.ansible -i my.inv -m shell -a 'executable=/opt/bin/sh id' managedhostand it succeedsEXPECTED RESULTS
The modules called use ansible_shell_executable and succeed.
ACTUAL RESULTS
The modules fail with a file not found error, which relates obscurely to /bin/sh not being found.
The text was updated successfully, but these errors were encountered: