Merge d05a5c70b5 into 9171b7fcf1

changelogs/fragments/584_firewalld_opt_type.yml

@@ -1,3 +0,0 @@
----
-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,13 +112,11 @@ 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.
-      - Note that the option type is changed to bool in ansible.posix version 2.0.0 and later.
+    type: str
-    type: bool
   masquerade:
     description:
       - The masquerade setting you would like to enable/disable to/from zones within firewalld.
-      - Note that the option type is changed to bool in ansible.posix version 2.0.0 and later.
+    type: str
-    type: bool
   offline:
     description:
       - Ignores O(immediate) if O(permanent=true) and firewalld is not running.
@@ -877,8 +875,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='bool'),
+            forward=dict(type='str'),
-            masquerade=dict(type='bool'),
+            masquerade=dict(type='str'),
             offline=dict(type='bool', default=False),
             target=dict(type='str', choices=['default', 'ACCEPT', 'DROP', '%%REJECT%%']),
         ),
@@ -1131,7 +1129,16 @@ def main():
         msgs = msgs + transaction_msgs
 
     if forward is not None:
-        expected_state = 'enabled' if (desired_state == 'enabled') == forward else 'disabled'
+        # 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'
         transaction = ForwardTransaction(
             module,
             action_args=(),
@@ -1145,7 +1152,16 @@ def main():
         msgs = msgs + transaction_msgs
 
     if masquerade is not None:
-        expected_state = 'enabled' if (desired_state == 'enabled') == masquerade else 'disabled'
+        # 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=(),

tests/integration/targets/firewalld/tasks/masquerade_test_cases.yml

@@ -114,3 +114,60 @@
       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