Discussing "Fail module - all" PR #8410

Hi,

per your comments on github, here to discuss this pr https://github.com/ansible/ansible/pull/8410

This appears to insert a situation where returning the string ‘all’ in a module will fail on all hosts.
This has some possible security usage, in that one host could choose to fail a playbook for all hosts.
Plus I’m not sure it should be called “all” in the return. Ideally I think we need a way for just the fail module to leverage this.

actually i thought of this while doing the pr, thats why it’s coded to specifically work with the fail module only.

what happens here is :

1- if you pass the “all” parameter to the fail module it will add a new return data parameter called “fatal”

  • in 'lib/ansible/runner/action_plugins/fail.py"

if "all" in args: result = dict(failed=True, msg=args['msg'], fatal=True)

2- this new return data parameter “fatal” is then checked in playbook init while running the task
in “lib/ansible/playbook/init.py” _run_task
for host, result in contacted.iteritems(): if task.module_name == 'fail' and 'fatal' in result and 'msg' in result: hosts_remaining = False return hosts_remaining

so as you can see it explicitly checks that the fatal in result is coming from the “fail” module, that way it will not trigger that behavior if by any chance fatal was added to return data by any other module, not really happy about the way this is accomplished, so open to better suggestions here? in particular not happy that we have to reference/check the task.module_name in playbook run_taks.

however this does answer your question about it failing for modules other than “fail”

Amr

Hi Amr!

Thanks for the clarification, I think I misread the code that was reading the result and the explicit check for the module name seems quite reasonable to me.

I’d definitely add the “version_added” attribute to the new parameter so the docs can indicate what version this is added in (probably will be 1.8), which is minor.

Much appreciated!