Add integration test for --system option

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>

.github/BOTMETA.yml

@@ -40,7 +40,6 @@ files:
     labels: debug
   $plugins/patch.py:
     labels: patch
-  $plugins/skippy.py:
   $plugins/synchronize.py:
     labels: synchronize
   $plugins/timer.py:

changelogs/fragments/387_callback_output_header.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - callback plugins - Add recap information to timer, profile_roles and profile_tasks callback outputs (https://github.com/ansible-collections/ansible.posix/pull/387).

changelogs/fragments/568_update_authorized_key.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - authorized_keys - allow using absolute path to a file as a SSH key(s) source (https://github.com/ansible-collections/ansible.posix/pull/568)

changelogs/fragments/576_bump_version_2.yml

@@ -0,0 +1,3 @@
+---
+trivial:
+  - Bump ansible.posix version to 2.0.0.

changelogs/fragments/581_ci_selinux.yml

@@ -0,0 +1,3 @@
+---
+trivial:
+  - selinux - conditions for selinux integration tests have been modified to be more accurate.

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

galaxy.yml

@@ -1,7 +1,7 @@
 ---
 namespace: ansible
 name: posix
-version: 1.6.1
+version: 2.0.0
 readme: README.md
 authors:
   - Ansible (github.com/ansible)

meta/runtime.yml

@@ -1,8 +1,2 @@
 ---
-requires_ansible: ">=2.14.0"
-plugin_routing:
-  callback:
-    skippy:
-      deprecation:
-        removal_date: "2022-06-01"
-        warning_text: See the plugin documentation for more details
+requires_ansible: ">=2.15.0"

plugins/callback/profile_roles.py

@@ -128,7 +128,10 @@ class CallbackModule(CallbackBase):
         self._display_tasktime()
 
     def playbook_on_stats(self, stats):
-        self._display_tasktime()
+        # Align summary report header with other callback plugin summary
+        self._display.banner("ROLES RECAP")
+
+        self._display.display(tasktime())
         self._display.display(filled("", fchar="="))
 
         timestamp(self)

plugins/callback/profile_tasks.py

@@ -193,7 +193,10 @@ class CallbackModule(CallbackBase):
         self._display_tasktime()
 
     def playbook_on_stats(self, stats):
-        self._display_tasktime()
+        # Align summary report header with other callback plugin summary
+        self._display.banner("TASKS RECAP")
+
+        self._display.display(tasktime())
         self._display.display(filled("", fchar="="))
 
         timestamp(self)

plugins/callback/skippy.py

@@ -1,43 +0,0 @@
-# (c) 2012-2014, Michael DeHaan <michael.dehaan@gmail.com>
-# (c) 2017 Ansible Project
-# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
-
-# Make coding more python3-ish
-from __future__ import (absolute_import, division, print_function)
-__metaclass__ = type
-
-DOCUMENTATION = '''
-    name: skippy
-    type: stdout
-    requirements:
-      - set as main display callback
-    short_description: Ansible screen output that ignores skipped status
-    deprecated:
-      why: The 'default' callback plugin now supports this functionality
-      removed_at_date: '2022-06-01'
-      alternative: "'default' callback plugin with 'display_skipped_hosts = no' option"
-    extends_documentation_fragment:
-      - default_callback
-    description:
-      - This callback does the same as the default except it does not output skipped host/task/item status
-'''
-
-from ansible.plugins.callback.default import CallbackModule as CallbackModule_default
-
-
-class CallbackModule(CallbackModule_default):
-
-    '''
-    This is the default callback interface, which simply prints messages
-    to stdout when new callback events are received.
-    '''
-
-    CALLBACK_VERSION = 2.0
-    CALLBACK_TYPE = 'stdout'
-    CALLBACK_NAME = 'ansible.posix.skippy'
-
-    def v2_runner_on_skipped(self, result):
-        pass
-
-    def v2_runner_item_on_skipped(self, result):
-        pass

plugins/callback/timer.py

@@ -46,4 +46,6 @@ class CallbackModule(CallbackBase):
     def v2_playbook_on_stats(self, stats):
         end_time = datetime.utcnow()
         runtime = end_time - self.start_time
-        self._display.display("Playbook run took %s days, %s hours, %s minutes, %s seconds" % (self.days_hours_minutes_seconds(runtime)))
+        # Align summary report header with other callback plugin summary
+        self._display.banner("PLAYBOOK RECAP")
+        self._display.display("Playbook run took %s days, %s hours, %s minutes, %s seconds\n\r" % (self.days_hours_minutes_seconds(runtime)))

plugins/modules/authorized_key.py

@@ -24,6 +24,7 @@ options:
   key:
     description:
       - The SSH public key(s), as a string or (since Ansible 1.9) url (https://github.com/username.keys).
+      - You can also use V(file://) prefix to search remote for a file with SSH key(s).
     type: str
     required: true
   path:
@@ -96,6 +97,12 @@ EXAMPLES = r'''
     state: present
     key: https://github.com/charlie.keys
 
+- name: Set authorized keys taken from path on controller node
+  ansible.posix.authorized_key:
+    user: charlie
+    state: present
+    key: file:///home/charlie/.ssh/id_rsa.pub
+
 - name: Set authorized keys taken from url using lookup
   ansible.posix.authorized_key:
     user: charlie
@@ -223,6 +230,7 @@ from operator import itemgetter
 from ansible.module_utils._text import to_native
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.urls import fetch_url
+from ansible.module_utils.six.moves.urllib.parse import urlparse
 
 
 class keydict(dict):
@@ -556,7 +564,7 @@ def enforce_state(module, params):
     follow = params.get('follow', False)
     error_msg = "Error getting key from: %s"
 
-    # if the key is a url, request it and use it as key source
+    # if the key is a url or file, request it and use it as key source
     if key.startswith("http"):
         try:
             resp, info = fetch_url(module, key)
@@ -570,6 +578,19 @@ def enforce_state(module, params):
         # resp.read gives bytes on python3, convert to native string type
         key = to_native(key, errors='surrogate_or_strict')
 
+    if key.startswith("file"):
+        # if the key is an absolute path, check for existense and use it as a key source
+        key_path = urlparse(key).path
+        if not os.path.exists(key_path):
+            module.fail_json(msg="Path to a key file not found: %s" % key_path)
+        if not os.path.isfile(key_path):
+            module.fail_json(msg="Path to a key is a directory and must be a file: %s" % key_path)
+        try:
+            with open(key_path, 'r') as source_fh:
+                key = source_fh.read()
+        except OSError as e:
+            module.fail_json(msg="Failed to read key file %s : %s" % (key_path, to_native(e)))
+
     # extract individual keys into an array, skipping blank lines and comments
     new_keys = [s for s in key.splitlines() if s and not s.startswith('#')]
 

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/authorized_key/defaults/main.yml

@@ -35,3 +35,5 @@ multiple_keys_comments: |
   ssh-rsa DATA_BASIC 1@testing
   # I like adding comments yo-dude-this-is-not-a-key INVALID_DATA 2@testing
   ecdsa-sha2-nistp521 ECDSA_DATA 4@testing
+
+key_path: /tmp/id_rsa.pub

tests/integration/targets/authorized_key/tasks/check_path.yml

@@ -0,0 +1,32 @@
+---
+- name: Create key file for test
+  ansible.builtin.copy:
+    dest: "{{ key_path }}"
+    content: "{{ rsa_key_basic }}"
+    mode: "0600"
+
+- name: Add key using path
+  ansible.posix.authorized_key:
+    user: root
+    key: file://{{ key_path }}
+    state: present
+    path: "{{ output_dir | expanduser }}/authorized_keys"
+  register: result
+
+- name: Assert that the key was added
+  ansible.builtin.assert:
+    that:
+      - result.changed == true
+
+- name: Add key using path again
+  ansible.posix.authorized_key:
+    user: root
+    key: file://{{ key_path }}
+    state: present
+    path: "{{ output_dir | expanduser }}/authorized_keys"
+  register: result
+
+- name: Assert that no changes were applied
+  ansible.builtin.assert:
+    that:
+      - result.changed == false

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

@@ -31,3 +31,6 @@
 
 - name: Test for the management of comments with key
   ansible.builtin.import_tasks: comments.yml
+
+- name: Test for specifying key as a path
+  ansible.builtin.import_tasks: check_path.yml

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

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:

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

@@ -229,6 +229,40 @@
       ansible.builtin.assert:
         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:
@@ -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"]