From 70ba960f6d0afb6d50244da83ae208b0c5786f20 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 10 Dec 2018 11:38:42 -0500 Subject: [PATCH] More specificity in errors for cfg mgr (#48995) * More specificity in errors for cfg mgr --- changelogs/fragments/better_cfgmgr_errors.yml | 2 ++ lib/ansible/config/manager.py | 29 ++++++++++++------- test/integration/targets/config/runme.sh | 2 +- 3 files changed, 21 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/better_cfgmgr_errors.yml diff --git a/changelogs/fragments/better_cfgmgr_errors.yml b/changelogs/fragments/better_cfgmgr_errors.yml new file mode 100644 index 0000000000..8e39b7a6d1 --- /dev/null +++ b/changelogs/fragments/better_cfgmgr_errors.yml @@ -0,0 +1,2 @@ +bugfixes: + - Now be specific about the entry that trips an error diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index edabf77166..06d95338eb 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -38,6 +38,17 @@ Setting = namedtuple('Setting', 'name value origin type') INTERNAL_DEFS = {'lookup': ('_terms',)} +def _get_entry(plugin_type, plugin_name, config): + ''' construct entry for requested config ''' + entry = '' + if plugin_type: + entry += 'plugin_type: %s ' % plugin_type + if plugin_name: + entry += 'plugin: %s ' % plugin_name + entry += 'setting: %s ' % config + return entry + + # FIXME: see if we can unify in module_utils with similar function used by argspec def ensure_type(value, value_type, origin=None): ''' return a configuration variable with casting @@ -351,7 +362,7 @@ class ConfigManager(object): except AnsibleError: raise except Exception as e: - raise AnsibleError("Unhandled exception when retrieving %s:\n%s" % (config, traceback.format_exc())) + raise AnsibleError("Unhandled exception when retrieving %s:\n%s" % (config), orig_exc=e) return value def get_config_value_and_origin(self, config, cfile=None, plugin_type=None, plugin_name=None, keys=None, variables=None, direct=None): @@ -414,14 +425,9 @@ class ConfigManager(object): # set default if we got here w/o a value if value is None: if defs[config].get('required', False): - entry = '' - if plugin_type: - entry += 'plugin_type: %s ' % plugin_type - if plugin_name: - entry += 'plugin: %s ' % plugin_name - entry += 'setting: %s ' % config if not plugin_type or config not in INTERNAL_DEFS.get(plugin_type, {}): - raise AnsibleError("No setting was provided for required configuration %s" % (entry)) + raise AnsibleError("No setting was provided for required configuration %s" % + to_native(_get_entry(plugin_type, plugin_name, config))) else: value = defs[config].get('default') origin = 'default' @@ -438,13 +444,14 @@ class ConfigManager(object): origin = 'default' value = ensure_type(defs[config].get('default'), defs[config].get('type'), origin=origin) else: - raise AnsibleOptionsError('Invalid type for configuration option %s: %s' % (to_native(config), to_native(e))) + raise AnsibleOptionsError('Invalid type for configuration option %s: %s' % + (to_native(_get_entry(plugin_type, plugin_name, config)), to_native(e))) # deal with deprecation of the setting if 'deprecated' in defs[config] and origin != 'default': self.DEPRECATED.append((config, defs[config].get('deprecated'))) else: - raise AnsibleError('Requested option %s was not defined in configuration' % to_native(config)) + raise AnsibleError('Requested entry (%s) was not defined in configuration.' % to_native(_get_entry(plugin_type, plugin_name, config))) return value, origin @@ -498,7 +505,7 @@ class ConfigManager(object): # above problem #1 has been fixed. Revamp this to be more like the try: except # in get_config_value() at that time. sys.stderr.write("Unhandled error:\n %s\n\n" % traceback.format_exc()) - raise AnsibleError("Invalid settings supplied for %s: %s\n%s" % (config, to_native(e), traceback.format_exc())) + raise AnsibleError("Invalid settings supplied for %s: %s\n" % (config, to_native(e)), orig_exc=e) # set the constant self.data.update_setting(Setting(config, value, origin, defs[config].get('type', 'string'))) diff --git a/test/integration/targets/config/runme.sh b/test/integration/targets/config/runme.sh index f0181afc3a..4d6703e3b7 100755 --- a/test/integration/targets/config/runme.sh +++ b/test/integration/targets/config/runme.sh @@ -7,4 +7,4 @@ set -eux ANSIBLE_TIMEOUT= ansible -m ping localhost "$@" # env var is wrong type, this should be a fatal error pointing at the setting -ANSIBLE_TIMEOUT='lola' ansible -m ping localhost "$@" 2>&1|grep 'AnsibleOptionsError: Invalid type for configuration option DEFAULT_TIMEOUT' +ANSIBLE_TIMEOUT='lola' ansible -m ping localhost "$@" 2>&1|grep 'Invalid type for configuration option setting: DEFAULT_TIMEOUT'