Merge c4ff0545f1 into b39ee97ccc

docs: fix broken badge and restore coverage badge

SUMMARY
Replaced the outdated Shippable badge and active Codecov coverage badge, like other repos in ansible-collections org
ISSUE TYPE


Docs Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Hideki Saito <saito@fgrep.org>

README.md

@@ -2,7 +2,7 @@
 <!-- Add CI and code coverage badges here. Samples included below. -->
 [![Build Status](
 https://dev.azure.com/ansible/ansible.posix/_apis/build/status/CI?branchName=main)](https://dev.azure.com/ansible/ansible.posix/_build?definitionId=26)
-[![Run Status](https://api.shippable.com/projects/5e669aaf8b17a60007e4d18d/badge?branch=main)]() <!--[![Codecov](https://img.shields.io/codecov/c/github/ansible-collections/ansible.posix)](https://codecov.io/gh/ansible-collections/ansible.posix)-->
+[![Codecov](https://img.shields.io/codecov/c/github/ansible-collections/ansible.posix)](https://codecov.io/gh/ansible-collections/ansible.posix)
 
 ## Communication
 

changelogs/fragments/639_fix_authorized_key.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - ansible.posix.authorized_key - fixes error on permission denied in authorized_key module (https://github.com/ansible-collections/ansible.posix/issues/462).

plugins/modules/authorized_key.py

@@ -225,6 +225,8 @@ import os.path
 import tempfile
 import re
 import shlex
+import errno
+import traceback
 from operator import itemgetter
 
 from ansible.module_utils._text import to_native
@@ -475,16 +477,18 @@ def parsekey(module, raw_key, rank=None):
     return (key, key_type, options, comment, rank)
 
 
-def readfile(filename):
+def readfile(module, filename):
-
-    if not os.path.isfile(filename):
-        return ''
-
-    f = open(filename)
     try:
-        return f.read()
+        with open(filename, 'r') as f:
-    finally:
+            return f.read()
-        f.close()
+    except IOError as e:
+        if e.errno == errno.EACCES:
+            module.fail_json(msg="Permission denied on file or path for authorized keys file: %s" % filename,
+                             exception=traceback.format_exc())
+        elif e.errno == errno.ENOENT:
+            return ''
+        else:
+            raise
 
 
 def parsekeys(module, lines):
@@ -597,7 +601,7 @@ def enforce_state(module, params):
     # check current state -- just get the filename, don't create file
     do_write = False
     params["keyfile"] = keyfile(module, user, do_write, path, manage_dir)
-    existing_content = readfile(params["keyfile"])
+    existing_content = readfile(module, params["keyfile"])
     existing_keys = parsekeys(module, existing_content)
 
     # Add a place holder for keys that should exist in the state=present and

plugins/modules/firewalld.py

@@ -28,6 +28,11 @@ options:
       - Name of a port or port range to add/remove to/from firewalld.
       - Must be in the form PORT/PROTOCOL or PORT-PORT/PROTOCOL for port ranges.
     type: str
+  source_port:
+    description:
+      - Name of a source port or port range to add/remove to/from firewalld.
+      - Must be in the form PORT/PROTOCOL or PORT-PORT/PROTOCOL for port ranges.
+    type: str
   port_forward:
     description:
       - Port and protocol to forward using firewalld.
@@ -185,6 +190,13 @@ EXAMPLES = r'''
     permanent: true
     state: enabled
 
+- name: Permit traffic in home zone from port 20561/udp
+  ansible.posix.firewalld:
+    source_port: 20561/udp
+    zone: home
+    permanent: true
+    state: enabled
+
 - name: Permit traffic in dmz zone on http service
   ansible.posix.firewalld:
     zone: dmz
@@ -552,6 +564,43 @@ class PortTransaction(FirewallTransaction):
         self.update_fw_settings(fw_zone, fw_settings)
 
 
+class SourcePortTransaction(FirewallTransaction):
+    """
+    SourcePortTransaction
+    """
+
+    def __init__(self, module, action_args=None, zone=None, desired_state=None, permanent=False, immediate=False):
+        super(SourcePortTransaction, self).__init__(
+            module, action_args=action_args, desired_state=desired_state, zone=zone, permanent=permanent, immediate=immediate
+        )
+
+    def get_enabled_immediate(self, port, protocol, timeout):
+        if self.fw_offline:
+            dummy, fw_settings = self.get_fw_zone_settings()
+            return fw_settings.querySourcePort(port=port, protocol=protocol)
+        return self.fw.querySourcePort(zone=self.zone, port=port, protocol=protocol)
+
+    def get_enabled_permanent(self, port, protocol, timeout):
+        dummy, fw_settings = self.get_fw_zone_settings()
+        return fw_settings.querySourcePort(port=port, protocol=protocol)
+
+    def set_enabled_immediate(self, port, protocol, timeout):
+        self.fw.addSourcePort(zone=self.zone, port=port, protocol=protocol, timeout=timeout)
+
+    def set_enabled_permanent(self, port, protocol, timeout):
+        fw_zone, fw_settings = self.get_fw_zone_settings()
+        fw_settings.addSourcePort(port=port, protocol=protocol)
+        self.update_fw_settings(fw_zone, fw_settings)
+
+    def set_disabled_immediate(self, port, protocol, timeout):
+        self.fw.removeSourcePort(zone=self.zone, port=port, protocol=protocol)
+
+    def set_disabled_permanent(self, port, protocol, timeout):
+        fw_zone, fw_settings = self.get_fw_zone_settings()
+        fw_settings.removeSourcePort(port=port, protocol=protocol)
+        self.update_fw_settings(fw_zone, fw_settings)
+
+
 class InterfaceTransaction(FirewallTransaction):
     """
     InterfaceTransaction
@@ -879,6 +928,7 @@ def main():
             service=dict(type='str'),
             protocol=dict(type='str'),
             port=dict(type='str'),
+            source_port=dict(type='str'),
             port_forward=dict(type='list', elements='dict'),
             rich_rule=dict(type='str'),
             zone=dict(type='str'),
@@ -900,8 +950,8 @@ def main():
             source=('permanent',),
         ),
         mutually_exclusive=[
-            ['icmp_block', 'icmp_block_inversion', 'service', 'protocol', 'port', 'port_forward', 'rich_rule',
+            ['icmp_block', 'icmp_block_inversion', 'service', 'protocol', 'port', 'source_port', 'port_forward',
-             'interface', 'forward', 'masquerade', 'source', 'target']
+             'rich_rule', 'interface', 'forward', 'masquerade', 'source', 'target']
         ],
     )
 
@@ -957,6 +1007,17 @@ def main():
     else:
         port_protocol = None
 
+    source_port = None
+    if module.params['source_port'] is not None:
+        if '/' in module.params['source_port']:
+            source_port, source_port_protocol = module.params['source_port'].strip().split('/')
+        else:
+            source_port_protocol = None
+        if not source_port_protocol:
+            module.fail_json(msg='improper source_port format (missing protocol?)')
+    else:
+        source_port_protocol = None
+
     port_forward_toaddr = ''
     port_forward = None
     if module.params['port_forward'] is not None:
@@ -973,7 +1034,7 @@ def main():
             port_forward_toaddr = port_forward['toaddr']
 
     modification = False
-    if any([icmp_block, icmp_block_inversion, service, protocol, port, port_forward, rich_rule,
+    if any([icmp_block, icmp_block_inversion, service, protocol, port, source_port, port_forward, rich_rule,
             interface, forward, masquerade, source, target]):
         modification = True
     if modification and desired_state in ['absent', 'present'] and target is None:
@@ -1079,6 +1140,26 @@ def main():
                 )
             )
 
+    if source_port is not None:
+
+        transaction = SourcePortTransaction(
+            module,
+            action_args=(source_port, source_port_protocol, timeout),
+            zone=zone,
+            desired_state=desired_state,
+            permanent=permanent,
+            immediate=immediate,
+        )
+
+        changed, transaction_msgs = transaction.run()
+        msgs = msgs + transaction_msgs
+        if changed is True:
+            msgs.append(
+                "Changed source_port %s to %s" % (
+                    "%s/%s" % (source_port, source_port_protocol), desired_state
+                )
+            )
+
     if port_forward is not None:
         transaction = ForwardPortTransaction(
             module,

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

@@ -0,0 +1,41 @@
+---
+# -------------------------------------------------------------
+# check permissions
+
+- name: Create a file that is not accessible
+  ansible.builtin.file:
+    state: touch
+    path: "{{ output_dir | expanduser }}/file_permissions"
+    owner: root
+    mode: '0000'
+
+- name: Create unprivileged user
+  ansible.builtin.user:
+    name: nopriv
+    create_home: true
+
+- name: Try to delete a key from an unreadable file
+  become: true
+  become_user: nopriv
+  ansible.posix.authorized_key:
+    user: root
+    key: "{{ dss_key_basic }}"
+    state: absent
+    path: "{{ output_dir | expanduser }}/file_permissions"
+  register: result
+  ignore_errors: true
+
+- name: Assert that the key deletion has failed
+  ansible.builtin.assert:
+    that:
+      - result is failed
+
+- name: Remove the file
+  ansible.builtin.file:
+    state: absent
+    path: "{{ output_dir | expanduser }}/file_permissions"
+
+- name: Remove the user
+  ansible.builtin.user:
+    name: nopriv
+    state: absent

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

@@ -34,3 +34,6 @@
 
 - name: Test for specifying key as a path
   ansible.builtin.import_tasks: check_path.yml
+
+- name: Test for permission denied files
+  ansible.builtin.import_tasks: check_permissions.yml

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

@@ -21,6 +21,10 @@
 - name: Include port test cases for firewalld module
   ansible.builtin.include_tasks: port_test_cases.yml
 
+# firewalld source_port operation test cases
+- name: Include source_port test cases for firewalld module
+  ansible.builtin.include_tasks: source_port_test_cases.yml
+
 # firewalld source operation test cases
 - name: Include source test cases for firewalld module
   ansible.builtin.include_tasks: source_test_cases.yml

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

@@ -0,0 +1,107 @@
+---
+# Test playbook for the firewalld module - source_port operations
+
+- name: Firewalld source_port range test permanent enabled
+  ansible.posix.firewalld:
+    source_port: 5500-6850/tcp
+    permanent: true
+    state: enabled
+  register: result
+
+- name: Assert firewalld source_port range test permanent enabled worked
+  ansible.builtin.assert:
+    that:
+      - result is changed
+
+- name: Firewalld source_port range test permanent enabled rerun (verify not changed)
+  ansible.posix.firewalld:
+    source_port: 5500-6850/tcp
+    permanent: true
+    state: enabled
+  register: result
+
+- name: Assert firewalld source_port range test permanent enabled rerun worked (verify not changed)
+  ansible.builtin.assert:
+    that:
+      - result is not changed
+
+- name: Firewalld source_port test permanent enabled
+  ansible.posix.firewalld:
+    source_port: 6900/tcp
+    permanent: true
+    state: enabled
+  register: result
+
+- name: Assert firewalld source_port test permanent enabled worked
+  ansible.builtin.assert:
+    that:
+      - result is changed
+
+- name: Firewalld source_port test permanent enabled
+  ansible.posix.firewalld:
+    source_port: 6900/tcp
+    permanent: true
+    state: enabled
+  register: result
+
+- name: Assert firewalld source_port test permanent enabled worked
+  ansible.builtin.assert:
+    that:
+      - result is not changed
+
+- name: Firewalld source_port test disabled
+  ansible.posix.firewalld:
+    source_port: "{{ item }}"
+    permanent: true
+    state: disabled
+  loop:
+    - 6900/tcp
+    - 5500-6850/tcp
+
+- name: Firewalld source_port test permanent enabled
+  ansible.posix.firewalld:
+    source_port: 8081/tcp
+    permanent: true
+    state: enabled
+  register: result
+
+- name: Assert firewalld source_port test permanent enabled worked
+  ansible.builtin.assert:
+    that:
+      - result is changed
+
+- name: Firewalld source_port test permanent enabled rerun (verify not changed)
+  ansible.posix.firewalld:
+    source_port: 8081/tcp
+    permanent: true
+    state: enabled
+  register: result
+
+- name: Assert firewalld source_port test permanent enabled rerun worked (verify not changed)
+  ansible.builtin.assert:
+    that:
+      - result is not changed
+
+- name: Firewalld source_port test permanent disabled
+  ansible.posix.firewalld:
+    source_port: 8081/tcp
+    permanent: true
+    state: disabled
+  register: result
+
+- name: Assert firewalld source_port test permanent disabled worked
+  ansible.builtin.assert:
+    that:
+      - result is changed
+
+- name: Firewalld source_port test permanent disabled rerun (verify not changed)
+  ansible.posix.firewalld:
+    source_port: 8081/tcp
+    permanent: true
+    state: disabled
+  register: result
+
+- name: Assert firewalld source_port test permanent disabled rerun worked (verify not changed)
+  ansible.builtin.assert:
+    that:
+      - result is not changed

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

@@ -85,4 +85,4 @@
       - result is not changed
       - >
         result.msg == 'parameters are mutually exclusive:
-        icmp_block|icmp_block_inversion|service|protocol|port|port_forward|rich_rule|interface|forward|masquerade|source|target'
+        icmp_block|icmp_block_inversion|service|protocol|port|source_port|port_forward|rich_rule|interface|forward|masquerade|source|target'