Merge pull request #584 from saito-hideki/issue/582

Changed the type of the forward and masquerade options from str to bool

SUMMARY
The forward and masquerade options for the firewall module takes either True or False as a value.
Currently, it is defined as a string, but it should be a boolean.

Fixes #582

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ansible.posix.firewalld

ADDITIONAL INFORMATION
None

Reviewed-by: Adam Miller <admiller@redhat.com>
Reviewed-by: Andrew Klychkov <aklychko@redhat.com>

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).

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'),
+            forward=dict(type='bool'),
-            masquerade=dict(type='str'),
+            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.
+        expected_state = 'enabled' if (desired_state == 'enabled') == forward else 'disabled'
-        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'
         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.
+        expected_state = 'enabled' if (desired_state == 'enabled') == masquerade else 'disabled'
-        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=(),

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