diff --git a/changelogs/fragments/404_firewalld_masquerade_icmp_block_inversion_bugfixes.yml b/changelogs/fragments/404_firewalld_masquerade_icmp_block_inversion_bugfixes.yml new file mode 100644 index 0000000..144b80e --- /dev/null +++ b/changelogs/fragments/404_firewalld_masquerade_icmp_block_inversion_bugfixes.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - firewalld - Consider value of masquerade and icmp_block_inversion parameters when a boolean like value is passed diff --git a/plugins/modules/firewalld.py b/plugins/modules/firewalld.py index 6cd46ca..3546749 100644 --- a/plugins/modules/firewalld.py +++ b/plugins/modules/firewalld.py @@ -907,12 +907,21 @@ def main(): msgs.append("Changed icmp-block %s to %s" % (icmp_block, desired_state)) if icmp_block_inversion is not None: + # Type of icmp_block_inversion will be changed to boolean in a future release. + icmp_block_inversion_status = True + try: + icmp_block_inversion_status = boolean(icmp_block_inversion, True) + except TypeError: + module.warn('The value of the icmp_block_inversion option is "%s". ' + 'The type of the option will be changed from string to boolean in a future release. ' + 'To avoid unexpected behavior, please change the value to boolean.' % icmp_block_inversion) + expected_state = 'enabled' if (desired_state == 'enabled') == icmp_block_inversion_status else 'disabled' transaction = IcmpBlockInversionTransaction( module, action_args=(), zone=zone, - desired_state=desired_state, + desired_state=expected_state, permanent=permanent, immediate=immediate, ) @@ -922,14 +931,6 @@ def main(): if changed is True: msgs.append("Changed icmp-block-inversion %s to %s" % (icmp_block_inversion, desired_state)) - # Type of icmp_block_inversion will be changed to boolean in a future release. - try: - boolean(icmp_block_inversion, True) - except TypeError: - module.warn('The value of the icmp_block_inversion option is "%s". ' - 'The type of the option will be changed from string to boolean in a future release. ' - 'To avoid unexpected behavior, please change the value to boolean.' % icmp_block_inversion) - if service is not None: transaction = ServiceTransaction( @@ -1050,12 +1051,21 @@ def main(): msgs = msgs + transaction_msgs if masquerade is not None: + # Type of masquerade will be changed to boolean in a future release. + masquerade_status = True + try: + masquerade_status = boolean(masquerade, True) + except TypeError: + module.warn('The value of the masquerade option is "%s". ' + 'The type of the option will be changed from string to boolean in a future release. ' + 'To avoid unexpected behavior, please change the value to boolean.' % masquerade) + expected_state = 'enabled' if (desired_state == 'enabled') == masquerade_status else 'disabled' transaction = MasqueradeTransaction( module, action_args=(), zone=zone, - desired_state=desired_state, + desired_state=expected_state, permanent=permanent, immediate=immediate, ) @@ -1063,14 +1073,6 @@ def main(): changed, transaction_msgs = transaction.run() msgs = msgs + transaction_msgs - # Type of masquerade will be changed to boolean in a future release. - try: - boolean(masquerade, True) - except TypeError: - module.warn('The value of the masquerade option is "%s". ' - 'The type of the option will be changed from string to boolean in a future release. ' - 'To avoid unexpected behavior, please change the value to boolean.' % masquerade) - if target is not None: transaction = ZoneTargetTransaction( diff --git a/tests/integration/targets/firewalld/tasks/icmp_block_inversion_test_cases.yml b/tests/integration/targets/firewalld/tasks/icmp_block_inversion_test_cases.yml new file mode 100644 index 0000000..62fcbe4 --- /dev/null +++ b/tests/integration/targets/firewalld/tasks/icmp_block_inversion_test_cases.yml @@ -0,0 +1,172 @@ +# Test playbook for the firewalld module - icmp block inversion operations +# (c) 2022, Gregory Furlong +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +- name: Icmp block inversion enabled when icmp block inversion is truthy and state is enabled + block: + - name: Testing enable icmp block inversion + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: yes + permanent: yes + state: enabled + register: result + + - name: assert icmp block inversion is enabled + assert: + that: + - result is changed + + - name: Testing enable icmp block inversion (verify not changed) + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: yes + permanent: yes + state: enabled + register: result + + - name: assert icmp block inversion is enabled (verify not changed) + assert: + that: + - result is not changed + +- name: Icmp block inversion disabled when icmp block inversion is falsy and state is enabled + block: + - name: Testing disable icmp block inversion + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: no + permanent: yes + state: enabled + register: result + + - name: assert icmp block inversion is disabled + assert: + that: + - result is changed + + - name: Testing disable icmp block inversion (verify not changed) + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: no + permanent: yes + state: enabled + register: result + + - name: assert icmp block inversion is disabled (verify not changed) + assert: + that: + - result is not changed + +- name: Icmp block inversion enabled when icmp block inversion is falsy and state is disabled + block: + - name: Testing enable icmp block inversion + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: no + permanent: yes + state: disabled + register: result + + - name: assert icmp block inversion is enabled + assert: + that: + - result is changed + + - name: Testing enable icmp block inversion (verify not changed) + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: no + permanent: yes + state: disabled + register: result + + - name: assert icmp block inversion is enabled (verify not changed) + assert: + that: + - result is not changed + +- name: Icmp block inversion disabled when icmp block inversion is truthy and state is disabled + block: + - name: Testing disable icmp block inversion + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: yes + permanent: yes + state: disabled + register: result + + - name: assert icmp block inversion is disabled + assert: + that: + - result is changed + + - name: Testing disable icmp block inversion (verify not changed) + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: yes + permanent: yes + state: disabled + register: result + + - name: assert icmp block inversion is disabled (verify not changed) + assert: + that: + - result is not changed + +# Validate backwards compatible behavior until icmp block inversion is switched from string to boolean type +- name: Icmp block inversion enabled when icmp block inversion is non-boolean string and state is enabled + block: + - name: Testing enable icmp block inversion + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: 'some string' + permanent: yes + state: enabled + register: result + + - name: assert icmp block inversion is enabled + assert: + that: + - result is changed + + - name: Testing enable icmp block inversion (verify not changed) + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: 'some string' + permanent: yes + state: enabled + register: result + + - name: assert icmp block inversion is enabled (verify not changed) + assert: + that: + - result is not changed + +- name: Icmp block inversion disabled when icmp block inversion is non-boolean string and state is disabled + block: + - name: Testing disable icmp block inversion + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: 'some string' + permanent: yes + state: disabled + register: result + + - name: assert icmp block inversion is disabled + assert: + that: + - result is changed + + - name: Testing disable icmp block inversion (verify not changed) + ansible.posix.firewalld: + zone: trusted + icmp_block_inversion: 'some string' + permanent: yes + state: disabled + register: result + + - name: assert icmp block inversion is disabled (verify not changed) + assert: + that: + - result is not changed diff --git a/tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml b/tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml new file mode 100644 index 0000000..860378f --- /dev/null +++ b/tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml @@ -0,0 +1,172 @@ +# Test playbook for the firewalld module - masquerade operations +# (c) 2022, Gregory Furlong +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +- name: Masquerade enabled when masquerade is truthy and state is enabled + block: + - name: Testing enable masquerade + ansible.posix.firewalld: + zone: trusted + masquerade: yes + permanent: yes + state: enabled + register: result + + - name: assert masquerade is enabled + assert: + that: + - result is changed + + - name: Testing enable masquerade (verify not changed) + ansible.posix.firewalld: + zone: trusted + masquerade: yes + permanent: yes + state: enabled + register: result + + - name: assert masquerade is enabled (verify not changed) + assert: + that: + - result is not changed + +- name: Masquerade disabled when masquerade is falsy and state is enabled + block: + - name: Testing disable masquerade + ansible.posix.firewalld: + zone: trusted + masquerade: no + permanent: yes + state: enabled + register: result + + - name: assert masquerade is disabled + assert: + that: + - result is changed + + - name: Testing disable masquerade (verify not changed) + ansible.posix.firewalld: + zone: trusted + masquerade: no + permanent: yes + state: enabled + register: result + + - name: assert masquerade is disabled (verify not changed) + assert: + that: + - result is not changed + +- name: Masquerade enabled when masquerade is falsy and state is disabled + block: + - name: Testing enable masquerade + ansible.posix.firewalld: + zone: trusted + masquerade: no + permanent: yes + state: disabled + register: result + + - name: assert masquerade is enabled + assert: + that: + - result is changed + + - name: Testing enable masquerade (verify not changed) + ansible.posix.firewalld: + zone: trusted + masquerade: no + permanent: yes + state: disabled + register: result + + - name: assert masquerade is enabled (verify not changed) + assert: + that: + - result is not changed + +- name: Masquerade disabled when masquerade is truthy and state is disabled + block: + - name: Testing disable masquerade + ansible.posix.firewalld: + zone: trusted + masquerade: yes + permanent: yes + state: disabled + register: result + + - name: assert masquerade is disabled + assert: + that: + - result is changed + + - name: Testing disable masquerade (verify not changed) + ansible.posix.firewalld: + zone: trusted + masquerade: yes + permanent: yes + state: disabled + register: result + + - name: assert masquerade is disabled (verify not changed) + assert: + that: + - result is not changed + +# Validate backwards compatible behavior until masquerade is switched from string to boolean type +- name: Masquerade enabled when masquerade is non-boolean string and state is enabled + block: + - name: Testing enable masquerade + ansible.posix.firewalld: + zone: trusted + masquerade: 'some string' + permanent: yes + state: enabled + register: result + + - name: assert masquerade is enabled + assert: + that: + - result is changed + + - name: Testing enable masquerade (verify not changed) + ansible.posix.firewalld: + zone: trusted + masquerade: 'some string' + permanent: yes + state: enabled + register: result + + - name: assert masquerade is enabled (verify not changed) + assert: + that: + - result is not changed + +- name: Masquerade disabled when masquerade is non-boolean string and state is disabled + block: + - name: Testing disable masquerade + ansible.posix.firewalld: + zone: trusted + masquerade: 'some string' + permanent: yes + state: disabled + register: result + + - name: assert masquerade is disabled + assert: + that: + - result is changed + + - name: Testing disable masquerade (verify not changed) + ansible.posix.firewalld: + zone: trusted + masquerade: 'some string' + permanent: yes + state: disabled + register: result + + - name: assert masquerade is disabled (verify not changed) + assert: + that: + - result is not changed diff --git a/tests/integration/targets/firewalld/tasks/run_all_tests.yml b/tests/integration/targets/firewalld/tasks/run_all_tests.yml index 06a7d31..e88b007 100644 --- a/tests/integration/targets/firewalld/tasks/run_all_tests.yml +++ b/tests/integration/targets/firewalld/tasks/run_all_tests.yml @@ -28,5 +28,11 @@ # firewalld port forwarding operation test cases - include_tasks: port_forward_test_cases.yml +# firewalld masquerade operation test cases +- include_tasks: masquerade_test_cases.yml + +# firewalld icmp block inversion operation test cases +- include_tasks: icmp_block_inversion_test_cases.yml + # firewalld interface operation test cases - include_tasks: interface_test_cases.yml