From e4c61ea9a11adef4d1dd55d6672fa140e0816762 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 6 Oct 2017 00:10:41 -0400 Subject: [PATCH] fix 'return false' from parse this was abandoned early on the manger side but seems like we left behind on plugin side. more flexible extensions with yaml plugin validate data correctly for yaml/constructed fixed issue with only adding one child to keyed, the group only got the host that forced it's creation fixes #31382 fixes #31365 --- lib/ansible/plugins/inventory/__init__.py | 9 ++++++--- lib/ansible/plugins/inventory/constructed.py | 9 +++++++-- lib/ansible/plugins/inventory/openstack.py | 21 +++++++++----------- lib/ansible/plugins/inventory/virtualbox.py | 2 +- lib/ansible/plugins/inventory/yaml.py | 6 +++++- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/lib/ansible/plugins/inventory/__init__.py b/lib/ansible/plugins/inventory/__init__.py index 9b63c74c76..0c38c222ef 100644 --- a/lib/ansible/plugins/inventory/__init__.py +++ b/lib/ansible/plugins/inventory/__init__.py @@ -142,10 +142,13 @@ class BaseInventoryPlugin(object): continue if isinstance(groups, string_types): groups = [groups] - for group_name in groups: - if group_name not in self.inventory.groups: - self.inventory.add_group(group_name) + if isinstance(groups, list): + for group_name in groups: + if group_name not in self.inventory.groups: + self.inventory.add_group(group_name) self.inventory.add_child(group_name, host) + else: + raise AnsibleOptionsError("Invalid group name format, expected string or list of strings, got: %s" % type(groups)) else: raise AnsibleOptionsError("No key supplied, invalid entry") else: diff --git a/lib/ansible/plugins/inventory/constructed.py b/lib/ansible/plugins/inventory/constructed.py index 7730992a0a..65eef611c9 100644 --- a/lib/ansible/plugins/inventory/constructed.py +++ b/lib/ansible/plugins/inventory/constructed.py @@ -10,7 +10,7 @@ DOCUMENTATION = ''' version_added: "2.4" short_description: Uses Jinja2 to construct vars and groups based on existing inventory. description: - - Uses a YAML configuration file with a ``.config`` extension to define var expresisions and group conditionals + - Uses a YAML configuration file with a valid YAML or ``.config`` extension to define var expressions and group conditionals - The Jinja2 conditionals that qualify a host for membership. - The JInja2 exprpessions are calculated and assigned to the variables - Only variables already available from previous inventories or the fact cache can be used for templating. @@ -54,6 +54,9 @@ EXAMPLES = ''' import os +from collections import MutableMapping + +from ansible import constants as C from ansible.errors import AnsibleParserError from ansible.plugins.inventory import BaseInventoryPlugin from ansible.module_utils._text import to_native @@ -75,7 +78,7 @@ class InventoryModule(BaseInventoryPlugin): if super(InventoryModule, self).verify_file(path): file_name, ext = os.path.splitext(path) - if not ext or ext == '.config': + if not ext or ext in ['.config'] + C.YAML_FILENAME_EXTENSIONS: valid = True return valid @@ -92,6 +95,8 @@ class InventoryModule(BaseInventoryPlugin): if not data: raise AnsibleParserError("%s is empty" % (to_native(path))) + elif not isinstance(data, MutableMapping): + raise AnsibleParserError('inventory source has invalid structure, it should be a dictionary, got: %s' % type(data)) elif data.get('plugin') != self.NAME: raise AnsibleParserError("%s is not a constructed groups config file, plugin entry must be 'constructed'" % (to_native(path))) diff --git a/lib/ansible/plugins/inventory/openstack.py b/lib/ansible/plugins/inventory/openstack.py index a4514b502a..0aa59e86c1 100644 --- a/lib/ansible/plugins/inventory/openstack.py +++ b/lib/ansible/plugins/inventory/openstack.py @@ -129,21 +129,18 @@ class InventoryModule(BaseInventoryPlugin): except Exception as e: raise AnsibleParserError(e) + msg = '' if not self._config_data: - # empty. this is not my config file - return False - if 'plugin' in self._config_data and self._config_data['plugin'] != self.NAME: - # plugin config file, but not for us - return False + msg = 'File empty. this is not my config file' + elif 'plugin' in self._config_data and self._config_data['plugin'] != self.NAME: + msg = 'plugin config file, but not for us: %s' % self._config_data['plugin'] elif 'plugin' not in self._config_data and 'clouds' not in self._config_data: - # it's not a clouds.yaml file either - return False + msg = "it's not a plugin configuration nor a clouds.yaml file" + elif not HAS_SHADE: + msg = "shade is required for the OpenStack inventory plugin. OpenStack inventory sources will be skipped." - if not HAS_SHADE: - self.display.warning( - 'shade is required for the OpenStack inventory plugin.' - ' OpenStack inventory sources will be skipped.') - return False + if msg: + raise AnsibleParserError(msg) # The user has pointed us at a clouds.yaml file. Use defaults for # everything. diff --git a/lib/ansible/plugins/inventory/virtualbox.py b/lib/ansible/plugins/inventory/virtualbox.py index 1b80945b91..2b033c0e76 100644 --- a/lib/ansible/plugins/inventory/virtualbox.py +++ b/lib/ansible/plugins/inventory/virtualbox.py @@ -173,7 +173,7 @@ class InventoryModule(BaseInventoryPlugin): if not config_data or config_data.get('plugin') != self.NAME: # this is not my config file - return False + raise AnsibleParserError("Incorrect plugin name in file: %s" % config_data.get('plugin', 'none found')) source_data = None if cache and cache_key in inventory.cache: diff --git a/lib/ansible/plugins/inventory/yaml.py b/lib/ansible/plugins/inventory/yaml.py index 3b7777a5b1..f991a97347 100644 --- a/lib/ansible/plugins/inventory/yaml.py +++ b/lib/ansible/plugins/inventory/yaml.py @@ -88,7 +88,11 @@ class InventoryModule(BaseFileInventoryPlugin): raise AnsibleParserError(e) if not data: - return False + raise AnsibleParserError('Parsed empty YAML file') + elif not isinstance(data, MutableMapping): + raise AnsibleParserError('YAML inventory has invalid structure, it should be a dictionary, got: %s' % type(data)) + elif data.get('plugin'): + raise AnsibleParserError('Plugin configuration YAML file, not YAML inventory') # We expect top level keys to correspond to groups, iterate over them # to get host, vars and subgroups (which we iterate over recursivelly)