Merge 6280bb8ec8 into f632fad9d4

[Breaking Change] [firewalld] Change type of icmp_block_inversion option from str to bool

SUMMARY
Changed the type of icmp_block_inversion option from str to bool

Fixes #586

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ansible.posix.firewalld

ADDITIONAL INFORMATION
Related  #582 and #584

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

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

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.
+        expected_state = 'enabled' if (desired_state == 'enabled') == icmp_block_inversion else 'disabled'
-        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'
-
         transaction = IcmpBlockInversionTransaction(
             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
@@ -298,6 +311,13 @@ class SysctlModule(object):
             # so return here and do not continue to the error processing below
             # https://github.com/ansible/ansible/issues/58158
             return
+        else:
+            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]
@@ -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/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

tests/integration/targets/sysctl/tasks/main.yml

@@ -230,6 +230,40 @@
         that:
           - sysctl_test4 is failed
         
+      ##
+      ## sysctl --system
+      ##
+
+    - name: Set vm.swappiness to 10 with --system option
+      ansible.posix.sysctl:
+        name: vm.swappiness
+        value: 10
+        state: present
+        reload: false
+        sysctl_set: true
+        system: true
+      register: sysctl_system_test1
+
+    - name: Check with sysctl command
+      ansible.builtin.command: sysctl vm.swappiness
+      changed_when: false
+      register: sysctl_check_system1
+
+    - name: Debug sysctl_system_test1 sysctl_check_system1
+      ansible.builtin.debug:
+        var: item
+        verbosity: 1
+      with_items:
+        - "{{ sysctl_system_test1 }}"
+        - "{{ sysctl_check_system1 }}"
+
+    - name: Validate results for --system option
+      ansible.builtin.assert:
+        that:
+          - sysctl_system_test1 is changed
+          - sysctl_check_system1.stdout_lines == ["vm.swappiness = 10"]
+
+
 - name: Test on RHEL VMs
   when:
     - ansible_facts.virtualization_type != 'docker'
@@ -366,3 +400,33 @@
         that:
           - stat_result.stat.islnk is defined and stat_result.stat.islnk
           - stat_result.stat.lnk_source == '/tmp/ansible_sysctl_test.conf'
+    
+    # Test sysctl: --system
+    - name: Set vm.swappiness to 10 with --system option
+      ansible.posix.sysctl:
+        name: vm.swappiness
+        value: 10
+        state: present
+        reload: false
+        sysctl_set: true
+        system: true
+      register: sysctl_system_test1
+
+    - name: Check with sysctl command
+      ansible.builtin.command: sysctl vm.swappiness
+      changed_when: false
+      register: sysctl_check_system1
+
+    - name: Debug sysctl_system_test1 sysctl_check_system1
+      ansible.builtin.debug:
+        var: item
+        verbosity: 1
+      with_items:
+        - "{{ sysctl_system_test1 }}"
+        - "{{ sysctl_check_system1 }}"
+
+    - name: Validate results for --system option
+      ansible.builtin.assert:
+        that:
+          - sysctl_system_test1 is changed
+          - sysctl_check_system1.stdout_lines == ["vm.swappiness = 10"]