From dc531b183d2f1b48378ddfd3dd976e067bc02eb6 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Sat, 7 Jan 2023 22:21:13 +1300 Subject: [PATCH] ModuleHelper - lax handling of conflicting output (#5765) * ModuleHelper - lax handling of conflicting output * add changelog fragment * only create _var when really needed * adjust changelog * Update changelogs/fragments/5765-mh-lax-output-conflict.yml Co-authored-by: Felix Fontein Co-authored-by: Felix Fontein --- .../fragments/5765-mh-lax-output-conflict.yml | 9 ++++++ plugins/module_utils/mh/deco.py | 17 +++++++++-- plugins/module_utils/mh/module_helper.py | 5 ---- .../targets/module_helper/library/msimple.py | 5 ++-- .../tasks/msimple_output_conflict.yml | 29 +++++++++++++++++-- 5 files changed, 52 insertions(+), 13 deletions(-) create mode 100644 changelogs/fragments/5765-mh-lax-output-conflict.yml diff --git a/changelogs/fragments/5765-mh-lax-output-conflict.yml b/changelogs/fragments/5765-mh-lax-output-conflict.yml new file mode 100644 index 0000000000..2e8cc292bd --- /dev/null +++ b/changelogs/fragments/5765-mh-lax-output-conflict.yml @@ -0,0 +1,9 @@ +breaking_changes: + - > + ModuleHelper module utils - when the module sets output variables named ``msg``, ``exception``, ``output``, ``vars``, or ``changed``, + the actual output will prefix those names with ``_`` (underscore symbol) only when they clash with output variables generated by ModuleHelper + itself, which only occurs when handling exceptions. Please note that this breaking + change does not require a new major release since before this release, it was not possible + to add such variables to the output + `due to a bug `__ + (https://github.com/ansible-collections/community.general/pull/5765). diff --git a/plugins/module_utils/mh/deco.py b/plugins/module_utils/mh/deco.py index 3073e4e9e7..5138b212c7 100644 --- a/plugins/module_utils/mh/deco.py +++ b/plugins/module_utils/mh/deco.py @@ -37,8 +37,17 @@ def cause_changes(on_success=None, on_failure=None): def module_fails_on_exception(func): + conflict_list = ('msg', 'exception', 'output', 'vars', 'changed') + @wraps(func) def wrapper(self, *args, **kwargs): + def fix_var_conflicts(output): + result = dict([ + (k if k not in conflict_list else "_" + k, v) + for k, v in output.items() + ]) + return result + try: func(self, *args, **kwargs) except SystemExit: @@ -46,12 +55,16 @@ def module_fails_on_exception(func): except ModuleHelperException as e: if e.update_output: self.update_output(e.update_output) + # patchy solution to resolve conflict with output variables + output = fix_var_conflicts(self.output) self.module.fail_json(msg=e.msg, exception=traceback.format_exc(), - output=self.output, vars=self.vars.output(), **self.output) + output=self.output, vars=self.vars.output(), **output) except Exception as e: + # patchy solution to resolve conflict with output variables + output = fix_var_conflicts(self.output) msg = "Module failed with exception: {0}".format(str(e).strip()) self.module.fail_json(msg=msg, exception=traceback.format_exc(), - output=self.output, vars=self.vars.output(), **self.output) + output=self.output, vars=self.vars.output(), **output) return wrapper diff --git a/plugins/module_utils/mh/module_helper.py b/plugins/module_utils/mh/module_helper.py index 51696b6ff6..6813b5454b 100644 --- a/plugins/module_utils/mh/module_helper.py +++ b/plugins/module_utils/mh/module_helper.py @@ -18,7 +18,6 @@ from ansible_collections.community.general.plugins.module_utils.mh.mixins.deprec class ModuleHelper(DeprecateAttrsMixin, VarsMixin, DependencyMixin, ModuleHelperBase): - _output_conflict_list = ('msg', 'exception', 'output', 'vars', 'changed') facts_name = None output_params = () diff_params = () @@ -60,10 +59,6 @@ class ModuleHelper(DeprecateAttrsMixin, VarsMixin, DependencyMixin, ModuleHelper vars_diff = self.vars.diff() or {} result['diff'] = dict_merge(dict(diff), vars_diff) - for varname in list(result): - if varname in self._output_conflict_list: - result["_" + varname] = result[varname] - del result[varname] return result diff --git a/tests/integration/targets/module_helper/library/msimple.py b/tests/integration/targets/module_helper/library/msimple.py index 3729b6c702..096e515247 100644 --- a/tests/integration/targets/module_helper/library/msimple.py +++ b/tests/integration/targets/module_helper/library/msimple.py @@ -57,6 +57,8 @@ class MSimple(ModuleHelper): self.vars['c'] = str(self.vars.c) * 3 def __run__(self): + if self.vars.m: + self.vars.msg = self.vars.m if self.vars.a >= 100: raise Exception("a >= 100") if self.vars.c == "abc change": @@ -66,9 +68,6 @@ class MSimple(ModuleHelper): self.vars['c'] = str(self.vars.c) * 2 self.process_a3_bc() - if self.vars.m: - self.vars.msg = self.vars.m - def main(): msimple = MSimple() diff --git a/tests/integration/targets/module_helper/tasks/msimple_output_conflict.yml b/tests/integration/targets/module_helper/tasks/msimple_output_conflict.yml index 21ffd37d35..55e0a06eca 100644 --- a/tests/integration/targets/module_helper/tasks/msimple_output_conflict.yml +++ b/tests/integration/targets/module_helper/tasks/msimple_output_conflict.yml @@ -2,7 +2,7 @@ # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later -- name: test msimple (set a=80) +- name: test msimple conflict output (set a=80) msimple: a: 80 register: simple1 @@ -15,7 +15,7 @@ - simple1 is not changed - simple1.value is none -- name: test msimple 2 +- name: test msimple conflict output 2 msimple: a: 80 m: a message in a bottle @@ -28,4 +28,27 @@ - simple1.abc == "abc" - simple1 is not changed - simple1.value is none - - 'simple2._msg == "a message in a bottle"' + - > + "_msg" not in simple2 + - > + simple2.msg == "a message in a bottle" + +- name: test msimple 3 + msimple: + a: 101 + m: a message in a bottle + ignore_errors: yes + register: simple3 + +- name: assert simple3 + assert: + that: + - simple3.a == 101 + - > + simple3.msg == "Module failed with exception: a >= 100" + - > + simple3._msg == "a message in a bottle" + - simple3.abc == "abc" + - simple3 is failed + - simple3 is not changed + - simple3.value is none