From b5208e0176ff19ee5c0e6cebe079bb00f4cafafe Mon Sep 17 00:00:00 2001 From: AnsibleGuy Date: Wed, 13 Mar 2024 22:11:41 +0100 Subject: [PATCH] refactoring api-handling (#51) --- plugins/module_utils/base/base.py | 28 ++++++++++++++------ plugins/module_utils/helper/rule.py | 4 +-- plugins/module_utils/main/ids_policy_rule.py | 3 +-- plugins/module_utils/main/ids_user_rule.py | 2 -- plugins/module_utils/main/ipsec_pool.py | 5 ++-- plugins/module_utils/main/ipsec_vti.py | 5 ++-- tests/list.yml | 17 ++++++++++++ 7 files changed, 44 insertions(+), 20 deletions(-) diff --git a/plugins/module_utils/base/base.py b/plugins/module_utils/base/base.py index dcb10c95..35d61ef9 100644 --- a/plugins/module_utils/base/base.py +++ b/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): diff --git a/plugins/module_utils/helper/rule.py b/plugins/module_utils/helper/rule.py index 4ba2ef93..d10aaa8e 100644 --- a/plugins/module_utils/helper/rule.py +++ b/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!" ) diff --git a/plugins/module_utils/main/ids_policy_rule.py b/plugins/module_utils/main/ids_policy_rule.py index b90ec4e2..d24cd0bb 100644 --- a/plugins/module_utils/main/ids_policy_rule.py +++ b/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'], diff --git a/plugins/module_utils/main/ids_user_rule.py b/plugins/module_utils/main/ids_user_rule.py index b0bdec92..083f7f2c 100644 --- a/plugins/module_utils/main/ids_user_rule.py +++ b/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'], diff --git a/plugins/module_utils/main/ipsec_pool.py b/plugins/module_utils/main/ipsec_pool.py index 37a518cd..da2c7b41 100644 --- a/plugins/module_utils/main/ipsec_pool.py +++ b/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] diff --git a/plugins/module_utils/main/ipsec_vti.py b/plugins/module_utils/main/ipsec_vti.py index 57f91bc4..cd539db0 100644 --- a/plugins/module_utils/main/ipsec_vti.py +++ b/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', diff --git a/tests/list.yml b/tests/list.yml index 163eedbf..f8b6b3d5 100644 --- a/tests/list.yml +++ b/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', + ]