diff --git a/changelogs/fragments/297_firewalld_exclusive_options_handling.yml b/changelogs/fragments/297_firewalld_exclusive_options_handling.yml new file mode 100644 index 0000000..4727000 --- /dev/null +++ b/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). diff --git a/plugins/modules/firewalld.py b/plugins/modules/firewalld.py index cd7fae2..39a3b18 100644 --- a/plugins/modules/firewalld.py +++ b/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, diff --git a/tests/integration/targets/firewalld/tasks/source_test_cases.yml b/tests/integration/targets/firewalld/tasks/source_test_cases.yml index f7c4f00..172a47e 100644 --- a/tests/integration/targets/firewalld/tasks/source_test_cases.yml +++ b/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'"