[ufw] Insert or delete biased when deletion enabled - as for append or delete. (#3514)

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

* [ufw] Insert or delete biased when deletion enabled - as for append or delete.

changelogs/fragments/3514-ufw_insert_or_delete_biased_when_deletion_enabled.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - "ufw - if ``delete=true`` and ``insert`` option is present, then ``insert`` is now ignored rather than failing with a syntax error (https://github.com/ansible-collections/community.general/pull/3514)."

plugins/modules/system/ufw.py

@@ -54,6 +54,8 @@ options:
     description:
       - Insert the corresponding rule as rule number NUM.
       - Note that ufw numbers rules starting with 1.
+      - If I(delete=true) and a value is provided for I(insert),
+        then I(insert) is ignored.
     type: int
   insert_relative_to:
     description:
@@ -120,6 +122,8 @@ options:
   delete:
     description:
       - Delete rule.
+      - If I(delete=true) and a value is provided for I(insert),
+        then I(insert) is ignored.
     type: bool
     default: false
   interface:
@@ -511,12 +515,12 @@ def main():
                                  'interface_in and interface_out')
             # Rules are constructed according to the long format
             #
-            # ufw [--dry-run] [route] [delete] [insert NUM] allow|deny|reject|limit [in|out on INTERFACE] [log|log-all] \
+            # ufw [--dry-run] [route] [delete | insert NUM] allow|deny|reject|limit [in|out on INTERFACE] [log|log-all] \
             #     [from ADDRESS [port PORT]] [to ADDRESS [port PORT]] \
             #     [proto protocol] [app application] [comment COMMENT]
             cmd.append([module.boolean(params['route']), 'route'])
             cmd.append([module.boolean(params['delete']), 'delete'])
-            if params['insert'] is not None:
+            if params['insert'] is not None and not params['delete']:
                 relative_to_cmd = params['insert_relative_to']
                 if relative_to_cmd == 'zero':
                     insert_to = params['insert']

tests/unit/plugins/modules/system/test_ufw.py

@@ -54,6 +54,7 @@ dry_mode_cmd_with_port_700 = {
     "ufw status verbose": ufw_status_verbose_with_port_7000,
     "ufw --version": ufw_version_35,
     "ufw --dry-run allow from any to any port 7000 proto tcp": skippg_adding_existing_rules,
+    "ufw --dry-run insert 1 allow from any to any port 7000 proto tcp": skippg_adding_existing_rules,
     "ufw --dry-run delete allow from any to any port 7000 proto tcp": "",
     "ufw --dry-run delete allow from any to any port 7001 proto tcp": user_rules_with_port_7000,
     "ufw --dry-run route allow in on foo out on bar from 1.1.1.1 port 7000 to 8.8.8.8 port 7001 proto tcp": "",
@@ -178,6 +179,17 @@ class TestUFW(unittest.TestCase):
         result = self.__getResult(do_nothing_func_port_7000)
         self.assertFalse(result.exception.args[0]['changed'])
 
+    def test_check_mode_add_insert_rules(self):
+        set_module_args({
+            'insert': '1',
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7000',
+            '_ansible_check_mode': True
+        })
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertFalse(result.exception.args[0]['changed'])
+
     def test_check_mode_add_detailed_route(self):
         set_module_args({
             'rule': 'allow',
@@ -318,6 +330,19 @@ class TestUFW(unittest.TestCase):
 
         self.assertTrue(self.__getResult(do_nothing_func_port_7000).exception.args[0]['changed'])
 
+    def test_check_mode_delete_existing_insert_rules(self):
+
+        set_module_args({
+            'insert': '1',
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7000',
+            'delete': 'yes',
+            '_ansible_check_mode': True,
+        })
+
+        self.assertTrue(self.__getResult(do_nothing_func_port_7000).exception.args[0]['changed'])
+
     def test_check_mode_delete_not_existing_rules(self):
 
         set_module_args({
@@ -330,6 +355,19 @@ class TestUFW(unittest.TestCase):
 
         self.assertFalse(self.__getResult(do_nothing_func_port_7000).exception.args[0]['changed'])
 
+    def test_check_mode_delete_not_existing_insert_rules(self):
+
+        set_module_args({
+            'insert': '1',
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7001',
+            'delete': 'yes',
+            '_ansible_check_mode': True,
+        })
+
+        self.assertFalse(self.__getResult(do_nothing_func_port_7000).exception.args[0]['changed'])
+
     def test_enable_mode(self):
         set_module_args({
             'state': 'enabled',