From ac56a2f1385d0f7e5b22e3b6b1d79d56da319b80 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 11 Aug 2017 20:23:17 -0700 Subject: [PATCH] Remove wildcard imports and get_exception calls Fixed module_utils --- lib/ansible/module_utils/aos.py | 11 +- lib/ansible/module_utils/basic.py | 114 ++++++++----------- lib/ansible/module_utils/ce.py | 35 +++--- lib/ansible/module_utils/cloud.py | 7 +- lib/ansible/module_utils/connection.py | 16 ++- lib/ansible/module_utils/exoscale.py | 14 +-- lib/ansible/module_utils/fortios.py | 33 +++--- lib/ansible/module_utils/ipa.py | 15 +-- lib/ansible/module_utils/network.py | 24 ++-- lib/ansible/module_utils/postgres.py | 3 - lib/ansible/module_utils/shell.py | 18 +-- lib/ansible/module_utils/urls.py | 70 +++++------- test/sanity/code-smell/boilerplate.sh | 2 +- test/sanity/code-smell/no-get-exception.sh | 27 +++-- test/sanity/code-smell/no-wildcard-import.sh | 3 - 15 files changed, 174 insertions(+), 218 deletions(-) diff --git a/lib/ansible/module_utils/aos.py b/lib/ansible/module_utils/aos.py index 1e00c7bbeba..82439ff2e6c 100644 --- a/lib/ansible/module_utils/aos.py +++ b/lib/ansible/module_utils/aos.py @@ -32,12 +32,12 @@ This module adds shared support for Apstra AOS modules In order to use this module, include it as part of your module -from ansible.module_utils.aos import * +from ansible.module_utils.aos import (check_aos_version, get_aos_session, find_collection_item, + content_to_dict, do_load_resource) """ import json -from ansible.module_utils.pycompat24 import get_exception from distutils.version import LooseVersion try: @@ -53,6 +53,8 @@ try: except ImportError: HAS_AOS_PYEZ = False +from ansible.module_utils._text import to_native + def check_aos_version(module, min=False): """ @@ -172,8 +174,7 @@ def do_load_resource(module, collection, name): try: item.datum = module.params['content'] item.write() - except: - e = get_exception() - module.fail_json(msg="Unable to write item content : %r" % e) + except Exception as e: + module.fail_json(msg="Unable to write item content : %r" % to_native(e)) module.exit_json(changed=True, name=item.name, id=item.id, value=item.value) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 354e2d959f6..0f0197ec735 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -825,10 +825,9 @@ class AnsibleModule(object): # append to legal_inputs and then possibly check against them try: self.aliases = self._handle_aliases() - except Exception: - e = get_exception() + except Exception as e: # Use exceptions here because it isn't safe to call fail_json until no_log is processed - print('\n{"failed": true, "msg": "Module alias error: %s"}' % str(e)) + print('\n{"failed": true, "msg": "Module alias error: %s"}' % to_native(e)) sys.exit(1) # Save parameter values that should never be logged @@ -1000,8 +999,7 @@ class AnsibleModule(object): return context try: ret = selinux.lgetfilecon_raw(to_native(path, errors='surrogate_or_strict')) - except OSError: - e = get_exception() + except OSError as e: if e.errno == errno.ENOENT: self.fail_json(path=path, msg='path %s does not exist' % path) else: @@ -1097,11 +1095,10 @@ class AnsibleModule(object): try: if self.check_mode: return True - rc = selinux.lsetfilecon(to_native(path), - str(':'.join(new_context))) - except OSError: - e = get_exception() - self.fail_json(path=path, msg='invalid selinux context: %s' % str(e), new_context=new_context, cur_context=cur_context, input_was=context) + rc = selinux.lsetfilecon(to_native(path), ':'.join(new_context)) + except OSError as e: + self.fail_json(path=path, msg='invalid selinux context: %s' % to_native(e), + new_context=new_context, cur_context=cur_context, input_was=context) if rc != 0: self.fail_json(path=path, msg='set selinux context failed') changed = True @@ -1192,12 +1189,11 @@ class AnsibleModule(object): except Exception: try: mode = self._symbolic_mode_to_octal(path_stat, mode) - except Exception: - e = get_exception() + except Exception as e: path = to_text(b_path) self.fail_json(path=path, msg="mode must be in octal or symbolic form", - details=str(e)) + details=to_native(e)) if mode != stat.S_IMODE(mode): # prevent mode from having extra info orbeing invalid long number @@ -1235,18 +1231,17 @@ class AnsibleModule(object): new_underlying_stat = os.stat(b_path) if underlying_stat.st_mode != new_underlying_stat.st_mode: os.chmod(b_path, stat.S_IMODE(underlying_stat.st_mode)) - except OSError: - e = get_exception() + except OSError as e: if os.path.islink(b_path) and e.errno == errno.EPERM: # Can't set mode on symbolic links pass elif e.errno in (errno.ENOENT, errno.ELOOP): # Can't set mode on broken symbolic links pass else: - raise e - except Exception: - e = get_exception() + raise + except Exception as e: path = to_text(b_path) - self.fail_json(path=path, msg='chmod failed', details=str(e)) + self.fail_json(path=path, msg='chmod failed', details=to_native(e), + exception=traceback.format_exc()) path_stat = os.lstat(b_path) new_mode = stat.S_IMODE(path_stat.st_mode) @@ -1285,10 +1280,10 @@ class AnsibleModule(object): rc, out, err = self.run_command(attrcmd) if rc != 0 or err: raise Exception("Error while setting attributes: %s" % (out + err)) - except: - e = get_exception() + except Exception as e: path = to_text(b_path) - self.fail_json(path=path, msg='chattr failed', details=str(e)) + self.fail_json(path=path, msg='chattr failed', details=to_native(e), + exception=traceback.format_exc()) return changed def get_file_attributes(self, path): @@ -1524,9 +1519,9 @@ class AnsibleModule(object): os.environ['LANG'] = 'C' os.environ['LC_ALL'] = 'C' os.environ['LC_MESSAGES'] = 'C' - except Exception: - e = get_exception() - self.fail_json(msg="An unknown error was encountered while attempting to validate the locale: %s" % e) + except Exception as e: + self.fail_json(msg="An unknown error was encountered while attempting to validate the locale: %s" % + to_native(e), exception=traceback.format_exc()) def _handle_aliases(self, spec=None, param=None): # this uses exceptions as it happens before we can safely call fail_json @@ -1791,8 +1786,7 @@ class AnsibleModule(object): return (result, None) else: return result - except Exception: - e = get_exception() + except Exception as e: if include_exceptions: return (value, e) return value @@ -2006,9 +2000,9 @@ class AnsibleModule(object): try: param[k] = type_checker(value) - except (TypeError, ValueError): - e = get_exception() - self.fail_json(msg="argument %s is of type %s and we were unable to convert to %s: %s" % (k, type(value), wanted, e)) + except (TypeError, ValueError) as e: + self.fail_json(msg="argument %s is of type %s and we were unable to convert to %s: %s" % + (k, type(value), wanted, to_native(e))) def _set_defaults(self, pre=True, spec=None, param=None): if spec is None: @@ -2356,9 +2350,8 @@ class AnsibleModule(object): try: self.preserved_copy(fn, backupdest) - except (shutil.Error, IOError): - e = get_exception() - self.fail_json(msg='Could not make backup of %s to %s: %s' % (fn, backupdest, e)) + except (shutil.Error, IOError) as e: + self.fail_json(msg='Could not make backup of %s to %s: %s' % (fn, backupdest, to_native(e))) return backupdest @@ -2366,9 +2359,8 @@ class AnsibleModule(object): if os.path.exists(tmpfile): try: os.unlink(tmpfile) - except OSError: - e = get_exception() - sys.stderr.write("could not cleanup %s: %s" % (tmpfile, e)) + except OSError as e: + sys.stderr.write("could not cleanup %s: %s" % (tmpfile, to_native(e))) def preserved_copy(self, src, dest): """Copy a file with preserved ownership, permissions and context""" @@ -2426,15 +2418,13 @@ class AnsibleModule(object): if hasattr(os, 'chflags') and hasattr(dest_stat, 'st_flags'): try: os.chflags(b_src, dest_stat.st_flags) - except OSError: - e = get_exception() + except OSError as e: for err in 'EOPNOTSUPP', 'ENOTSUP': if hasattr(errno, err) and e.errno == getattr(errno, err): break else: raise - except OSError: - e = get_exception() + except OSError as e: if e.errno != errno.EPERM: raise if self.selinux_enabled(): @@ -2448,12 +2438,12 @@ class AnsibleModule(object): try: # Optimistically try a rename, solves some corner cases and can avoid useless work, throws exception if not atomic. os.rename(b_src, b_dest) - except (IOError, OSError): - e = get_exception() + except (IOError, OSError) as e: if e.errno not in [errno.EPERM, errno.EXDEV, errno.EACCES, errno.ETXTBSY, errno.EBUSY]: # only try workarounds for errno 18 (cross device), 1 (not permitted), 13 (permission denied) # and 26 (text file busy) which happens on vagrant synced folders and other 'exotic' non posix file systems - self.fail_json(msg='Could not replace file: %s to %s: %s' % (src, dest, e), exception=traceback.format_exc()) + self.fail_json(msg='Could not replace file: %s to %s: %s' % (src, dest, to_native(e)), + exception=traceback.format_exc()) else: b_dest_dir = os.path.dirname(b_dest) # Use bytes here. In the shippable CI, this fails with @@ -2466,9 +2456,8 @@ class AnsibleModule(object): tmp_dest_name = None try: tmp_dest_fd, tmp_dest_name = tempfile.mkstemp(prefix=native_prefix, dir=native_dest_dir, suffix=native_suffix) - except (OSError, IOError): - e = get_exception() - error_msg = 'The destination directory (%s) is not writable by the current user. Error was: %s' % (os.path.dirname(dest), e) + except (OSError, IOError) as e: + error_msg = 'The destination directory (%s) is not writable by the current user. Error was: %s' % (os.path.dirname(dest), to_native(e)) except TypeError: # We expect that this is happening because python3.4.x and # below can't handle byte strings in mkstemp(). Traceback @@ -2506,21 +2495,20 @@ class AnsibleModule(object): tmp_stat = os.stat(b_tmp_dest_name) if dest_stat and (tmp_stat.st_uid != dest_stat.st_uid or tmp_stat.st_gid != dest_stat.st_gid): os.chown(b_tmp_dest_name, dest_stat.st_uid, dest_stat.st_gid) - except OSError: - e = get_exception() + except OSError as e: if e.errno != errno.EPERM: raise try: os.rename(b_tmp_dest_name, b_dest) - except (shutil.Error, OSError, IOError): - e = get_exception() + except (shutil.Error, OSError, IOError) as e: if unsafe_writes and e.errno == errno.EBUSY: self._unsafe_writes(b_tmp_dest_name, b_dest) else: - self.fail_json(msg='Unable to rename file: %s to %s: %s' % (src, dest, e), exception=traceback.format_exc()) - except (shutil.Error, OSError, IOError): - e = get_exception() - self.fail_json(msg='Failed to replace file: %s to %s: %s' % (src, dest, e), exception=traceback.format_exc()) + self.fail_json(msg='Unable to rename file: %s to %s: %s' % (src, dest, to_native(e)), + exception=traceback.format_exc()) + except (shutil.Error, OSError, IOError) as e: + self.fail_json(msg='Failed to replace file: %s to %s: %s' % (src, dest, to_native(e)), + exception=traceback.format_exc()) finally: self.cleanup(b_tmp_dest_name) @@ -2554,9 +2542,9 @@ class AnsibleModule(object): out_dest.close() if in_src: in_src.close() - except (shutil.Error, OSError, IOError): - e = get_exception() - self.fail_json(msg='Could not write data to file (%s) from (%s): %s' % (dest, src, e), exception=traceback.format_exc()) + except (shutil.Error, OSError, IOError) as e: + self.fail_json(msg='Could not write data to file (%s) from (%s): %s' % (dest, src, to_native(e)), + exception=traceback.format_exc()) def _read_from_pipes(self, rpipes, rfds, file_descriptor): data = b('') @@ -2742,9 +2730,9 @@ class AnsibleModule(object): kwargs['cwd'] = cwd try: os.chdir(cwd) - except (OSError, IOError): - e = get_exception() - self.fail_json(rc=e.errno, msg="Could not open %s, %s" % (cwd, str(e))) + except (OSError, IOError) as e: + self.fail_json(rc=e.errno, msg="Could not open %s, %s" % (cwd, to_native(e)), + exception=traceback.format_exc()) old_umask = None if umask: @@ -2800,12 +2788,10 @@ class AnsibleModule(object): cmd.stderr.close() rc = cmd.returncode - except (OSError, IOError): - e = get_exception() + except (OSError, IOError) as e: self.log("Error Executing CMD:%s Exception:%s" % (clean_args, to_native(e))) self.fail_json(rc=e.errno, msg=to_native(e), cmd=clean_args) - except Exception: - e = get_exception() + except Exception as e: self.log("Error Executing CMD:%s Exception:%s" % (clean_args, to_native(traceback.format_exc()))) self.fail_json(rc=257, msg=to_native(e), exception=traceback.format_exc(), cmd=clean_args) diff --git a/lib/ansible/module_utils/ce.py b/lib/ansible/module_utils/ce.py index e7c40dc1421..1f234997289 100644 --- a/lib/ansible/module_utils/ce.py +++ b/lib/ansible/module_utils/ce.py @@ -31,12 +31,13 @@ import re import socket import sys +import traceback from ansible.module_utils.basic import env_fallback from ansible.module_utils.network_common import to_list, ComplexList from ansible.module_utils.connection import exec_command -from ansible.module_utils.pycompat24 import get_exception from ansible.module_utils.six import iteritems +from ansible.module_utils._text import to_native try: @@ -66,7 +67,6 @@ ce_argument_spec = { def check_args(module, warnings): - provider = module.params['provider'] or {} for key in ce_argument_spec: if key not in ['provider', 'transport'] and module.params[key]: warnings.append('argument %s has been deprecated and will be ' @@ -323,9 +323,9 @@ class Netconf(object): timeout=30) except AuthenticationError: self._module.fail_json(msg='Error: Authentication failed while connecting to device.') - except Exception: - err = get_exception() - self._module.fail_json(msg='Error: %s' % str(err).replace("\r\n", "")) + except Exception as err: + self._module.fail_json(msg='Error: %s' % to_native(err).replace("\r\n", ""), + exception=traceback.format_exc()) raise def __del__(self): @@ -339,9 +339,8 @@ class Netconf(object): try: con_obj = self.mc.edit_config(target='running', config=xml_str) - except RPCError: - err = get_exception() - self._module.fail_json(msg='Error: %s' % str(err).replace("\r\n", "")) + except RPCError as err: + self._module.fail_json(msg='Error: %s' % to_native(err).replace("\r\n", "")) return con_obj.xml @@ -351,9 +350,8 @@ class Netconf(object): con_obj = None try: con_obj = self.mc.get(filter=xml_str) - except RPCError: - err = get_exception() - self._module.fail_json(msg='Error: %s' % str(err).replace("\r\n", "")) + except RPCError as err: + self._module.fail_json(msg='Error: %s' % to_native(err).replace("\r\n", "")) set_id = get_nc_set_id(con_obj.xml) if not set_id: @@ -368,9 +366,8 @@ class Netconf(object): # get next data try: con_obj_next = self.mc.dispatch(xsd_fetch) - except RPCError: - err = get_exception() - self._module.fail_json(msg='Error: %s' % str(err).replace("\r\n", "")) + except RPCError as err: + self._module.fail_json(msg='Error: %s' % to_native(err).replace("\r\n", "")) if "" in con_obj_next.xml: break @@ -388,9 +385,8 @@ class Netconf(object): try: con_obj = self.mc.action(action=xml_str) - except RPCError: - err = get_exception() - self._module.fail_json(msg='Error: %s' % str(err).replace("\r\n", "")) + except RPCError as err: + self._module.fail_json(msg='Error: %s' % to_native(err).replace("\r\n", "")) except TimeoutExpiredError: raise @@ -403,9 +399,8 @@ class Netconf(object): try: con_obj = self.mc.cli(command=xml_str) - except RPCError: - err = get_exception() - self._module.fail_json(msg='Error: %s' % str(err).replace("\r\n", "")) + except RPCError as err: + self._module.fail_json(msg='Error: %s' % to_native(err).replace("\r\n", "")) return con_obj.xml diff --git a/lib/ansible/module_utils/cloud.py b/lib/ansible/module_utils/cloud.py index 52bb18e53ed..96024588c85 100644 --- a/lib/ansible/module_utils/cloud.py +++ b/lib/ansible/module_utils/cloud.py @@ -22,7 +22,7 @@ This module adds shared support for generic cloud modules In order to use this module, include it as part of a custom module as shown below. -from ansible.module_utils.cloud import * +from ansible.module_utils.cloud import CloudRetry The 'cloud' module provides the following common classes: @@ -44,8 +44,6 @@ from functools import wraps import syslog import time -from ansible.module_utils.pycompat24 import get_exception - def _exponential_backoff(retries=10, delay=2, backoff=2, max_delay=60): """ Customizable exponential backoff strategy. @@ -140,8 +138,7 @@ class CloudRetry(object): for delay in backoff_strategy(): try: return f(*args, **kwargs) - except Exception: - e = get_exception() + except Exception as e: if isinstance(e, cls.base_class): response_code = cls.status_code_from_exception(e) if cls.found(response_code): diff --git a/lib/ansible/module_utils/connection.py b/lib/ansible/module_utils/connection.py index 0be18381d44..676fcc42604 100644 --- a/lib/ansible/module_utils/connection.py +++ b/lib/ansible/module_utils/connection.py @@ -26,15 +26,14 @@ # LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import signal +import os import socket import struct -import os +import traceback import uuid from functools import partial -from ansible.module_utils.basic import get_exception from ansible.module_utils._text import to_bytes, to_native, to_text @@ -72,10 +71,9 @@ def exec_command(module, command): rc = int(recv_data(sf), 10) stdout = recv_data(sf) stderr = recv_data(sf) - except socket.error: - exc = get_exception() + except socket.error as e: sf.close() - module.fail_json(msg='unable to connect to socket', err=str(exc)) + module.fail_json(msg='unable to connect to socket', err=to_native(e), exception=traceback.format_exc()) sf.close() @@ -128,9 +126,9 @@ class Connection: data = self._module.jsonify(req) rc, out, err = exec_command(self._module, data) - except socket.error: - exc = get_exception() - self._module.fail_json(msg='unable to connect to socket', err=str(exc)) + except socket.error as e: + self._module.fail_json(msg='unable to connect to socket', err=to_native(e), + exception=traceback.format_exc()) try: response = self._module.from_json(to_text(out, errors='surrogate_then_replace')) diff --git a/lib/ansible/module_utils/exoscale.py b/lib/ansible/module_utils/exoscale.py index b319396fcf4..0c88b046c57 100644 --- a/lib/ansible/module_utils/exoscale.py +++ b/lib/ansible/module_utils/exoscale.py @@ -29,11 +29,9 @@ import os -# import module snippets -from ansible.module_utils.pycompat24 import get_exception from ansible.module_utils.six.moves import configparser from ansible.module_utils.six import integer_types, string_types -from ansible.module_utils._text import to_text +from ansible.module_utils._text import to_native, to_text from ansible.module_utils.urls import fetch_url EXO_DNS_BASEURL = "https://api.exoscale.ch/dns/v1" @@ -66,9 +64,8 @@ class ExoDns(object): config = self.read_config(ini_group=region) self.api_key = config['key'] self.api_secret = config['secret'] - except Exception: - e = get_exception() - self.module.fail_json(msg="Error while processing config: %s" % e) + except Exception as e: + self.module.fail_json(msg="Error while processing config: %s" % to_native(e)) self.headers = { 'X-DNS-Token': "%s:%s" % (self.api_key, self.api_secret), @@ -133,9 +130,8 @@ class ExoDns(object): try: return self.module.from_json(to_text(response.read())) - except Exception: - e = get_exception() - self.module.fail_json(msg="Could not process response into json: %s" % e) + except Exception as e: + self.module.fail_json(msg="Could not process response into json: %s" % to_native(e)) def has_changed(self, want_dict, current_dict, only_keys=None): changed = False diff --git a/lib/ansible/module_utils/fortios.py b/lib/ansible/module_utils/fortios.py index 4ad949bd027..65deef5be5a 100644 --- a/lib/ansible/module_utils/fortios.py +++ b/lib/ansible/module_utils/fortios.py @@ -28,15 +28,15 @@ # import os import time +import traceback -from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.pycompat24 import get_exception +from ansible.module_utils._text import to_native # check for pyFG lib try: from pyFG import FortiOS, FortiConfig - from pyFG.exceptions import CommandExecutionException, FailedCommit + from pyFG.exceptions import FailedCommit HAS_PYFG = True except ImportError: HAS_PYFG = False @@ -115,9 +115,9 @@ class AnsibleFortios(object): try: self.forti_device.open() - except Exception: - e = get_exception() - self.module.fail_json(msg='Error connecting device. %s' % e) + except Exception as e: + self.module.fail_json(msg='Error connecting device. %s' % to_native(e), + exception=traceback.format_exc()) def load_config(self, path): self.path = path @@ -128,19 +128,19 @@ class AnsibleFortios(object): f = open(self.module.params['config_file'], 'r') running = f.read() f.close() - except IOError: - e = get_exception() - self.module.fail_json(msg='Error reading configuration file. %s' % e) + except IOError as e: + self.module.fail_json(msg='Error reading configuration file. %s' % to_native(e), + exception=traceback.format_exc()) self.forti_device.load_config(config_text=running, path=path) else: # get config try: self.forti_device.load_config(path=path) - except Exception: + except Exception as e: self.forti_device.close() - e = get_exception() - self.module.fail_json(msg='Error reading running config. %s' % e) + self.module.fail_json(msg='Error reading running config. %s' % to_native(e), + exception=traceback.format_exc()) # set configs in object self.result['running_config'] = self.forti_device.running_config.to_text() @@ -163,16 +163,15 @@ class AnsibleFortios(object): f = open(self.module.params['config_file'], 'w') f.write(self.candidate_config.to_text()) f.close() - except IOError: - e = get_exception() - self.module.fail_json(msg='Error writing configuration file. %s' % e) + except IOError as e: + self.module.fail_json(msg='Error writing configuration file. %s' % + to_native(e), exception=traceback.format_exc()) else: try: self.forti_device.commit() - except FailedCommit: + except FailedCommit as e: # Something's wrong (rollback is automatic) self.forti_device.close() - e = get_exception() error_list = self.get_error_infos(e) self.module.fail_json(msg_error_list=error_list, msg="Unable to commit change, check your args, the error was %s" % e.message) diff --git a/lib/ansible/module_utils/ipa.py b/lib/ansible/module_utils/ipa.py index 10473833a8f..ab5ffa9f787 100644 --- a/lib/ansible/module_utils/ipa.py +++ b/lib/ansible/module_utils/ipa.py @@ -32,8 +32,7 @@ try: except ImportError: import simplejson as json -from ansible.module_utils._text import to_bytes, to_text -from ansible.module_utils.pycompat24 import get_exception +from ansible.module_utils._text import to_bytes, to_native, to_text from ansible.module_utils.six import PY3 from ansible.module_utils.six.moves.urllib.parse import quote from ansible.module_utils.urls import fetch_url @@ -69,9 +68,8 @@ class IPAClient(object): 'Content-Type': 'application/json', 'Accept': 'application/json', 'Cookie': resp.info().get('Set-Cookie')} - except Exception: - e = get_exception() - self._fail('login', str(e)) + except Exception as e: + self._fail('login', to_native(e)) def _fail(self, msg, e): if 'message' in e: @@ -90,9 +88,8 @@ class IPAClient(object): status_code = info['status'] if status_code not in [200, 201, 204]: self._fail(method, info['msg']) - except Exception: - e = get_exception() - self._fail('post %s' % method, str(e)) + except Exception as e: + self._fail('post %s' % method, to_native(e)) if PY3: charset = resp.headers.get_content_charset('latin-1') @@ -105,7 +102,7 @@ class IPAClient(object): resp = json.loads(to_text(resp.read(), encoding=charset), encoding=charset) err = resp.get('error') if err is not None: - self._fail('repsonse %s' % method, err) + self._fail('response %s' % method, err) if 'result' in resp: result = resp.get('result') diff --git a/lib/ansible/module_utils/network.py b/lib/ansible/module_utils/network.py index ab22beebcf5..108f215c769 100644 --- a/lib/ansible/module_utils/network.py +++ b/lib/ansible/module_utils/network.py @@ -24,13 +24,16 @@ # INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT # LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -# + +import traceback + from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.basic import env_fallback, get_exception -from ansible.module_utils.netcli import Cli, Command +from ansible.module_utils.basic import env_fallback +from ansible.module_utils.netcli import Cli from ansible.module_utils._text import to_native from ansible.module_utils.six import iteritems + NET_TRANSPORT_ARGS = dict( host=dict(required=True), port=dict(type='int'), @@ -130,9 +133,8 @@ class NetworkModule(AnsibleModule): self.connection = cls() except KeyError: self.fail_json(msg='Unknown transport or no default transport specified') - except (TypeError, NetworkError): - exc = get_exception() - self.fail_json(msg=to_native(exc)) + except (TypeError, NetworkError) as exc: + self.fail_json(msg=to_native(exc), exception=traceback.format_exc()) if connect_on_load: self.connect() @@ -176,18 +178,16 @@ class NetworkModule(AnsibleModule): self.connection.authorize(self.params) self.log('connected to %s:%s using %s' % (self.params['host'], self.params['port'], self.params['transport'])) - except NetworkError: - exc = get_exception() - self.fail_json(msg=to_native(exc)) + except NetworkError as exc: + self.fail_json(msg=to_native(exc), exception=traceback.format_exc()) def disconnect(self): try: if self.connected: self.connection.disconnect() self.log('disconnected from %s' % self.params['host']) - except NetworkError: - exc = get_exception() - self.fail_json(msg=to_native(exc)) + except NetworkError as exc: + self.fail_json(msg=to_native(exc), exception=traceback.format_exc()) def register_transport(transport, default=False): diff --git a/lib/ansible/module_utils/postgres.py b/lib/ansible/module_utils/postgres.py index 457d160bd2d..afa8ad90667 100644 --- a/lib/ansible/module_utils/postgres.py +++ b/lib/ansible/module_utils/postgres.py @@ -34,9 +34,6 @@ try: except ImportError: HAS_PSYCOPG2 = False -from ansible.module_utils.basic import get_exception -from ansible.module_utils.six import iteritems - class LibraryError(Exception): pass diff --git a/lib/ansible/module_utils/shell.py b/lib/ansible/module_utils/shell.py index bb8a14bc11e..1e429a6c180 100644 --- a/lib/ansible/module_utils/shell.py +++ b/lib/ansible/module_utils/shell.py @@ -19,9 +19,7 @@ import os import re import socket -import time import signal -import json try: import paramiko @@ -30,7 +28,6 @@ try: except ImportError: HAS_PARAMIKO = False -from ansible.module_utils.basic import get_exception from ansible.module_utils.network import NetworkError from ansible.module_utils.six import BytesIO from ansible.module_utils._text import to_native @@ -105,8 +102,7 @@ class Shell(object): raise ShellError('Unable to authenticate to remote device') except socket.timeout: raise ShellError("timeout trying to connect to remote device") - except socket.error: - exc = get_exception() + except socket.error as exc: if exc.errno == 60: raise ShellError('timeout trying to connect to host') raise @@ -147,8 +143,7 @@ class Shell(object): if cmd: resp = self.sanitize(cmd, resp) return resp - except ShellError: - exc = get_exception() + except ShellError as exc: exc.command = cmd['command'] raise @@ -167,8 +162,7 @@ class Shell(object): signal.alarm(0) return (0, out, '') - except ShellError: - exc = get_exception() + except ShellError as exc: return (1, '', to_native(exc)) def close(self): @@ -236,8 +230,7 @@ class CliBase(object): self.shell.open(host, port=port, username=username, password=password, key_filename=key_file) - except ShellError: - exc = get_exception() + except ShellError as exc: raise NetworkError(msg='failed to connect to %s:%s' % (host, port), exc=to_native(exc)) @@ -277,8 +270,7 @@ class CliBase(object): raise ShellError(err) responses.append(out) return responses - except ShellError: - exc = get_exception() + except ShellError as exc: raise NetworkError(to_native(exc)) def run_commands(self, x): diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py index b9c21704e89..9f80e0c1d14 100644 --- a/lib/ansible/module_utils/urls.py +++ b/lib/ansible/module_utils/urls.py @@ -96,14 +96,15 @@ for users making use of a module. If possible, avoid third party libraries by us this code instead. ''' +import base64 import netrc import os -import re -import sys -import socket import platform +import re +import socket +import sys import tempfile -import base64 +import traceback try: import httplib @@ -114,8 +115,7 @@ except ImportError: import ansible.module_utils.six.moves.http_cookiejar as cookiejar import ansible.module_utils.six.moves.urllib.request as urllib_request import ansible.module_utils.six.moves.urllib.error as urllib_error -from ansible.module_utils.basic import get_distribution, get_exception -from ansible.module_utils.six import b +from ansible.module_utils.basic import get_distribution from ansible.module_utils._text import to_bytes, to_native, to_text try: @@ -311,7 +311,7 @@ if not HAS_MATCH_HOSTNAME: # ca cert, regardless of validity, for Python on Mac OS to use the # keychain functionality in OpenSSL for validating SSL certificates. # See: http://mercurial.selenic.com/wiki/CACertificates#Mac_OS_X_10.6_and_higher -b_DUMMY_CA_CERT = b("""-----BEGIN CERTIFICATE----- +b_DUMMY_CA_CERT = b"""-----BEGIN CERTIFICATE----- MIICvDCCAiWgAwIBAgIJAO8E12S7/qEpMA0GCSqGSIb3DQEBBQUAMEkxCzAJBgNV BAYTAlVTMRcwFQYDVQQIEw5Ob3J0aCBDYXJvbGluYTEPMA0GA1UEBxMGRHVyaGFt MRAwDgYDVQQKEwdBbnNpYmxlMB4XDTE0MDMxODIyMDAyMloXDTI0MDMxNTIyMDAy @@ -328,7 +328,7 @@ MUB80IR6knq9K/tY+hvPsZer6eFMzO3JGkRFBh2kn6JdMDnhYGX7AXVHGflrwNQH qFy+aenWXsC0ZvrikFxbQnX8GVtDADtVznxOi7XzFw7JOxdsVrpXgSN0eh0aMzvV zKPZsZ2miVGclicJHzm5q080b1p/sZtuKIEZk6vZqEg= -----END CERTIFICATE----- -""") +""" # # Exceptions @@ -657,11 +657,11 @@ class SSLValidationHandler(urllib_request.BaseHandler): cert = cert_file.read() cert_file.close() os.write(tmp_fd, cert) - os.write(tmp_fd, b('\n')) + os.write(tmp_fd, b'\n') if full_path not in LOADED_VERIFY_LOCATIONS: to_add = True os.write(to_add_fd, cert) - os.write(to_add_fd, b('\n')) + os.write(to_add_fd, b'\n') LOADED_VERIFY_LOCATIONS.add(full_path) except (OSError, IOError): pass @@ -729,10 +729,10 @@ class SSLValidationHandler(urllib_request.BaseHandler): s.sendall(self.CONNECT_COMMAND % (self.hostname, self.port)) if proxy_parts.get('username'): credentials = "%s:%s" % (proxy_parts.get('username', ''), proxy_parts.get('password', '')) - s.sendall(b('Proxy-Authorization: Basic %s\r\n') % base64.b64encode(to_bytes(credentials, errors='surrogate_or_strict')).strip()) - s.sendall(b('\r\n')) - connect_result = b("") - while connect_result.find(b("\r\n\r\n")) <= 0: + s.sendall(b'Proxy-Authorization: Basic %s\r\n' % base64.b64encode(to_bytes(credentials, errors='surrogate_or_strict')).strip()) + s.sendall(b'\r\n') + connect_result = b"" + while connect_result.find(b"\r\n\r\n") <= 0: connect_result += s.recv(4096) # 128 kilobytes of headers should be enough for everyone. if len(connect_result) > 131072: @@ -759,11 +759,9 @@ class SSLValidationHandler(urllib_request.BaseHandler): # close the ssl connection # ssl_s.unwrap() s.close() - except (ssl.SSLError, CertificateError): - e = get_exception() + except (ssl.SSLError, CertificateError) as e: build_ssl_validation_error(self.hostname, self.port, paths_checked, e) - except socket.error: - e = get_exception() + except socket.error as e: raise ConnectionError('Failed to connect to %s at port %s: %s' % (self.hostname, self.port, to_native(e))) try: @@ -962,7 +960,7 @@ def basic_auth_header(username, password): """Takes a username and password and returns a byte string suitable for using as value of an Authorization header to do basic auth. """ - return b("Basic %s") % base64.b64encode(to_bytes("%s:%s" % (username, password), errors='surrogate_or_strict')) + return b"Basic %s" % base64.b64encode(to_bytes("%s:%s" % (username, password), errors='surrogate_or_strict')) def url_argument_spec(): @@ -1052,41 +1050,35 @@ def fetch_url(module, url, data=None, headers=None, method=None, info['cookies'] = cookie_dict # finally update the result with a message about the fetch info.update(dict(msg="OK (%s bytes)" % r.headers.get('Content-Length', 'unknown'), url=r.geturl(), status=r.code)) - except NoSSLError: - e = get_exception() + except NoSSLError as e: distribution = get_distribution() if distribution is not None and distribution.lower() == 'redhat': - module.fail_json(msg='%s. You can also install python-ssl from EPEL' % str(e)) + module.fail_json(msg='%s. You can also install python-ssl from EPEL' % to_native(e)) else: - module.fail_json(msg='%s' % str(e)) - except (ConnectionError, ValueError): - e = get_exception() - module.fail_json(msg=str(e)) - except urllib_error.HTTPError: - e = get_exception() + module.fail_json(msg='%s' % to_native(e)) + except (ConnectionError, ValueError) as e: + module.fail_json(msg=to_native(e)) + except urllib_error.HTTPError as e: try: body = e.read() except AttributeError: body = '' # Try to add exception info to the output but don't fail if we can't - exc_info = e.info() try: info.update(dict(**e.info())) except: pass - info.update({'msg': str(e), 'body': body, 'status': e.code}) + info.update({'msg': to_native(e), 'body': body, 'status': e.code}) - except urllib_error.URLError: - e = get_exception() + except urllib_error.URLError as e: code = int(getattr(e, 'code', -1)) - info.update(dict(msg="Request failed: %s" % str(e), status=code)) - except socket.error: - e = get_exception() - info.update(dict(msg="Connection failure: %s" % str(e), status=-1)) - except Exception: - e = get_exception() - info.update(dict(msg="An unknown error occurred: %s" % str(e), status=-1)) + info.update(dict(msg="Request failed: %s" % to_native(e), status=code)) + except socket.error as e: + info.update(dict(msg="Connection failure: %s" % to_native(e), status=-1)) + except Exception as e: + info.update(dict(msg="An unknown error occurred: %s" % to_native(e), status=-1), + exception=traceback.format_exc()) return r, info diff --git a/test/sanity/code-smell/boilerplate.sh b/test/sanity/code-smell/boilerplate.sh index f81f68992c1..5e7a44a68fd 100755 --- a/test/sanity/code-smell/boilerplate.sh +++ b/test/sanity/code-smell/boilerplate.sh @@ -3,7 +3,7 @@ metaclass1=$(find ./bin -type f -exec grep -HL '__metaclass__ = type' '{}' '+') future1=$(find ./bin -type f -exec grep -HL 'from __future__ import (absolute_import, division, print_function)' '{}' '+') -# We eventually want to remove the module_utils and modules pruning from metaclass2 and future2 +# We eventually want to remove the module_utils pruning from metaclass2 and future2 metaclass2=$(find ./lib/ansible -path ./lib/ansible/modules -prune \ -o -path ./lib/ansible/module_utils -prune \ -o -path ./lib/ansible/module_utils/six/_six.py -prune \ diff --git a/test/sanity/code-smell/no-get-exception.sh b/test/sanity/code-smell/no-get-exception.sh index 6283dea3cfb..32510e005c2 100755 --- a/test/sanity/code-smell/no-get-exception.sh +++ b/test/sanity/code-smell/no-get-exception.sh @@ -3,9 +3,10 @@ # We're getting rid of get_exception in our code so that we can deprecate it. # get_exception is no longer needed as we no longer support python-2.4 on the controller. -# We eventually want this list to be empty +# We eventually want pycompat24 and basic.py to be the only things on this list get_exception=$(find . -path ./test/runner/.tox -prune \ - -o -path ./lib/ansible/module_utils -prune \ + -o -path ./lib/ansible/module_utils/pycompat24.py -prune \ + -o -path ./lib/ansible/module_utils/basic.py -prune \ -o -path ./lib/ansible/modules/storage/netapp -prune \ -o -path ./lib/ansible/modules/packaging/os -prune \ -o -path ./lib/ansible/modules/monitoring -prune \ @@ -24,13 +25,21 @@ get_exception=$(find . -path ./test/runner/.tox -prune \ -o -path ./lib/ansible/plugins/action/wait_for_connection.py -prune \ -o -name '*.py' -type f -exec grep -H 'get_exception' '{}' '+') +basic_failed=0 -if test -n "$get_exception" ; then - printf "\n== Contains get_exception() calls ==\n" - printf "%s" "$get_exception" - failures=$(printf "%s" "$get_exception"| wc -l) - failures=$((failures + 2)) - exit "$failures" +if test "$(grep -c get_exception ./lib/ansible/module_utils/basic.py)" -gt 1 ; then + printf "\n== basic.py contains get_exception calls ==\n\n" + printf " basic.py is allowed to import get_exception for backwards compat but\n" + printf " should not call it anywhere\n" + basic_failed=1 fi -exit 0 +failures=0 +if test -n "$get_exception" ; then + printf "\n== Contains get_exception() calls ==\n" + printf " %s\n" "$get_exception" + failures=$(printf "%s" "$get_exception"| wc -l) + failures=$((failures + 2)) +fi + +exit $((basic_failed + failures)) diff --git a/test/sanity/code-smell/no-wildcard-import.sh b/test/sanity/code-smell/no-wildcard-import.sh index fc5d8daac25..0602c9009ad 100755 --- a/test/sanity/code-smell/no-wildcard-import.sh +++ b/test/sanity/code-smell/no-wildcard-import.sh @@ -17,9 +17,6 @@ wildcard_imports=$(find . -path ./test/runner/.tox -prune \ -o -path ./test/units/plugins/action/test_action.py \ -o -path ./lib/ansible/compat/tests/mock.py -prune \ -o -path ./lib/ansible/compat/tests/unittest.py \ - -o -path ./lib/ansible/module_utils/api.py -prune \ - -o -path ./lib/ansible/module_utils/cloud.py -prune \ - -o -path ./lib/ansible/module_utils/aos.py -prune \ -o -path ./test/units/modules/network/cumulus/test_nclu.py -prune \ -o -path ./lib/ansible/modules/cloud/amazon -prune \ -o -path ./lib/ansible/modules/cloud/openstack -prune \