proxmox_firewall: Fix minor bugs and sanity issues

Author: JanaHoch

plugins/module_utils/proxmox.py

@@ -102,20 +102,20 @@ def compare_list_of_dicts(existing_list, new_list, uid, params_to_ignore=None):
 
     common_uids = set(existing_list.keys()).intersection(set(new_list.keys()))
     missing_uids = set(new_list.keys()) - set(existing_list.keys())
-    items_to_create = [new_list[id] for id in missing_uids]
+    items_to_create = [new_list[uid] for uid in missing_uids]
 
-    for id in common_uids:
+    for uid in common_uids:
         # If new rule has a parameter that is not present in existing rule we need to update
-        if set(new_list[id].keys()) - set(existing_list[id].keys()) != set():
-            items_to_update.append(new_list[id])
+        if set(new_list[uid].keys()) - set(existing_list[uid].keys()) != set():
+            items_to_update.append(new_list[uid])
             continue
 
         # If existing rule param value doesn't match new rule param OR
         # If existing rule has a param that is not present in new rule except for params in params_to_ignore
-        for existing_rule_param, existing_parm_value in existing_list[id].items():
+        for existing_rule_param, existing_parm_value in existing_list[uid].items():
             if (existing_rule_param not in params_to_ignore and
-                    new_list[id].get(existing_rule_param) != existing_parm_value):
-                items_to_update.append(new_list[id])
+                    new_list[uid].get(existing_rule_param) != existing_parm_value):
+                items_to_update.append(new_list[uid])
 
     return items_to_create, items_to_update
 

plugins/module_utils/proxmox_sdn.py

@@ -113,7 +113,7 @@ def get_aliases(self, firewall_obj):
             return firewall_obj().aliases().get()
         except Exception as e:
             self.module.fail_json(
-                msg='Failed to retrieve aliases'
+                msg=f'Failed to retrieve aliases - {e}'
             )
 
     def get_fw_rules(self, rules_obj, pos=None):

plugins/modules/proxmox_firewall.py

@@ -14,8 +14,9 @@
 short_description: Manage firewall rules in Proxmox
 version_added: "1.4.0"
 description:
-    - Create/update/delete firewall rules at cluster/group/vnet/node/vm level. 
-    - Create/delete firewall security groups.
+    - create/update/delete FW rules at cluster/group/vnet/node/vm level
+    - Create/delete firewall security groups
+    - Create/delete aliases
 author: 'Jana Hoch <[email protected]> (!UNKNOWN)'
 attributes:
   check_mode:

plugins/modules/proxmox_firewall_info.py

@@ -15,6 +15,8 @@
 version_added: "1.4.0"
 description:
     - Get firewall rules at cluster/group/vnet/node/vm level.
+    - Get firewall security groups at cluster level.
+    - Get aliases at cluster/VM level.
 author: 'Jana Hoch <[email protected]> (!UNKNOWN)'
 options:
   level:

tests/unit/plugins/modules/test_proxmox_firewall.py

@@ -123,8 +123,8 @@ def setUp(self):
         self.connect_mock = patch(
             "ansible_collections.community.proxmox.plugins.module_utils.proxmox.ProxmoxAnsible._connect",
         ).start()
-        self.connect_mock.return_value.cluster.return_value.firewall.return_value.rules.get.return_value = RAW_FIREWALL_RULES
-        self.connect_mock.return_value.cluster.return_value.firewall.return_value.groups.return_value.get.return_value = RAW_GROUPS
+        self.connect_mock.return_value.cluster.return_value.firewall.return_value.rules.get.return_value = RAW_FIREWALL_RULES.copy()
+        self.connect_mock.return_value.cluster.return_value.firewall.return_value.groups.return_value.get.return_value = RAW_GROUPS.copy()
 
     def tearDown(self):
         self.connect_mock.stop()

tests/unit/plugins/modules/test_proxmox_firewall_info.py

@@ -147,12 +147,12 @@ def setUp(self):
         mock_cluster_fw = self.connect_mock.return_value.cluster.return_value.firewall.return_value
         mock_vm100_fw = self.connect_mock.return_value.nodes.return_value.return_value.return_value.firewall.return_value
 
-        mock_cluster_fw.rules.get.return_value = RAW_FIREWALL_RULES
-        mock_cluster_fw.groups.return_value.get.return_value = RAW_GROUPS
-        mock_cluster_fw.aliases.return_value.get.return_value = RAW_ALIASES
+        mock_cluster_fw.rules.get.return_value = RAW_FIREWALL_RULES.copy()
+        mock_cluster_fw.groups.return_value.get.return_value = RAW_GROUPS.copy()
+        mock_cluster_fw.aliases.return_value.get.return_value = RAW_ALIASES.copy()
 
-        mock_vm100_fw.rules.get.return_value = RAW_FIREWALL_RULES
-        mock_vm100_fw.aliases.return_value.get.return_value = RAW_ALIASES
+        mock_vm100_fw.rules.get.return_value = RAW_FIREWALL_RULES.copy()
+        mock_vm100_fw.aliases.return_value.get.return_value = RAW_ALIASES.copy()
 
     def tearDown(self):
         self.connect_mock.stop()