diff --git a/changelogs/fragments/3514-ufw_insert_or_delete_biased_when_deletion_enabled.yml b/changelogs/fragments/3514-ufw_insert_or_delete_biased_when_deletion_enabled.yml new file mode 100644 index 0000000000..93c1bf96d5 --- /dev/null +++ b/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)." diff --git a/plugins/modules/system/ufw.py b/plugins/modules/system/ufw.py index 465df6adb5..b6e0b3de78 100644 --- a/plugins/modules/system/ufw.py +++ b/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'] diff --git a/tests/unit/plugins/modules/system/test_ufw.py b/tests/unit/plugins/modules/system/test_ufw.py index 44882e0e93..a522ba98a8 100644 --- a/tests/unit/plugins/modules/system/test_ufw.py +++ b/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',