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 <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>

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/5755>`__
+    (https://github.com/ansible-collections/community.general/pull/5765).

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
 
 

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
 
 

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()

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