From 1c4b50fa51ebc4763b2efaf6b45dc5300c53ecb8 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 2 Dec 2024 22:58:51 -0300 Subject: [PATCH 1/2] modules: Do not hide errors using IPA *_show command with Exception When searching for objects with *_show IPA API command, most plugins were hiding errors other than "ipalib_errors.NotFound" by handling the broad exception Exception instead. This patch uses "ipalib_errors.NotFound" whenever "*_show" is used so that the only exception handled is when an object is not found. Other errors will not be handled making the module break as expected. --- plugins/modules/ipaautomountmap.py | 6 +++--- plugins/modules/ipaconfig.py | 2 +- plugins/modules/ipadelegation.py | 4 ++-- plugins/modules/ipaidoverridegroup.py | 4 ++-- plugins/modules/ipaidoverrideuser.py | 5 +++-- plugins/modules/ipaidp.py | 5 +++-- plugins/modules/ipaidrange.py | 5 +++-- plugins/modules/ipaidview.py | 2 +- plugins/modules/ipalocation.py | 4 ++-- plugins/modules/ipapermission.py | 4 ++-- plugins/modules/ipaprivilege.py | 4 ++-- plugins/modules/iparole.py | 4 ++-- plugins/modules/ipaselfservice.py | 4 ++-- plugins/modules/ipaserver.py | 6 +++--- plugins/modules/ipaservicedelegationrule.py | 2 +- plugins/modules/ipaservicedelegationtarget.py | 4 ++-- 16 files changed, 34 insertions(+), 31 deletions(-) diff --git a/plugins/modules/ipaautomountmap.py b/plugins/modules/ipaautomountmap.py index bf0c438e36..ca3c33d8d7 100644 --- a/plugins/modules/ipaautomountmap.py +++ b/plugins/modules/ipaautomountmap.py @@ -106,7 +106,7 @@ ''' from ansible.module_utils.ansible_freeipa_module import ( - IPAAnsibleModule, compare_args_ipa + IPAAnsibleModule, compare_args_ipa, ipalib_errors ) @@ -124,7 +124,7 @@ def get_automountmap(self, location, name): location, {"automountmapname": name, "all": True} ) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: return None return response["result"] @@ -132,7 +132,7 @@ def get_indirect_map_keys(self, location, name): """Check if 'name' is an indirect map for 'parentmap'.""" try: maps = self.ipa_command("automountmap_find", location, {}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: return [] result = [] diff --git a/plugins/modules/ipaconfig.py b/plugins/modules/ipaconfig.py index da57e7cc38..c80da42937 100644 --- a/plugins/modules/ipaconfig.py +++ b/plugins/modules/ipaconfig.py @@ -356,7 +356,7 @@ def config_show(module): def get_netbios_name(module): try: _result = module.ipa_command_no_name("trustconfig_show", {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: return None return _result["result"]["ipantflatname"][0] diff --git a/plugins/modules/ipadelegation.py b/plugins/modules/ipadelegation.py index 7dba924ab9..2e1409815d 100644 --- a/plugins/modules/ipadelegation.py +++ b/plugins/modules/ipadelegation.py @@ -124,14 +124,14 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa + IPAAnsibleModule, compare_args_ipa, ipalib_errors def find_delegation(module, name): """Find if a delegation with the given name already exist.""" try: _result = module.ipa_command("delegation_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if delegation name is not found. return None return _result["result"] diff --git a/plugins/modules/ipaidoverridegroup.py b/plugins/modules/ipaidoverridegroup.py index 4432596c64..b3e5e4d470 100644 --- a/plugins/modules/ipaidoverridegroup.py +++ b/plugins/modules/ipaidoverridegroup.py @@ -155,7 +155,7 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa + IPAAnsibleModule, compare_args_ipa, ipalib_errors from ansible.module_utils import six if six.PY3: @@ -168,7 +168,7 @@ def find_idoverridegroup(module, idview, anchor): _result = module.ipa_command("idoverridegroup_show", idview, {"ipaanchoruuid": anchor, "all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if idoverridegroup anchor is not found. return None return _result["result"] diff --git a/plugins/modules/ipaidoverrideuser.py b/plugins/modules/ipaidoverrideuser.py index 9bae4c9312..3d44d39b6b 100644 --- a/plugins/modules/ipaidoverrideuser.py +++ b/plugins/modules/ipaidoverrideuser.py @@ -315,7 +315,8 @@ from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \ - gen_intersection_list, encode_certificate, convert_input_certificates + gen_intersection_list, encode_certificate, convert_input_certificates, \ + ipalib_errors from ansible.module_utils import six if six.PY3: @@ -328,7 +329,7 @@ def find_idoverrideuser(module, idview, anchor): _result = module.ipa_command("idoverrideuser_show", idview, {"ipaanchoruuid": anchor, "all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if idoverrideuser anchor is not found. return None diff --git a/plugins/modules/ipaidp.py b/plugins/modules/ipaidp.py index b30d60aabf..d9034eb2b6 100644 --- a/plugins/modules/ipaidp.py +++ b/plugins/modules/ipaidp.py @@ -184,7 +184,8 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa, template_str, urlparse + IPAAnsibleModule, compare_args_ipa, template_str, urlparse, \ + ipalib_errors from ansible.module_utils import six from copy import deepcopy import string @@ -269,7 +270,7 @@ def find_idp(module, name): """Find if a idp with the given name already exist.""" try: _result = module.ipa_command("idp_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if idp name is not found. return None diff --git a/plugins/modules/ipaidrange.py b/plugins/modules/ipaidrange.py index 5d70adbcf8..60a2fdc78e 100644 --- a/plugins/modules/ipaidrange.py +++ b/plugins/modules/ipaidrange.py @@ -143,7 +143,8 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa, get_trusted_domain_sid_from_name + IPAAnsibleModule, compare_args_ipa, get_trusted_domain_sid_from_name, \ + ipalib_errors from ansible.module_utils import six if six.PY3: @@ -154,7 +155,7 @@ def find_idrange(module, name): """Find if a idrange with the given name already exist.""" try: _result = module.ipa_command("idrange_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if idrange name is not found. return None return _result["result"] diff --git a/plugins/modules/ipaidview.py b/plugins/modules/ipaidview.py index 9a10eef9aa..c26291d3e5 100644 --- a/plugins/modules/ipaidview.py +++ b/plugins/modules/ipaidview.py @@ -138,7 +138,7 @@ def find_idview(module, name): """Find if a idview with the given name already exist.""" try: _result = module.ipa_command("idview_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if idview name is not found. return None return _result["result"] diff --git a/plugins/modules/ipalocation.py b/plugins/modules/ipalocation.py index 667e77b280..cb5b439062 100644 --- a/plugins/modules/ipalocation.py +++ b/plugins/modules/ipalocation.py @@ -76,14 +76,14 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa + IPAAnsibleModule, compare_args_ipa, ipalib_errors def find_location(module, name): """Find if a location with the given name already exist.""" try: _result = module.ipa_command("location_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if location name is not found. return None return _result["result"] diff --git a/plugins/modules/ipapermission.py b/plugins/modules/ipapermission.py index 9d3d122d30..0c929451d7 100644 --- a/plugins/modules/ipapermission.py +++ b/plugins/modules/ipapermission.py @@ -154,14 +154,14 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa, to_text + IPAAnsibleModule, compare_args_ipa, to_text, ipalib_errors def find_permission(module, name): """Find if a permission with the given name already exist.""" try: _result = module.ipa_command("permission_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if permission name is not found. return None _res = _result["result"] diff --git a/plugins/modules/ipaprivilege.py b/plugins/modules/ipaprivilege.py index 60033bd99e..46df913664 100644 --- a/plugins/modules/ipaprivilege.py +++ b/plugins/modules/ipaprivilege.py @@ -124,7 +124,7 @@ from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \ - gen_intersection_list + gen_intersection_list, ipalib_errors from ansible.module_utils import six if six.PY3: @@ -135,7 +135,7 @@ def find_privilege(module, name): """Find if a privilege with the given name already exist.""" try: _result = module.ipa_command("privilege_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if privilege name is not found. return None return _result["result"] diff --git a/plugins/modules/iparole.py b/plugins/modules/iparole.py index ba77db9ac6..bf44983970 100644 --- a/plugins/modules/iparole.py +++ b/plugins/modules/iparole.py @@ -129,7 +129,7 @@ from ansible.module_utils._text import to_text from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, gen_add_del_lists, compare_args_ipa, \ - gen_intersection_list, ensure_fqdn + gen_intersection_list, ensure_fqdn, ipalib_errors from ansible.module_utils import six if six.PY3: @@ -140,7 +140,7 @@ def find_role(module, name): """Find if a role with the given name already exist.""" try: _result = module.ipa_command("role_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if role name is not found. return None return _result["result"] diff --git a/plugins/modules/ipaselfservice.py b/plugins/modules/ipaselfservice.py index a7480ad673..0a12d3e908 100644 --- a/plugins/modules/ipaselfservice.py +++ b/plugins/modules/ipaselfservice.py @@ -113,14 +113,14 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa + IPAAnsibleModule, compare_args_ipa, ipalib_errors def find_selfservice(module, name): """Find if a selfservice with the given name already exist.""" try: _result = module.ipa_command("selfservice_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if selfservice name is not found. return None return _result["result"] diff --git a/plugins/modules/ipaserver.py b/plugins/modules/ipaserver.py index 35755ae135..5b76d8a6a2 100644 --- a/plugins/modules/ipaserver.py +++ b/plugins/modules/ipaserver.py @@ -192,14 +192,14 @@ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa, DNSName + IPAAnsibleModule, compare_args_ipa, DNSName, ipalib_errors def find_server(module, name): """Find if a server with the given name already exist.""" try: _result = module.ipa_command("server_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if server name is not found. return None return _result["result"] @@ -214,7 +214,7 @@ def server_role_status(module, name): "include_master": True, "raw": True, "all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if server name is not found. return None return _result["result"][0] diff --git a/plugins/modules/ipaservicedelegationrule.py b/plugins/modules/ipaservicedelegationrule.py index 4b9e3435bb..8e7f9585e9 100644 --- a/plugins/modules/ipaservicedelegationrule.py +++ b/plugins/modules/ipaservicedelegationrule.py @@ -142,7 +142,7 @@ def find_servicedelegationrule(module, name): try: _result = module.ipa_command("servicedelegationrule_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if servicedelegationrule name is not found. return None return _result["result"] diff --git a/plugins/modules/ipaservicedelegationtarget.py b/plugins/modules/ipaservicedelegationtarget.py index 857083145f..34556a727f 100644 --- a/plugins/modules/ipaservicedelegationtarget.py +++ b/plugins/modules/ipaservicedelegationtarget.py @@ -106,7 +106,7 @@ from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, gen_add_del_lists, gen_add_list, gen_intersection_list, \ - servicedelegation_normalize_principals + servicedelegation_normalize_principals, ipalib_errors from ansible.module_utils import six if six.PY3: @@ -118,7 +118,7 @@ def find_servicedelegationtarget(module, name): try: _result = module.ipa_command("servicedelegationtarget_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if servicedelegationtarget name is not found. return None return _result["result"] From 5abb515c92f361892994b638ecde150ffb34e82a Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 2 Dec 2024 23:26:08 -0300 Subject: [PATCH 2/2] utils/templates: Use ipalib_errors.NotFound instead of Exception Modify the plugin templates so that the code generated does not hide errors when querying IPA wih *_show command by handlig only the exeption where an object is not found. --- utils/templates/ipamodule+member.py.in | 4 ++-- utils/templates/ipamodule.py.in | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/utils/templates/ipamodule+member.py.in b/utils/templates/ipamodule+member.py.in index daa7fdd49b..714c0d7eb1 100644 --- a/utils/templates/ipamodule+member.py.in +++ b/utils/templates/ipamodule+member.py.in @@ -113,7 +113,7 @@ RETURN = """ from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \ - gen_intersection_list + gen_intersection_list, ipalib_errors from ansible.module_utils import six if six.PY3: @@ -124,7 +124,7 @@ def find_$name(module, name): """Find if a $name with the given name already exist.""" try: _result = module.ipa_command("$name_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if $name name is not found. return None return _result["result"] diff --git a/utils/templates/ipamodule.py.in b/utils/templates/ipamodule.py.in index 0dc282dd6b..afb119a8a8 100644 --- a/utils/templates/ipamodule.py.in +++ b/utils/templates/ipamodule.py.in @@ -89,7 +89,7 @@ RETURN = """ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa + IPAAnsibleModule, compare_args_ipa, ipalib_errors from ansible.module_utils import six if six.PY3: @@ -100,7 +100,7 @@ def find_$name(module, name): """Find if a $name with the given name already exist.""" try: _result = module.ipa_command("$name_show", name, {"all": True}) - except Exception: # pylint: disable=broad-except + except ipalib_errors.NotFound: # An exception is raised if $name name is not found. return None return _result["result"]