Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3006.x] module.run: fix result detection from returned dict #67115

Open
wants to merge 1 commit into
base: 3006.x
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/65842.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed result detection of module.run from returned dict
38 changes: 18 additions & 20 deletions salt/states/module.py
Original file line number Diff line number Diff line change
@@ -452,7 +452,7 @@ def _run(**kwargs):
func_ret = _call_function(
_func, returner=kwargs.get("returner"), func_args=kwargs.get(func)
)
if not _get_result(func_ret, ret["changes"].get("ret", {})):
if not _get_result(func_ret):
if isinstance(func_ret, dict):
failures.append(
"'{}' failed: {}".format(
@@ -658,31 +658,29 @@ def _legacy_run(name, **kwargs):
if kwargs["returner"] in returners:
returners[kwargs["returner"]](ret_ret)
ret["comment"] = f"Module function {name} executed"
ret["result"] = _get_result(mret, ret["changes"])
ret["result"] = _get_result(mret)

return ret


def _get_result(func_ret, changes):
def _get_result(func_ret):
res = True
# if mret is a dict and there is retcode and its non-zero
if isinstance(func_ret, dict) and func_ret.get("retcode", 0) != 0:
res = False
# if its a boolean, return that as the result
elif isinstance(func_ret, bool):
# if mret a boolean, return that as the result
if isinstance(func_ret, bool):
res = func_ret
else:
changes_ret = changes.get("ret", {})
if isinstance(changes_ret, dict):
if isinstance(changes_ret.get("result", {}), bool):
res = changes_ret.get("result", {})
elif changes_ret.get("retcode", 0) != 0:
res = False
# Explore dict in depth to determine if there is a
# 'result' key set to False which sets the global
# state result.
else:
res = _get_dict_result(changes_ret)
# if mret is a dict, check if certain keys exist
elif isinstance(func_ret, dict):
# if return key exists and is boolean, return that as the result
if isinstance(func_ret.get("result", {}), bool):
res = func_ret.get("result", {})
# if retcode exists and it is non-zero, return False
elif func_ret.get("retcode", 0) != 0:
res = False
# Explore dict in depth to determine if there is a
# 'result' key set to False which sets the global
# state result.
else:
res = _get_dict_result(func_ret)

return res

17 changes: 17 additions & 0 deletions tests/unit/states/test_module.py
Original file line number Diff line number Diff line change
@@ -421,6 +421,23 @@ def test_module_run_missing_arg(self):
self.assertIn("world", ret["comment"])
self.assertIn("hello", ret["comment"])

def test_module_run_ret_result(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this file would be migrated to tests/pytests/.

with patch.dict(module.__salt__, {CMD: _mocked_none_return}), patch.dict(
module.__opts__, {"use_superseded": ["module.run"]}
):
for val, res in [
(True, True),
(False, False),
({"result": True}, True),
({"result": False}, False),
({"retcode": 0}, True),
({"retcode": 1}, False),
({"bla": {"result": True}}, True),
({"bla": {"result": False}}, False),
]:
ret = module.run(**{CMD: [{"ret": val}]})
self.assertEqual(ret["result"], res)

def test_call_function_named_args(self):
"""
Test _call_function routine when params are named. Their position ordering should not matter.