ansible-runner icon indicating copy to clipboard operation
ansible-runner copied to clipboard

Add validation to prevent unexpected types in environment variables

Open AlanCoding opened this issue 2 years ago • 2 comments

Demo in python console:

>>> import ansible_runner.interface
>>> ansible_runner.interface.run(private_data_dir='demo', playbook='test.yml')
[WARNING]: You are running the development version of Ansible. You should only
run Ansible from "devel" if you are modifying the Ansible engine, or trying out
features under development. This is a rapidly changing source of code and can
become unstable at any point.

PLAY [all] *********************************************************************

TASK [Gathering Facts] *********************************************************
ok: [localhost]

TASK [debug] *******************************************************************
ok: [localhost] => {
    "msg": "Test!"
}

PLAY RECAP *********************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
<ansible_runner.runner.Runner object at 0x7f83863bfbe0>
>>> ansible_runner.interface.run(private_data_dir='demo', playbook='test.yml', envvars={'ALAN': True})

There it hangs. This is experiencing the traceback pasted in https://github.com/ansible/awx/pull/11993, but doing so silently.

The fix is to use envvars={'ALAN': 'True'} instead.

However, the poor error handling incurs significant cost in terms of people tracking down the root cause. Runner should avoid this by giving a descriptive error right away.

It would also be ideal to figure out why tracebacks are not getting surfaced, which may be a pexpect upstream issue.

AlanCoding avatar Apr 01 '22 17:04 AlanCoding

Agreed. Definitely something we should investigate.

Shrews avatar Apr 05 '22 13:04 Shrews

We already have an upstream issue, and here it is: https://github.com/pexpect/pexpect/issues/512

There is a proposed fix at https://github.com/pexpect/ptyprocess/pull/47, which was not even in dispute, ending at

Whichever one of us finds time to get to this first

Predictably, it has been stuck there for over 2 years.

AlanCoding avatar Apr 08 '22 14:04 AlanCoding