From 21d5b6f9c9fa2920d6bf17ab0923a939df393794 Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Wed, 11 Oct 2023 19:40:27 +0100 Subject: [PATCH 01/13] Fix(cv_facts_v3): return authorization error when svc token expires --- .../cvp/plugins/module_utils/facts_tools.py | 17 +++++++++++++++-- .../arista/cvp/plugins/module_utils/tools_cv.py | 16 ++++++++++++++++ .../arista/cvp/plugins/modules/cv_facts_v3.py | 2 +- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py index eb6999363..ee7935716 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py @@ -24,11 +24,13 @@ import re import os from typing import List +from ansible.module_utils.basic import AnsibleModule from concurrent.futures import ThreadPoolExecutor import ansible_collections.arista.cvp.plugins.module_utils.logger # noqa # pylint: disable=unused-import from ansible_collections.arista.cvp.plugins.module_utils.resources.api.fields import Api from ansible_collections.arista.cvp.plugins.module_utils.resources.modules.fields import FactsResponseFields import ansible_collections.arista.cvp.plugins.module_utils.tools_schema as schema # noqa # pylint: disable=unused-import +from ansible_collections.arista.cvp.plugins.module_utils.tools_cv import authorization_error try: from cvprac.cvp_client import CvpClient # noqa # pylint: disable=unused-import from cvprac.cvp_client_errors import CvpApiError, CvpRequestError # noqa # pylint: disable=unused-import @@ -322,8 +324,9 @@ class CvFactsTools(): CvFactsTools Object to operate Facts from Cloudvision """ - def __init__(self, cv_connection): + def __init__(self, cv_connection, ansible_module: AnsibleModule = None): self.__cv_client = cv_connection + self.__ansible = ansible_module self._cache = {FactsResponseFields.CACHE_CONTAINERS: None, FactsResponseFields.CACHE_MAPPERS: None} self._max_worker = min(32, (os.cpu_count() or 1) + 4) self.__init_facts() @@ -592,6 +595,7 @@ def __fact_devices(self, filter: str = '.*', verbose: str = 'short'): cv_devices = self.__cv_client.api.get_inventory() except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting devices facts: %s', str(error_msg)) + authorization_error(self.__ansible.fail_json, error_msg) MODULE_LOGGER.info('Extract device data using filter %s', str(filter)) facts_builder = CvFactResource() for device in cv_devices: @@ -614,6 +618,7 @@ def __fact_containers(self, filter: str = '.*'): cv_containers = self.__cv_client.api.get_containers() except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting containers facts: %s', str(error_msg)) + authorization_error(self.__ansible.fail_json, error_msg) facts_builder = CvFactResource() for container in cv_containers['data']: if container[Api.generic.NAME] != 'Tenant': @@ -637,7 +642,11 @@ def __fact_configlets(self, filter: str = '.*', configlets_per_call: int = 10): configlets_per_call : int, optional Number of configlets to retrieve per API call, by default 10 """ - max_range_calc = self.__cv_client.api.get_configlets(start=0, end=1)['total'] + 1 + try: + max_range_calc = self.__cv_client.api.get_configlets(start=0, end=1)['total'] + 1 + except CvpApiError as error_msg: + MODULE_LOGGER.error('Error when collecting configlets facts: %s', str(error_msg)) + authorization_error(self.__ansible.fail_json, error_msg) futures_list = [] results = [] with ThreadPoolExecutor(max_workers=self._max_worker) as executor: @@ -678,6 +687,7 @@ def __fact_images(self, filter: str = '.*'): cv_images = self.__cv_client.api.get_images() except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting images facts: %s', str(error_msg)) + authorization_error(self.__ansible.fail_json, error_msg) facts_builder = CvFactResource() for image in cv_images['data']: @@ -710,18 +720,21 @@ def __fact_tasks(self, filter: str = '.*', verbose: str = 'short'): cv_tasks = cv_tasks['data'] except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting task facts: %s', str(error_msg)) + authorization_error(self.__ansible.fail_json, error_msg) elif isinstance(filter, str): # filter by task status try: cv_tasks = self.__cv_client.api.get_tasks_by_status(filter) except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting %s task facts: %s', filter, str(error_msg)) + authorization_error(self.__ansible.fail_json, error_msg) elif isinstance(filter, int): # filter by task_id try: cv_tasks = self.__cv_client.api.get_task_by_id(filter) except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting %s task facts: %s', filter, str(error_msg)) + authorization_error(self.__ansible.fail_json, error_msg) for task in cv_tasks: MODULE_LOGGER.debug('Got following information for task: %s', str(task)) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py b/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py index 614dd4381..b5a36b21b 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py @@ -207,3 +207,19 @@ def cv_update_configlets_on_device(module, device_facts, add_configlets, del_con LOGGER.info(" * cv_update_configlets_on_device - device_addition result: %s", str(device_addition)) LOGGER.info(" * cv_update_configlets_on_device - final result: %s", str(response)) return response + + +def authorization_error(module, error_msg): + """ + Function to handle authorization error and fail module with correct message. + + Parameters + ---------- + module : AnsibleModule + Ansible module information + error : str + Error message to check if it is an authorization error or not. + + """ + if "Unauthorized" in str(error_msg): + module(msg="Unauthorized access to CloudVision. Please check your credentials or service account token validty!") diff --git a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py index 470353f96..107f36647 100644 --- a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py +++ b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py @@ -183,7 +183,7 @@ def main(): cv_client = tools_cv.cv_connect(ansible_module) # Instantiate ansible results - facts_collector = CvFactsTools(cv_connection=cv_client) + facts_collector = CvFactsTools(cv_connection=cv_client, ansible_module=ansible_module) facts = facts_collector.facts(scope=ansible_module.params['facts'], regex_filter=ansible_module.params['regexp_filter'], verbose=ansible_module.params['verbose']) result = dict(changed=False, data=facts, failed=False) From 17fe83710958c350e98f04f2e5c884cdd635cff0 Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Fri, 13 Oct 2023 14:45:07 +0100 Subject: [PATCH 02/13] reverting previous changes and raising errors instead --- .../cvp/plugins/module_utils/facts_tools.py | 19 ++++++++----------- .../cvp/plugins/module_utils/tools_cv.py | 16 ---------------- .../arista/cvp/plugins/modules/cv_facts_v3.py | 9 ++++++--- 3 files changed, 14 insertions(+), 30 deletions(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py index ee7935716..1fd7d2a30 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/facts_tools.py @@ -24,13 +24,11 @@ import re import os from typing import List -from ansible.module_utils.basic import AnsibleModule from concurrent.futures import ThreadPoolExecutor import ansible_collections.arista.cvp.plugins.module_utils.logger # noqa # pylint: disable=unused-import from ansible_collections.arista.cvp.plugins.module_utils.resources.api.fields import Api from ansible_collections.arista.cvp.plugins.module_utils.resources.modules.fields import FactsResponseFields import ansible_collections.arista.cvp.plugins.module_utils.tools_schema as schema # noqa # pylint: disable=unused-import -from ansible_collections.arista.cvp.plugins.module_utils.tools_cv import authorization_error try: from cvprac.cvp_client import CvpClient # noqa # pylint: disable=unused-import from cvprac.cvp_client_errors import CvpApiError, CvpRequestError # noqa # pylint: disable=unused-import @@ -324,9 +322,8 @@ class CvFactsTools(): CvFactsTools Object to operate Facts from Cloudvision """ - def __init__(self, cv_connection, ansible_module: AnsibleModule = None): + def __init__(self, cv_connection): self.__cv_client = cv_connection - self.__ansible = ansible_module self._cache = {FactsResponseFields.CACHE_CONTAINERS: None, FactsResponseFields.CACHE_MAPPERS: None} self._max_worker = min(32, (os.cpu_count() or 1) + 4) self.__init_facts() @@ -595,7 +592,7 @@ def __fact_devices(self, filter: str = '.*', verbose: str = 'short'): cv_devices = self.__cv_client.api.get_inventory() except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting devices facts: %s', str(error_msg)) - authorization_error(self.__ansible.fail_json, error_msg) + raise error_msg MODULE_LOGGER.info('Extract device data using filter %s', str(filter)) facts_builder = CvFactResource() for device in cv_devices: @@ -618,7 +615,7 @@ def __fact_containers(self, filter: str = '.*'): cv_containers = self.__cv_client.api.get_containers() except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting containers facts: %s', str(error_msg)) - authorization_error(self.__ansible.fail_json, error_msg) + raise error_msg facts_builder = CvFactResource() for container in cv_containers['data']: if container[Api.generic.NAME] != 'Tenant': @@ -646,7 +643,7 @@ def __fact_configlets(self, filter: str = '.*', configlets_per_call: int = 10): max_range_calc = self.__cv_client.api.get_configlets(start=0, end=1)['total'] + 1 except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting configlets facts: %s', str(error_msg)) - authorization_error(self.__ansible.fail_json, error_msg) + raise error_msg futures_list = [] results = [] with ThreadPoolExecutor(max_workers=self._max_worker) as executor: @@ -687,7 +684,7 @@ def __fact_images(self, filter: str = '.*'): cv_images = self.__cv_client.api.get_images() except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting images facts: %s', str(error_msg)) - authorization_error(self.__ansible.fail_json, error_msg) + raise error_msg facts_builder = CvFactResource() for image in cv_images['data']: @@ -720,21 +717,21 @@ def __fact_tasks(self, filter: str = '.*', verbose: str = 'short'): cv_tasks = cv_tasks['data'] except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting task facts: %s', str(error_msg)) - authorization_error(self.__ansible.fail_json, error_msg) + raise error_msg elif isinstance(filter, str): # filter by task status try: cv_tasks = self.__cv_client.api.get_tasks_by_status(filter) except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting %s task facts: %s', filter, str(error_msg)) - authorization_error(self.__ansible.fail_json, error_msg) + raise error_msg elif isinstance(filter, int): # filter by task_id try: cv_tasks = self.__cv_client.api.get_task_by_id(filter) except CvpApiError as error_msg: MODULE_LOGGER.error('Error when collecting %s task facts: %s', filter, str(error_msg)) - authorization_error(self.__ansible.fail_json, error_msg) + raise error_msg for task in cv_tasks: MODULE_LOGGER.debug('Got following information for task: %s', str(task)) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py b/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py index b5a36b21b..614dd4381 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/tools_cv.py @@ -207,19 +207,3 @@ def cv_update_configlets_on_device(module, device_facts, add_configlets, del_con LOGGER.info(" * cv_update_configlets_on_device - device_addition result: %s", str(device_addition)) LOGGER.info(" * cv_update_configlets_on_device - final result: %s", str(response)) return response - - -def authorization_error(module, error_msg): - """ - Function to handle authorization error and fail module with correct message. - - Parameters - ---------- - module : AnsibleModule - Ansible module information - error : str - Error message to check if it is an authorization error or not. - - """ - if "Unauthorized" in str(error_msg): - module(msg="Unauthorized access to CloudVision. Please check your credentials or service account token validty!") diff --git a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py index 107f36647..810711497 100644 --- a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py +++ b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py @@ -183,9 +183,12 @@ def main(): cv_client = tools_cv.cv_connect(ansible_module) # Instantiate ansible results - facts_collector = CvFactsTools(cv_connection=cv_client, ansible_module=ansible_module) - facts = facts_collector.facts(scope=ansible_module.params['facts'], regex_filter=ansible_module.params['regexp_filter'], - verbose=ansible_module.params['verbose']) + facts_collector = CvFactsTools(cv_connection=cv_client) + try: + facts = facts_collector.facts(scope=ansible_module.params['facts'], regex_filter=ansible_module.params['regexp_filter'], + verbose=ansible_module.params['verbose']) + except Exception as e: + ansible_module.fail_json(msg=str(e)) result = dict(changed=False, data=facts, failed=False) # Implement logic From 524fa269b61c186451d5c50be5c3e645c5ab357f Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Mon, 16 Oct 2023 23:43:51 +0100 Subject: [PATCH 03/13] changing exception to CvpClientError --- ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py index 810711497..9fc54b4d0 100644 --- a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py +++ b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py @@ -187,7 +187,7 @@ def main(): try: facts = facts_collector.facts(scope=ansible_module.params['facts'], regex_filter=ansible_module.params['regexp_filter'], verbose=ansible_module.params['verbose']) - except Exception as e: + except CvpClientError as e: ansible_module.fail_json(msg=str(e)) result = dict(changed=False, data=facts, failed=False) From b14d714e0ce40e4be0f9d5abb92acb559d3a33c2 Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Fri, 27 Oct 2023 02:12:56 +0100 Subject: [PATCH 04/13] fixing ansible lint issues --- ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py index 9fc54b4d0..36760569b 100644 --- a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py +++ b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py @@ -185,10 +185,10 @@ def main(): # Instantiate ansible results facts_collector = CvFactsTools(cv_connection=cv_client) try: - facts = facts_collector.facts(scope=ansible_module.params['facts'], regex_filter=ansible_module.params['regexp_filter'], + facts = facts_collector.facts(scope=ansible_module.params['facts'], regex_filter=ansible_module.params['regexp_filter'], verbose=ansible_module.params['verbose']) except CvpClientError as e: - ansible_module.fail_json(msg=str(e)) + ansible_module.fail_json(msg=str(e)) result = dict(changed=False, data=facts, failed=False) # Implement logic From 68b8726523894b2830fbf373b43dd5fc1ac53e07 Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Fri, 27 Oct 2023 02:29:30 +0100 Subject: [PATCH 05/13] fixing ansible lint issues --- ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py index 36760569b..1ac749bdc 100644 --- a/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py +++ b/ansible_collections/arista/cvp/plugins/modules/cv_facts_v3.py @@ -186,7 +186,7 @@ def main(): facts_collector = CvFactsTools(cv_connection=cv_client) try: facts = facts_collector.facts(scope=ansible_module.params['facts'], regex_filter=ansible_module.params['regexp_filter'], - verbose=ansible_module.params['verbose']) + verbose=ansible_module.params['verbose']) except CvpClientError as e: ansible_module.fail_json(msg=str(e)) result = dict(changed=False, data=facts, failed=False) From 03cd0a0ac05c248d5fe587215b8e72d435f736aa Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Sat, 11 Nov 2023 02:53:31 +0000 Subject: [PATCH 06/13] adding CvpRequestError 403 handling for device tools --- .../cvp/plugins/module_utils/device_tools.py | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py index 419a5df55..d70e38bfd 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py @@ -1546,6 +1546,10 @@ def move_device(self, user_inventory: DeviceInventory): error_message = f"Error to move device {device.fqdn} to container {device.container}" MODULE_LOGGER.error(error_message) self.__ansible.fail_json(msg=error_message) + except CvpRequestError: + error_message = f"Move device {device.fqdn} to container {device.container} failed. User is unauthorized!" + MODULE_LOGGER.error(error_message) + self.__ansible.fail_json(msg=error_message) else: if resp and resp['data']['status'] == 'success': result_data.changed = True @@ -1924,6 +1928,11 @@ def apply_configlets(self, user_inventory: DeviceInventory): self.__ansible.fail_json( msg="Error applying configlets to device" ) + except CvpRequestError: + MODULE_LOGGER.error("Error applying configlets to device. User is unauthorized!") + self.__ansible.fail_json( + msg="Error applying configlets to device. User is unauthorized!" + ) else: if resp["data"]["status"] == "success": result_data.changed = True @@ -2014,6 +2023,11 @@ def detach_configlets(self, user_inventory: DeviceInventory): self.__ansible.fail_json( msg=f"Error detaching configlets from device {device.fqdn}: {catch_error}" ) + except CvpRequestError: + MODULE_LOGGER.error("Error detaching configlets to device. User is unauthorized!") + self.__ansible.fail_json( + msg="Error detaching configlets to device. User is unauthorized!" + ) else: if resp["data"]["status"] == "success": result_data.changed = True @@ -2213,6 +2227,11 @@ def delete_device(self, user_inventory: DeviceInventory): self.__ansible.fail_json( msg="Error removing device from provisioning" ) + except CvpRequestError: + MODULE_LOGGER.error("Removing device from provisioning failed. User is unauthorized!") + self.__ansible.fail_json( + msg="Removing device from provisioning failed. User is unauthorized!" + ) else: if resp["result"] == "success": result_data.changed = True @@ -2251,8 +2270,11 @@ def decommission_device(self, user_inventory: DeviceInventory): except CvpApiError: MODULE_LOGGER.error("Error decommissioning device") self.__ansible.fail_json(msg="Error decommissioning device") - except CvpRequestError: - request_err_msg = f"Device with {device_id} does not exist or is not registered to decommission" + except CvpRequestError as e: + if "403 Forbidden" in e.msg: + request_err_msg = f"User is unauthorized to decommission device {device_id}" + else: + request_err_msg = f"Device with {device_id} does not exist or is not registered to decommission" MODULE_LOGGER.error(request_err_msg) self.__ansible.fail_json(msg=request_err_msg) else: @@ -2315,6 +2337,9 @@ def reset_device(self, user_inventory: DeviceInventory): except CvpApiError: MODULE_LOGGER.error("Error resetting device") self.__ansible.fail_json(msg="Error resetting device") + except CvpRequestError: + MODULE_LOGGER.error("Error resetting device. Users is unauthorized!") + self.__ansible.fail_json(msg="Error resetting device. Users is unauthorized!") else: if resp and resp["data"]["status"] == "success": result_data.changed = True From 36ab311091c32b792914fa3f2f49f7e85cb6d294 Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Sat, 11 Nov 2023 03:00:59 +0000 Subject: [PATCH 07/13] adding CvpRequestError 403 handling for tag tools --- .../arista/cvp/plugins/module_utils/tag_tools.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/tag_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/tag_tools.py index 2a8cc9404..c6b4833b9 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/tag_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/tag_tools.py @@ -115,7 +115,11 @@ def tasker(self, tags: list, mode: string, auto_create: bool = True): workspace_id = workspace_name_id workspace_name = workspace_name_id - self.__cv_client.api.workspace_config(workspace_id, workspace_name) + try: + self.__cv_client.api.workspace_config(workspace_id, workspace_name) + except CvpRequestError: + MODULE_LOGGER.info('Workspace creation failed. User is unauthorized!') + self.__ansible.fail_json(msg='Workspace creation failed. User is unauthorized!') # create tags and assign tags for per_device in tags: From c8c72487d0c754da58ee61612c3da258f472c730 Mon Sep 17 00:00:00 2001 From: Sugetha Chandhrasekar Date: Mon, 13 Nov 2023 10:26:56 -0800 Subject: [PATCH 08/13] pylint fix --- .../arista/cvp/plugins/module_utils/device_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py index d70e38bfd..1b1665c7b 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py @@ -2272,7 +2272,7 @@ def decommission_device(self, user_inventory: DeviceInventory): self.__ansible.fail_json(msg="Error decommissioning device") except CvpRequestError as e: if "403 Forbidden" in e.msg: - request_err_msg = f"User is unauthorized to decommission device {device_id}" + request_err_msg = f"User is unauthorized to decommission device {device_id}" else: request_err_msg = f"Device with {device_id} does not exist or is not registered to decommission" MODULE_LOGGER.error(request_err_msg) From f89cbb638c55ee467ecd4223105b039119026f10 Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Tue, 14 Nov 2023 18:31:54 +0000 Subject: [PATCH 09/13] adding CvpRequestError 403 handling for apply image --- .../arista/cvp/plugins/module_utils/device_tools.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py index 1b1665c7b..6ebd54883 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py @@ -1700,6 +1700,13 @@ def apply_bundle(self, user_inventory: DeviceInventory): self.__ansible.fail_json( msg=f"Error applying bundle to device {device.fqdn}: {str(catch_error)}" ) + except CvpRequestError as catch_error: + MODULE_LOGGER.error( + "Error applying bundle to device: %s", str(catch_error) + ) + self.__ansible.fail_json( + msg=f"Error applying bundle to device {device.fqdn}: {str(catch_error)}" + ) else: if resp and resp["data"]["status"] == "success": result_data.changed = True From 5ffba4545d7ee05de0baae93fd6a99c7348d87cb Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Fri, 17 Nov 2023 21:53:13 +0000 Subject: [PATCH 10/13] adding CvpRequestError 403 handling for CC, task, container --- .../cvp/plugins/module_utils/change_tools.py | 23 ++++++++-- .../plugins/module_utils/container_tools.py | 44 ++++++++++++++++++- .../cvp/plugins/module_utils/task_tools.py | 10 ++++- 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/change_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/change_tools.py index be36b4f70..fef39d7ba 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/change_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/change_tools.py @@ -654,7 +654,12 @@ def module_action(self, change: dict, name: str = None, state: str = "show", cha MODULE_LOGGER.debug("Successfully deleted: %s", change_id) changed = True except Exception as e: - self.__ansible.fail_json(msg="{0}".format(e)) + if "Forbidden" in str(e): + message = "Failed to delete Change Control. User is unauthorized!" + else: + message = str(e) + logging.error(message) + self.__ansible.fail_json(msg="{0}".format(message)) return changed, {'remove': []}, warnings @@ -704,7 +709,12 @@ def module_action(self, change: dict, name: str = None, state: str = "show", cha data = cc_structure['key'] except Exception as e: - self.__ansible.fail_json(msg="{0}".format(e)) + if "Forbidden" in str(e): + message = "Failed to create Change Control. User is unauthorized!" + else: + message = str(e) + logging.error(message) + self.__ansible.fail_json(msg="{0}".format(message)) elif state in ['approve', 'unapprove', 'execute', 'schedule', 'approve_and_execute', 'schedule_and_approve'] and self.__check_mode is False: MODULE_LOGGER.debug("Change control state: %s", state) @@ -764,9 +774,14 @@ def module_action(self, change: dict, name: str = None, state: str = "show", cha e = "Change control {0} id not found".format(cc_id) self.__ansible.fail_json(msg="{0}".format(e)) return changed, {"approve": change_id}, warnings - except CvpRequestError: + except CvpRequestError as e: # Skip this - covers the case where an approved CC is approved again - pass + if "Forbidden" in str(e): + message = "Failed to approve Change Control. User is unauthorized!" + self.__ansible.fail_json(msg="{0}".format(message)) + logging.error(str(message)) + else: + pass except Exception as e: self.__ansible.fail_json(msg="{0}".format(e)) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py index c8ce49c86..b6535b2d4 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py @@ -32,7 +32,7 @@ from ansible_collections.arista.cvp.plugins.module_utils.tools_schema import validate_json_schema from ansible_collections.arista.cvp.plugins.module_utils.resources.exceptions import AnsibleCVPApiError, AnsibleCVPNotFoundError, CVPRessource try: - from cvprac.cvp_client_errors import CvpClientError, CvpApiError + from cvprac.cvp_client_errors import CvpClientError, CvpApiError, CvpRequestError HAS_CVPRAC = True except ImportError: HAS_CVPRAC = False @@ -356,6 +356,13 @@ def __configlet_add(self, container: dict, configlets: list, save_topology: bool container=container, create_task=save_topology ) + except CvpRequestError as e: + if "Forbidden" in str(e): + message = "Error configuring configlets. User is unauthorized!" + else: + message = "Error configuring configlets " + str(configlets) + " to container " + str(container) + ". Exception: " + str(e) + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) except CvpApiError as e: message = "Error configuring configlets " + str(configlets) + " to container " + str(container) + ". Exception: " + str(e) MODULE_LOGGER.error(message) @@ -425,6 +432,13 @@ def __configlet_del(self, container: dict, configlets: list, save_topology: bool container=container, create_task=save_topology ) + except CvpRequestError as e: + if "Forbidden" in str(e): + message = "Error removing configlets. User is unauthorized!" + else: + message = "Error removing configlets " + str(configlets) + " to container " + str(container) + ". Exception: " + str(e) + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) except CvpApiError as e: message = "Error removing configlets " + str(configlets) + " from container " + str(container) + ". Exception: " + str(e) MODULE_LOGGER.error(message) @@ -507,6 +521,13 @@ def __image_bundle_add(self, container: dict, image_bundle: str): container[Api.generic.NAME], 'container' ) + except CvpRequestError as e: + if "Forbidden" in str(e): + message = "Error applying bundle to container. User is unauthorized!" + else: + message = "Error applying bundle to container " + str(container[Api.generic.NAME]) + ". Exception: " + str(e) + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) except CvpApiError as catch_error: MODULE_LOGGER.error('Error applying bundle to device: %s', str(catch_error)) self.__ansible.fail_json(msg='Error applying bundle to container' + container[Api.generic.NAME] + ': ' + catch_error) @@ -567,6 +588,13 @@ def __image_bundle_del(self, container: dict): container[Api.generic.NAME], 'container' ) + except CvpRequestError as e: + if "Forbidden" in str(e): + message = "Error removing bundle from container. User is unauthorized!" + else: + message = "Error removing bundle from container " + str(container[Api.generic.NAME]) + ". Exception: " + str(e) + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) except CvpApiError as catch_error: MODULE_LOGGER.error('Error removing bundle from container: %s', str(catch_error)) self.__ansible.fail_json(msg='Error removing bundle from container: ' + container[Api.generic.NAME] + ': ' + catch_error) @@ -839,6 +867,13 @@ def create_container(self, container: str, parent: str): try: resp = self.__cvp_client.api.add_container( container_name=container, parent_key=parent_id, parent_name=parent) + except CvpRequestError as e: + if "Forbidden" in str(e): + message = "Error creating container. User is unauthorized!" + else: + message = "Error creating container " + str(container) + ". Exception: " + str(e) + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) except CvpApiError as e: # Add Ansible error management message = "Error creating container " + str(container) + " on CV. Exception: " + str(e) @@ -915,6 +950,13 @@ def delete_container(self, container: str, parent: str): try: resp = self.__cvp_client.api.delete_container( container_name=container, container_key=container_id, parent_key=parent_id, parent_name=parent) + except CvpRequestError as e: + if "Forbidden" in str(e): + message = "Error deleting container. User is unauthorized!" + else: + message = "Error deleting container " + str(container) + ". Exception: " + str(e) + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) except CvpApiError as e: # Add Ansible error management message = "Error deleting container " + str(container) + " on CV. Exception: " + str(e) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py index 8919c9230..47ceb2e02 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py @@ -137,7 +137,15 @@ def tasker(self, taskIds_list: list, state: str = 'executed'): api_result = CvApiResult(action_name='task_' + str(task_id)) if self.is_actionable(task_data=self.__get_task_data(task_id)): if self.__ansible.check_mode is False: - self.__cv_client.api.add_note_to_task(task_id, "Executed by Ansible") + try: + self.__cv_client.api.add_note_to_task(task_id, "Executed by Ansible") + except CvpRequestError as e: + if "Forbidden" in str(e): + message = "Error while adding note and executing task. User is unauthorized!" + else: + message = "Error while adding note to task: {}".format(str(e)) + logging.error(message) + self.__ansible.fail_json(msg=message) if state == "executed": api_result.add_entry(self.execute_task(task_id)) api_result.changed = True From 665d9b72fa27cad8121c7bbb6cd6fb37f5a3d4ae Mon Sep 17 00:00:00 2001 From: Tamas Plugor Date: Fri, 17 Nov 2023 21:59:02 +0000 Subject: [PATCH 11/13] update error messages for device tools --- .../cvp/plugins/module_utils/device_tools.py | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py index 6ebd54883..4eaa369bc 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/device_tools.py @@ -1936,10 +1936,9 @@ def apply_configlets(self, user_inventory: DeviceInventory): msg="Error applying configlets to device" ) except CvpRequestError: - MODULE_LOGGER.error("Error applying configlets to device. User is unauthorized!") - self.__ansible.fail_json( - msg="Error applying configlets to device. User is unauthorized!" - ) + message = "Failed to apply configlets to device. User is unauthorized!" + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) else: if resp["data"]["status"] == "success": result_data.changed = True @@ -2235,10 +2234,9 @@ def delete_device(self, user_inventory: DeviceInventory): msg="Error removing device from provisioning" ) except CvpRequestError: - MODULE_LOGGER.error("Removing device from provisioning failed. User is unauthorized!") - self.__ansible.fail_json( - msg="Removing device from provisioning failed. User is unauthorized!" - ) + message = "Failed to remove device from provisioning. User is unauthorized!" + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) else: if resp["result"] == "success": result_data.changed = True @@ -2279,7 +2277,7 @@ def decommission_device(self, user_inventory: DeviceInventory): self.__ansible.fail_json(msg="Error decommissioning device") except CvpRequestError as e: if "403 Forbidden" in e.msg: - request_err_msg = f"User is unauthorized to decommission device {device_id}" + request_err_msg = f"Failed to decommission device {device_id}. User is unauthorized!" else: request_err_msg = f"Device with {device_id} does not exist or is not registered to decommission" MODULE_LOGGER.error(request_err_msg) @@ -2345,8 +2343,9 @@ def reset_device(self, user_inventory: DeviceInventory): MODULE_LOGGER.error("Error resetting device") self.__ansible.fail_json(msg="Error resetting device") except CvpRequestError: - MODULE_LOGGER.error("Error resetting device. Users is unauthorized!") - self.__ansible.fail_json(msg="Error resetting device. Users is unauthorized!") + message = "Failed to reset device. Users is unauthorized!" + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) else: if resp and resp["data"]["status"] == "success": result_data.changed = True From 306ab08ddc42c6e3fb5b61e233f7fdff04e6ede9 Mon Sep 17 00:00:00 2001 From: Sugetha Chandhrasekar Date: Mon, 20 Nov 2023 14:48:57 -0800 Subject: [PATCH 12/13] pep8 fix --- .../cvp/plugins/module_utils/container_tools.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py index b6535b2d4..10729d6d1 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/container_tools.py @@ -433,12 +433,12 @@ def __configlet_del(self, container: dict, configlets: list, save_topology: bool create_task=save_topology ) except CvpRequestError as e: - if "Forbidden" in str(e): - message = "Error removing configlets. User is unauthorized!" - else: - message = "Error removing configlets " + str(configlets) + " to container " + str(container) + ". Exception: " + str(e) - MODULE_LOGGER.error(message) - self.__ansible.fail_json(msg=message) + if "Forbidden" in str(e): + message = "Error removing configlets. User is unauthorized!" + else: + message = "Error removing configlets " + str(configlets) + " to container " + str(container) + ". Exception: " + str(e) + MODULE_LOGGER.error(message) + self.__ansible.fail_json(msg=message) except CvpApiError as e: message = "Error removing configlets " + str(configlets) + " from container " + str(container) + ". Exception: " + str(e) MODULE_LOGGER.error(message) @@ -871,7 +871,7 @@ def create_container(self, container: str, parent: str): if "Forbidden" in str(e): message = "Error creating container. User is unauthorized!" else: - message = "Error creating container " + str(container) + ". Exception: " + str(e) + message = "Error creating container " + str(container) + ". Exception: " + str(e) MODULE_LOGGER.error(message) self.__ansible.fail_json(msg=message) except CvpApiError as e: @@ -954,7 +954,7 @@ def delete_container(self, container: str, parent: str): if "Forbidden" in str(e): message = "Error deleting container. User is unauthorized!" else: - message = "Error deleting container " + str(container) + ". Exception: " + str(e) + message = "Error deleting container " + str(container) + ". Exception: " + str(e) MODULE_LOGGER.error(message) self.__ansible.fail_json(msg=message) except CvpApiError as e: From 7eaeb26f118bc53e34552e3476dcd22d95d48a5f Mon Sep 17 00:00:00 2001 From: Sugetha Chandhrasekar Date: Mon, 20 Nov 2023 15:14:36 -0800 Subject: [PATCH 13/13] pylint fix-again --- .../arista/cvp/plugins/module_utils/task_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py b/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py index 47ceb2e02..fd7443957 100644 --- a/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py +++ b/ansible_collections/arista/cvp/plugins/module_utils/task_tools.py @@ -143,7 +143,7 @@ def tasker(self, taskIds_list: list, state: str = 'executed'): if "Forbidden" in str(e): message = "Error while adding note and executing task. User is unauthorized!" else: - message = "Error while adding note to task: {}".format(str(e)) + message = "Error while adding note to task: {0}".format(str(e)) logging.error(message) self.__ansible.fail_json(msg=message) if state == "executed":