From 6175a5028b1f02ab113b9c195a5a0a7b47d8f2ff Mon Sep 17 00:00:00 2001 From: Hideki Saito Date: Thu, 28 Nov 2024 13:59:52 +0900 Subject: [PATCH] Change type of icmp_block_inversion option from str to bool * Fixes #586 Signed-off-by: Hideki Saito --- .../fragments/598_icmp_block_inversion.yml | 3 + plugins/modules/firewalld.py | 58 ++++++++++--------- .../tasks/icmp_block_inversion_test_cases.yml | 57 ------------------ 3 files changed, 33 insertions(+), 85 deletions(-) create mode 100644 changelogs/fragments/598_icmp_block_inversion.yml diff --git a/changelogs/fragments/598_icmp_block_inversion.yml b/changelogs/fragments/598_icmp_block_inversion.yml new file mode 100644 index 0000000..0daaf8f --- /dev/null +++ b/changelogs/fragments/598_icmp_block_inversion.yml @@ -0,0 +1,3 @@ +--- +breaking_changes: + - firewalld - Changed the type of icmp_block_inversion option from str to bool (https://github.com/ansible-collections/ansible.posix/issues/586). diff --git a/plugins/modules/firewalld.py b/plugins/modules/firewalld.py index 4333afd..6dd26aa 100644 --- a/plugins/modules/firewalld.py +++ b/plugins/modules/firewalld.py @@ -74,7 +74,8 @@ options: icmp_block_inversion: description: - Enable/Disable inversion of ICMP blocks for a zone in firewalld. - type: str + - Note that the option type is changed to bool in ansible.posix version 2.0.0 and later. + type: bool zone: description: - The firewalld zone to add/remove to/from. @@ -152,7 +153,7 @@ author: ''' EXAMPLES = r''' -- name: permanently enable https service, also enable it immediately if possible +- name: Permanently enable https service, also enable it immediately if possible ansible.posix.firewalld: service: https state: enabled @@ -160,81 +161,92 @@ EXAMPLES = r''' immediate: true offline: true -- name: permit traffic in default zone for https service +- name: Permit traffic in default zone for https service ansible.posix.firewalld: service: https permanent: true state: enabled -- name: permit ospf traffic +- name: Permit ospf traffic ansible.posix.firewalld: protocol: ospf permanent: true state: enabled -- name: do not permit traffic in default zone on port 8081/tcp +- name: Do not permit traffic in default zone on port 8081/tcp ansible.posix.firewalld: port: 8081/tcp permanent: true state: disabled -- ansible.posix.firewalld: +- name: Permit traffic in default zone on port 161-162/ucp + ansible.posix.firewalld: port: 161-162/udp permanent: true state: enabled -- ansible.posix.firewalld: +- name: Permit traffic in dmz zone on http service + ansible.posix.firewalld: zone: dmz service: http permanent: true state: enabled -- ansible.posix.firewalld: +- name: Enable FTP service with rate limiting using firewalld rich rule + ansible.posix.firewalld: rich_rule: rule service name="ftp" audit limit value="1/m" accept permanent: true state: enabled -- ansible.posix.firewalld: +- name: Allow traffic from 192.0.2.0/24 in internal zone + ansible.posix.firewalld: source: 192.0.2.0/24 zone: internal state: enabled -- ansible.posix.firewalld: +- name: Assign eth2 interface to trusted zone + ansible.posix.firewalld: zone: trusted interface: eth2 permanent: true state: enabled -- ansible.posix.firewalld: +- name: Enable forwarding in internal zone + ansible.posix.firewalld: forward: true state: enabled permanent: true zone: internal -- ansible.posix.firewalld: +- name: Enable masquerade in dmz zone + ansible.posix.firewalld: masquerade: true state: enabled permanent: true zone: dmz -- ansible.posix.firewalld: +- name: Create custom zone if not already present + ansible.posix.firewalld: zone: custom state: present permanent: true -- ansible.posix.firewalld: +- name: Enable ICMP block inversion in drop zone + ansible.posix.firewalld: zone: drop state: enabled permanent: true icmp_block_inversion: true -- ansible.posix.firewalld: +- name: Block ICMP echo requests in drop zone + ansible.posix.firewalld: zone: drop state: enabled permanent: true icmp_block: echo-request -- ansible.posix.firewalld: +- name: Set internal zone target to ACCEPT + ansible.posix.firewalld: zone: internal state: present permanent: true @@ -250,7 +262,6 @@ EXAMPLES = r''' ''' from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.parsing.convert_bool import boolean from ansible_collections.ansible.posix.plugins.module_utils.firewalld import FirewallTransaction, fw_offline try: @@ -864,7 +875,7 @@ def main(): module = AnsibleModule( argument_spec=dict( icmp_block=dict(type='str'), - icmp_block_inversion=dict(type='str'), + icmp_block_inversion=dict(type='bool'), service=dict(type='str'), protocol=dict(type='str'), port=dict(type='str'), @@ -987,16 +998,7 @@ 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' - + expected_state = 'enabled' if (desired_state == 'enabled') == icmp_block_inversion else 'disabled' transaction = IcmpBlockInversionTransaction( module, action_args=(), 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 index 3bd5bf0..f416c85 100644 --- a/tests/integration/targets/firewalld/tasks/icmp_block_inversion_test_cases.yml +++ b/tests/integration/targets/firewalld/tasks/icmp_block_inversion_test_cases.yml @@ -114,60 +114,3 @@ ansible.builtin.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: true - state: enabled - register: result - - - name: Assert icmp block inversion is enabled - ansible.builtin.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: true - state: enabled - register: result - - - name: Assert icmp block inversion is enabled (verify not changed) - ansible.builtin.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: true - state: disabled - register: result - - - name: Assert icmp block inversion is disabled - ansible.builtin.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: true - state: disabled - register: result - - - name: Assert icmp block inversion is disabled (verify not changed) - ansible.builtin.assert: - that: - - result is not changed