Merge pull request #297 from saito-hideki/issue/255

Refine the handling of exclusive options

SUMMARY
Refine the handling of exclusive options using mutually_exclusive:

Fixes #255

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ansible.posix.firewalld

ADDITIONAL INFORMATION
None

Reviewed-by: Adam Miller <maxamillion@fedoraproject.org>
Reviewed-by: None <None>

changelogs/fragments/297_firewalld_exclusive_options_handling.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+- firewalld - Refine the handling of exclusive options (https://github.com/ansible-collections/ansible.posix/issues/255).

plugins/modules/firewalld.py

@@ -760,6 +760,10 @@ def main():
             target=('zone',),
             source=('permanent',),
         ),
+        mutually_exclusive=[
+            ['icmp_block', 'icmp_block_inversion', 'service', 'port', 'port_forward', 'rich_rule',
+             'interface', 'masquerade', 'source', 'target']
+        ],
     )
 
     permanent = module.params['permanent']
@@ -816,33 +820,11 @@ def main():
         if 'toaddr' in port_forward:
             port_forward_toaddr = port_forward['toaddr']
 
-    modification_count = 0
-    if icmp_block is not None:
-        modification_count += 1
-    if icmp_block_inversion is not None:
-        modification_count += 1
-    if service is not None:
-        modification_count += 1
-    if port is not None:
-        modification_count += 1
-    if port_forward is not None:
-        modification_count += 1
-    if rich_rule is not None:
-        modification_count += 1
-    if interface is not None:
-        modification_count += 1
-    if masquerade is not None:
-        modification_count += 1
-    if source is not None:
-        modification_count += 1
-    if target is not None:
-        modification_count += 1
-
-    if modification_count > 1:
-        module.fail_json(
-            msg='can only operate on port, service, rich_rule, masquerade, icmp_block, icmp_block_inversion, interface or source at once'
-        )
-    elif (modification_count > 0) and (desired_state in ['absent', 'present']) and (target is None):
+    modification = False
+    if any([icmp_block, icmp_block_inversion, service, port, port_forward, rich_rule,
+            interface, masquerade, source, target]):
+        modification = True
+    if modification and desired_state in ['absent', 'present'] and target is None:
         module.fail_json(
             msg='absent and present state can only be used in zone level operations'
         )
@@ -1027,7 +1009,7 @@ def main():
         msgs = msgs + transaction_msgs
 
     ''' If there are no changes within the zone we are operating on the zone itself '''
-    if modification_count == 0 and desired_state in ['absent', 'present']:
+    if not modification and desired_state in ['absent', 'present']:
 
         transaction = ZoneTransaction(
             module,

tests/integration/targets/firewalld/tasks/source_test_cases.yml

@@ -82,4 +82,4 @@
   assert:
     that:
     - result is not changed
-    - "result.msg == 'can only operate on port, service, rich_rule, masquerade, icmp_block, icmp_block_inversion, interface or source at once'"
+    - "result.msg == 'parameters are mutually exclusive: icmp_block|icmp_block_inversion|service|port|port_forward|rich_rule|interface|masquerade|source|target'"