Merge 05724a097b into 5eacaba86d

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'),
-            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=(),

plugins/modules/sysctl.py

@@ -101,6 +101,7 @@ import os
 import platform
 import re
 import tempfile
+import glob
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.six import string_types
@@ -114,12 +115,24 @@ class SysctlModule(object):
     # success or failure.
     LANG_ENV = {'LANG': 'C', 'LC_ALL': 'C', 'LC_MESSAGES': 'C'}
 
+    # We define a variable to keep all the directories to be read, equivalent to
+    # (/sbin/sysctl --system) option
+    SYSCTL_DIRS = [
+        '/etc/sysctl.d/*.conf',
+        '/run/sysctl.d/*.conf',
+        '/usr/local/lib/sysctl.d/*.conf',
+        '/usr/lib/sysctl.d/*.conf',
+        '/lib/sysctl.d/*.conf',
+        '/etc/sysctl.conf'
+    ]
+
     def __init__(self, module):
         self.module = module
         self.args = self.module.params
 
         self.sysctl_cmd = self.module.get_bin_path('sysctl', required=True)
         self.sysctl_file = self.args['sysctl_file']
+        self.system_Wide = self.args['system_Wide']
 
         self.proc_value = None  # current token value in proc fs
         self.file_value = None  # current token value in file
@@ -299,15 +312,22 @@ class SysctlModule(object):
             # https://github.com/ansible/ansible/issues/58158
             return
         else:
-            # system supports reloading via the -p flag to sysctl, so we'll use that
-            sysctl_args = [self.sysctl_cmd, '-p', self.sysctl_file]
-            if self.args['ignoreerrors']:
-                sysctl_args.insert(1, '-e')
+            if self.system_Wide:
+                for sysctl_file in self.SYSCTL_DIRS:
+                    for conf_file in glob.glob(sysctl_file):
+                        rc, out, err = self.module.run_command([self.sysctl_cmd, '-p', conf_file], environ_update=self.LANG_ENV)
+                        if rc != 0 or self._stderr_failed(err):
+                            self.module.fail_json(msg="Failed to reload sysctl: %s" % to_native(out) + to_native(err))
+            else:
+                # system supports reloading via the -p flag to sysctl, so we'll use that
+                sysctl_args = [self.sysctl_cmd, '-p', self.sysctl_file]
+                if self.args['ignoreerrors']:
+                    sysctl_args.insert(1, '-e')
 
-            rc, out, err = self.module.run_command(sysctl_args, environ_update=self.LANG_ENV)
+                rc, out, err = self.module.run_command(sysctl_args, environ_update=self.LANG_ENV)
 
-        if rc != 0 or self._stderr_failed(err):
-            self.module.fail_json(msg="Failed to reload sysctl: %s" % to_native(out) + to_native(err))
+            if rc != 0 or self._stderr_failed(err):
+                self.module.fail_json(msg="Failed to reload sysctl: %s" % to_native(out) + to_native(err))
 
     # ==============================================================
     #   SYSCTL FILE MANAGEMENT
@@ -394,7 +414,8 @@ def main():
             reload=dict(default=True, type='bool'),
             sysctl_set=dict(default=False, type='bool'),
             ignoreerrors=dict(default=False, type='bool'),
-            sysctl_file=dict(default='/etc/sysctl.conf', type='path')
+            sysctl_file=dict(default='/etc/sysctl.conf', type='path'),
+            system_wide=dict(default=False, type='bool'),  # system_wide parameter
         ),
         supports_check_mode=True,
         required_if=[('state', 'present', ['value'])],

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