refactoring api-handling (#51)

plugins/module_utils/base/base.py

@@ -62,7 +62,7 @@ def __init__(self, instance):
             if not hasattr(self.i, attr):
                 exit_bug(f"Module has no '{attr}' attribute set!")
 
-    def search(self) -> (dict, list):
+    def search(self, match_fields: list = None) -> (dict, list):
         # workaround if 'get' needs to be performed using other api module/controller
         cont_get, mod_get = self.i.API_CONT, self.i.API_MOD
 
@@ -81,21 +81,33 @@ def search(self) -> (dict, list):
                 exit_bug("To use the 'search' commands you need to also define the related 'detail' (get) command!")
 
             data = []
+            # if we can - we only perform the 'detail' call for the already matched entry to save on needed requests
+            base_match_fields = False
+            base_match_fields_checked = False
 
             for base_entry in self._api_post({
                 **self.i.call_cnf,
                 'command': self.i.CMDS['search'],
                 'data': {'current': 1, 'rowCount': self.QUERY_MAX_ENTRIES},
             })['rows']:
+                if match_fields is not None and not base_match_fields_checked:
+                    base_match_fields_checked = True
+                    base_match_fields = all(field in base_entry for field in match_fields)
+
                 # todo: perform async calls for parallel data fetching
-                data.append({
-                    **self._search_path_handling(
+                detail_entry = {}
+                if not base_match_fields or \
+                        all(base_entry[field] == self.i.p[field] for field in match_fields):
+                    detail_entry = self._search_path_handling(
                         self._api_get({
                             **self.i.call_cnf,
                             'command': self.i.CMDS['detail'],
                             'params': [base_entry[self.field_pk]]
                         })
-                    ),
+                    )
+
+                data.append({
+                    **detail_entry,
                     **base_entry,
                 })
                 if self.raw is None:
@@ -111,7 +123,7 @@ def search(self) -> (dict, list):
 
             return data
 
-        # legacy api handling (fewer requests needed)
+        # legacy api handling (fewer requests needed; much simpler client-side handling)
         data = self._api_get({
             **self.i.call_cnf,
             'command': self.i.CMDS['search'],
@@ -158,7 +170,7 @@ def get_existing(self, diff_filter: bool = False) -> list:
 
     def find(self, match_fields: list) -> None:
         if self.i.existing_entries is None:
-            self.i.existing_entries = self._call_search()
+            self.i.existing_entries = self._call_search(match_fields)
 
         match = get_matching(
             module=self.i.m, existing_items=self.i.existing_entries,
@@ -647,14 +659,14 @@ def _call_simple(self) -> Callable:
 
         return self.simplify_existing
 
-    def _call_search(self) -> (list, dict):
+    def _call_search(self, match_fields: list = None) -> (list, dict):
         if hasattr(self.i, '_search_call'):
             return self.i._search_call()
 
         if hasattr(self.i, 'search_call'):
             return self.i.search_call()
 
-        return self.search()
+        return self.search(match_fields)
 
     def _api_headers(self) -> dict:
         if hasattr(self.i, self.ATTR_HEADERS):

plugins/module_utils/helper/rule.py

@@ -31,13 +31,13 @@ def validate_values(error_func, module: AnsibleModule, cnf: dict) -> None:
         if cnf['source_net'] == 'any' and cnf['destination_net'] == 'any':
             module.warn(
                 "Configuring allow-rules with 'any' source and "
-                "'any' destination is bad practise!"
+                "'any' destination is bad practice!"
             )
 
         elif cnf['destination_net'] == 'any' and cnf['destination_port'] == 'any':
             module.warn(
                 "Configuring allow-rules to 'any' destination "
-                "using 'all' ports is bad practise!"
+                "using 'all' ports is bad practice!"
             )
 
 

plugins/module_utils/main/ids_policy_rule.py

@@ -14,6 +14,7 @@ class Rule(BaseModule):
         'del': 'delPolicyRule',
         'set': 'setPolicyRule',
         'search': 'searchPolicyRule',
+        'detail': 'getPolicyRule',
         'toggle': 'togglePolicyRule',
     }
     API_KEY_PATH = 'policies.rule'
@@ -43,8 +44,6 @@ def check(self) -> None:
         self.r['changed'] = self.r['diff']['before'] != self.r['diff']['after']
 
     def _search_call(self) -> list:
-        # NOTE: workaround for issue with incomplete response-data from 'get' endpoint:
-        #   https://github.com/opnsense/core/issues/7094
         existing = self.s.post(cnf={
             **self.call_cnf,
             'command': self.CMDS['search'],

plugins/module_utils/main/ids_user_rule.py

@@ -49,8 +49,6 @@ def get_existing(self) -> list:
         return self._search_call()
 
     def _search_call(self) -> list:
-        # NOTE: workaround for issue with incomplete response-data from 'get' endpoint:
-        #   https://github.com/opnsense/core/issues/7094
         existing = self.s.post(cnf={
             **self.call_cnf,
             'command': self.CMDS['search'],

plugins/module_utils/main/ipsec_pool.py

@@ -13,15 +13,14 @@ class Pool(BaseModule):
         'add': 'add',
         'del': 'del',
         'set': 'set',
-        'search': 'get',
+        'search': 'search',
+        'detail': 'get',
         'toggle': 'toggle',
     }
     API_KEY_PATH = 'pool'
-    API_KEY_PATH_GET = 'swanctl.Pools.Pool'
     API_MOD = 'ipsec'
     API_CONT = 'pools'
     API_CONT_REL = 'service'
-    API_CONT_GET = 'connections'
     API_CMD_REL = 'reconfigure'
     FIELDS_CHANGE = ['network']
     FIELDS_ALL = ['enabled', FIELD_ID]

plugins/module_utils/main/ipsec_vti.py

@@ -13,15 +13,14 @@ class Vti(BaseModule):
         'add': 'add',
         'del': 'del',
         'set': 'set',
-        'search': 'get',
+        'search': 'search',
+        'detail': 'get',
         'toggle': 'toggle',
     }
     API_KEY_PATH = 'vti'
-    API_KEY_PATH_GET = 'swanctl.VTIs.VTI'
     API_MOD = 'ipsec'
     API_CONT = 'vti'
     API_CONT_REL = 'service'
-    API_CONT_GET = 'connections'
     API_CMD_REL = 'reconfigure'
     FIELDS_CHANGE = [
         'request_id', 'local_address', 'remote_address',

tests/list.yml

@@ -80,3 +80,20 @@
         'ipsec_cert', 'ipsec_connection', 'ipsec_child', 'ipsec_pool',
         'ipsec_auth_local', 'ipsec_auth_remote', 'ipsec_vti',
       ]
+
+    - name: Querying config - IDS
+      ansibleguy.opnsense.list:
+        target: "{{ item }}"
+      when: not ansible_check_mode
+      loop: [
+        'ids_general', 'ids_policy', 'ids_rule', 'ids_ruleset', 'ids_user_rule',
+        'ids_policy_rule',
+      ]
+
+    - name: Querying config - OpenVPN
+      ansibleguy.opnsense.list:
+        target: "{{ item }}"
+      when: not ansible_check_mode
+      loop: [
+        'openvpn_instance', 'openvpn_static_key',
+      ]