Merge c4ff0545f1 into 6da1331018

Use module.warn() instead of returning warnings

SUMMARY
Returning warnings as warnings has been deprecated.
Ref: #635.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
firewalld_info
mount

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

changelogs/fragments/670-deprecations.yml

@@ -0,0 +1,3 @@
+bugfixes:
+  - "firewalld_info - stop returning warnings as return values; this has been deprecated by ansible-core (https://github.com/ansible-collections/ansible.posix/pull/670)."
+  - "mount - stop returning warnings as return values; this has been deprecated by ansible-core (https://github.com/ansible-collections/ansible.posix/pull/670)."

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',
-             'interface', 'forward', 'masquerade', 'source', 'target']
+            ['icmp_block', 'icmp_block_inversion', 'service', 'protocol', 'port', 'source_port', 'port_forward',
+             '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,

plugins/modules/firewalld_info.py

@@ -319,7 +319,6 @@ def main():
         active_zones=module.params['active_zones'],
         collected_zones=list(),
         undefined_zones=list(),
-        warnings=list(),
     )
 
     # Exit with failure message if requirements modules are not installed.

plugins/modules/mount.py

@@ -279,7 +279,7 @@ def _set_mount_save_old(module, args):
     old_lines = []
     exists = False
     changed = False
-    escaped_args = dict([(k, _escape_fstab(v)) for k, v in iteritems(args) if k != 'warnings'])
+    escaped_args = dict([(k, _escape_fstab(v)) for k, v in iteritems(args)])
     new_line = '%(src)s %(name)s %(fstype)s %(opts)s %(dump)s %(passno)s\n'
 
     if platform.system() == 'SunOS':
@@ -804,7 +804,6 @@ def main():
             passno='-',
             fstab=module.params['fstab'],
             boot='yes' if module.params['boot'] else 'no',
-            warnings=[]
         )
         if args['fstab'] is None:
             args['fstab'] = '/etc/vfstab'
@@ -816,7 +815,6 @@ def main():
             passno='0',
             fstab=module.params['fstab'],
             boot='yes',
-            warnings=[]
         )
         if args['fstab'] is None:
             args['fstab'] = '/etc/fstab'
@@ -834,8 +832,7 @@ def main():
         linux_mounts = get_linux_mounts(module)
 
         if linux_mounts is None:
-            args['warnings'].append('Cannot open file /proc/self/mountinfo.'
-                                    ' Bind mounts might be misinterpreted.')
+            module.warn('Cannot open file /proc/self/mountinfo. Bind mounts might be misinterpreted.')
 
     # Override defaults with user specified params
     for key in ('src', 'fstype', 'passno', 'opts', 'dump', 'fstab'):
@@ -847,7 +844,7 @@ def main():
         # specified in 'opts',  mount module will ignore 'boot'.
         opts = args['opts'].split(',')
         if module.params['boot'] and 'noauto' in opts:
-            args['warnings'].append("Ignore the 'boot' due to 'opts' contains 'noauto'.")
+            module.warn("Ignore the 'boot' due to 'opts' contains 'noauto'.")
         elif not module.params['boot']:
             args['boot'] = 'no'
             opts.append('noauto')

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'