From 2f095566c7fa87ea02cec43418799788d694de0d Mon Sep 17 00:00:00 2001 From: Hideki Saito Date: Tue, 15 Oct 2024 17:10:56 +0900 Subject: [PATCH 1/2] Modify conditions for selinux integratuion tests Signed-off-by: Hideki Saito --- changelogs/fragments/581_ci_selinux.yml | 3 +++ tests/integration/targets/selinux/tasks/selinux.yml | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/581_ci_selinux.yml diff --git a/changelogs/fragments/581_ci_selinux.yml b/changelogs/fragments/581_ci_selinux.yml new file mode 100644 index 0000000..c38b2f2 --- /dev/null +++ b/changelogs/fragments/581_ci_selinux.yml @@ -0,0 +1,3 @@ +--- +trivial: + - selinux - conditions for selinux integration tests have been modified to be more accurate. diff --git a/tests/integration/targets/selinux/tasks/selinux.yml b/tests/integration/targets/selinux/tasks/selinux.yml index b9cfb20..b6ab7cc 100644 --- a/tests/integration/targets/selinux/tasks/selinux.yml +++ b/tests/integration/targets/selinux/tasks/selinux.yml @@ -128,8 +128,8 @@ ansible.builtin.assert: that: - selinux_config_original | length == selinux_config_after | length - - selinux_config_after[selinux_config_after.index('SELINUX=disabled')] is search("^SELINUX=\w+$") - - selinux_config_after[selinux_config_after.index('SELINUXTYPE=targeted')] is search("^SELINUXTYPE=\w+$") + - (selinux_config_after | select("search", "^SELINUX=disabled\s*$") | list | length) > 0 + - (selinux_config_after | select("search", "^SELINUXTYPE=targeted\s*$") | list | length) > 0 - name: TEST 1 | Disable SELinux again, with kernel arguments update ansible.posix.selinux: From 8b611775d64f756475010980444bd847d7179aef Mon Sep 17 00:00:00 2001 From: Hideki Saito Date: Wed, 23 Oct 2024 18:54:36 +0900 Subject: [PATCH 2/2] Changed the type of forward and masquerade options from str to bool * Breaking Change * Fixes #582 Signed-off-by: Hideki Saito --- .../fragments/584_firewalld_opt_type.yml | 3 + plugins/modules/firewalld.py | 32 +++-------- .../firewalld/tasks/masquerade_test_cases.yml | 57 ------------------- 3 files changed, 11 insertions(+), 81 deletions(-) create mode 100644 changelogs/fragments/584_firewalld_opt_type.yml diff --git a/changelogs/fragments/584_firewalld_opt_type.yml b/changelogs/fragments/584_firewalld_opt_type.yml new file mode 100644 index 0000000..44ede21 --- /dev/null +++ b/changelogs/fragments/584_firewalld_opt_type.yml @@ -0,0 +1,3 @@ +--- +breaking_changes: + - firewalld - Changed the type of forward and masquerade options from str to bool (https://github.com/ansible-collections/ansible.posix/issues/582). diff --git a/plugins/modules/firewalld.py b/plugins/modules/firewalld.py index 167c449..4333afd 100644 --- a/plugins/modules/firewalld.py +++ b/plugins/modules/firewalld.py @@ -112,11 +112,13 @@ options: description: - The forward setting you would like to enable/disable to/from zones within firewalld. - This option only is supported by firewalld v0.9.0 or later. - type: str + - Note that the option type is changed to bool in ansible.posix version 2.0.0 and later. + type: bool masquerade: description: - The masquerade setting you would like to enable/disable to/from zones within firewalld. - type: str + - Note that the option type is changed to bool in ansible.posix version 2.0.0 and later. + type: bool offline: description: - Ignores O(immediate) if O(permanent=true) and firewalld is not running. @@ -875,8 +877,8 @@ def main(): state=dict(type='str', required=True, choices=['absent', 'disabled', 'enabled', 'present']), timeout=dict(type='int', default=0), interface=dict(type='str'), - forward=dict(type='str'), - masquerade=dict(type='str'), + forward=dict(type='bool'), + masquerade=dict(type='bool'), offline=dict(type='bool', default=False), target=dict(type='str', choices=['default', 'ACCEPT', 'DROP', '%%REJECT%%']), ), @@ -1129,16 +1131,7 @@ def main(): msgs = msgs + transaction_msgs if forward is not None: - # Type of forward will be changed to boolean in a future release. - forward_status = False - try: - forward_status = boolean(forward, False) - except TypeError: - module.warn('The value of the forward 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.' % forward) - - expected_state = 'enabled' if (desired_state == 'enabled') == forward_status else 'disabled' + expected_state = 'enabled' if (desired_state == 'enabled') == forward else 'disabled' transaction = ForwardTransaction( module, action_args=(), @@ -1152,16 +1145,7 @@ 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' + expected_state = 'enabled' if (desired_state == 'enabled') == masquerade else 'disabled' transaction = MasqueradeTransaction( module, action_args=(), diff --git a/tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml b/tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml index 82d321e..3588382 100644 --- a/tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml +++ b/tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml @@ -114,60 +114,3 @@ ansible.builtin.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: true - state: enabled - register: result - - - name: Assert masquerade is enabled - ansible.builtin.assert: - that: - - result is changed - - - name: Testing enable masquerade (verify not changed) - ansible.posix.firewalld: - zone: trusted - masquerade: some string - permanent: true - state: enabled - register: result - - - name: Assert masquerade is enabled (verify not changed) - ansible.builtin.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: true - state: disabled - register: result - - - name: Assert masquerade is disabled - ansible.builtin.assert: - that: - - result is changed - - - name: Testing disable masquerade (verify not changed) - ansible.posix.firewalld: - zone: trusted - masquerade: some string - permanent: true - state: disabled - register: result - - - name: Assert masquerade is disabled (verify not changed) - ansible.builtin.assert: - that: - - result is not changed