Skip to content

Commit

Permalink
Ensure that current uses of BaseException are required
Browse files Browse the repository at this point in the history
* In some cases, it appears that Exception should have been used instead
  as there's no need to catch sys.exit KeyboardInterrupt and similar.
* In a few cases, it appears that BaseException is used because
  a library we depend on calls sys.exit() contrary to good coding
  design.  Comment those so that we know that those have been audited
  and found to be correct and change to use (Exception, SystemExit)
  instead.
  • Loading branch information
abadger committed Dec 16, 2018
1 parent 3fba006 commit 175f3b5
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 20 deletions.
4 changes: 3 additions & 1 deletion contrib/inventory/zabbix.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ def __init__(self):
try:
api = ZabbixAPI(server=self.zabbix_server, validate_certs=self.validate_certs)
api.login(user=self.zabbix_username, password=self.zabbix_password)
except BaseException as e:
# zabbix_api tries to exit if it cannot parse what the zabbix server returned
# so we have to use SystemExit here
except (Exception, SystemExit) as e:
print("Error: Could not login to Zabbix server. Check your zabbix.ini.", file=sys.stderr)
sys.exit(1)

Expand Down
4 changes: 2 additions & 2 deletions lib/ansible/module_utils/vmware.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ def gather_vm_facts(content, vm):
for item in vm.layout.disk:
for disk in item.diskFile:
facts['hw_files'].append(disk)
except BaseException:
except Exception:
pass

facts['hw_folder'] = PyVmomi.get_vm_path(content, vm)
Expand Down Expand Up @@ -968,7 +968,7 @@ def get_vm_path(content, vm_name):
folder_name = fp.name + '/' + folder_name
try:
fp = fp.parent
except BaseException:
except Exception:
break
folder_name = '/' + folder_name
return folder_name
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/modules/cloud/amazon/rds_snapshot_facts.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@

try:
import botocore
except BaseException:
except Exception:
pass # caught by imported HAS_BOTO3


Expand Down
4 changes: 2 additions & 2 deletions lib/ansible/modules/cloud/univention/udm_dns_record.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def main():
obj.create()
else:
obj.modify()
except BaseException as e:
except Exception as e:
module.fail_json(
msg='Creating/editing dns entry {} in {} failed: {}'.format(name, container, e)
)
Expand All @@ -170,7 +170,7 @@ def main():
if not module.check_mode:
obj.remove()
changed = True
except BaseException as e:
except Exception as e:
module.fail_json(
msg='Removing dns entry {} in {} failed: {}'.format(name, container, e)
)
Expand Down
4 changes: 2 additions & 2 deletions lib/ansible/modules/cloud/univention/udm_share.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ def main():
obj.create()
elif changed:
obj.modify()
except BaseException as err:
except Exception as err:
module.fail_json(
msg='Creating/editing share {} in {} failed: {}'.format(
name,
Expand All @@ -528,7 +528,7 @@ def main():
if not module.check_mode:
obj.remove()
changed = True
except BaseException as err:
except Exception as err:
module.fail_json(
msg='Removing share {} in {} failed: {}'.format(
name,
Expand Down
21 changes: 14 additions & 7 deletions lib/ansible/modules/monitoring/zabbix/zabbix_maintenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ def create_maintenance(zbx, group_ids, host_ids, start_time, maintenance_type, p
}]
}
)
except BaseException as e:
# zabbix_api can call sys.exit() so we need to catch SystemExit here
except (Exception, SystemExit) as e:
return 1, None, str(e)
return 0, None, None

Expand All @@ -176,7 +177,8 @@ def update_maintenance(zbx, maintenance_id, group_ids, host_ids, start_time, mai
}]
}
)
except BaseException as e:
# zabbix_api can call sys.exit() so we need to catch SystemExit here
except (Exception, SystemExit) as e:
return 1, None, str(e)
return 0, None, None

Expand All @@ -193,7 +195,8 @@ def get_maintenance(zbx, name):
"selectHosts": "extend"
}
)
except BaseException as e:
# zabbix_api can call sys.exit() so we need to catch SystemExit here
except (Exception, SystemExit) as e:
return 1, None, str(e)

for maintenance in maintenances:
Expand All @@ -207,7 +210,8 @@ def get_maintenance(zbx, name):
def delete_maintenance(zbx, maintenance_id):
try:
zbx.maintenance.delete([maintenance_id])
except BaseException as e:
# zabbix_api can call sys.exit() so we need to catch SystemExit here
except (Exception, SystemExit) as e:
return 1, None, str(e)
return 0, None, None

Expand All @@ -225,7 +229,8 @@ def get_group_ids(zbx, host_groups):
}
}
)
except BaseException as e:
# zabbix_api can call sys.exit() so we need to catch SystemExit here
except (Exception, SystemExit) as e:
return 1, None, str(e)

if not result:
Expand All @@ -249,7 +254,8 @@ def get_host_ids(zbx, host_names):
}
}
)
except BaseException as e:
# zabbix_api can call sys.exit() so we need to catch SystemExit here
except (Exception, SystemExit) as e:
return 1, None, str(e)

if not result:
Expand Down Expand Up @@ -308,7 +314,8 @@ def main():
zbx = ZabbixAPI(server_url, timeout=timeout, user=http_login_user, passwd=http_login_password,
validate_certs=validate_certs)
zbx.login(login_user, login_password)
except BaseException as e:
# zabbix_api can call sys.exit() so we need to catch SystemExit here
except (Exception, SystemExit) as e:
module.fail_json(msg="Failed to connect to Zabbix server: %s" % e)

changed = False
Expand Down
8 changes: 4 additions & 4 deletions lib/ansible/plugins/callback/logdna.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@ def get_hostname():
def get_ip():
try:
return socket.gethostbyname(get_hostname())
except BaseException:
except Exception:
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
try:
s.connect(('10.255.255.255', 1))
IP = s.getsockname()[0]
except BaseException:
except Exception:
IP = '127.0.0.1'
finally:
s.close()
Expand All @@ -101,7 +101,7 @@ def isJSONable(obj):
try:
json.dumps(obj, sort_keys=True, cls=AnsibleJSONEncoder)
return True
except BaseException:
except Exception:
return False


Expand Down Expand Up @@ -165,7 +165,7 @@ def metaIndexing(self, meta):
def sanitizeJSON(self, data):
try:
return json.loads(json.dumps(data, sort_keys=True, cls=AnsibleJSONEncoder))
except BaseException:
except Exception:
return {'warnings': ['JSON Formatting Issue', json.dumps(data, sort_keys=True, cls=AnsibleJSONEncoder)]}

def flush(self, log, options):
Expand Down
2 changes: 2 additions & 0 deletions test/sanity/import/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ def test_python_module(path, base_dir, messages, ansible_module):
except ImporterAnsibleModuleException:
# module instantiated AnsibleModule without raising an exception
pass
# We truly want to catch anything the plugin might do here, including call sys.exit() so we
# catch BaseException
except BaseException as ex: # pylint: disable=locally-disabled, broad-except
capture_report(path, capture, messages)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from ansible.modules.network.cloudvision import cv_server_provision


class MockException(BaseException):
class MockException(Exception):
pass


Expand Down

0 comments on commit 175f3b5

Please sign in to comment.