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

(cherry picked from commit e4c61ea9a1)

CHANGELOG.md

@@ -107,6 +107,7 @@ Ansible Changes By Release
 * chown errors now more informative
 * Fix for win_copy to copy a source file that has invalid windows characters in the filename, the dest still must be have valid windows characters (https://github.com/ansible/ansible/issues/31336#issuecomment-334649927)
 * Fix systemd module to not run daemon-reload in check mode.
+* fixed some parsing and selection issues with inventory manager, fixed minor bugs in yaml and constructed plugins
 
 <a id="2.4"></a>
 

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:

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 can be used for templating.
@@ -49,6 +49,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
@@ -70,7 +73,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
@@ -87,6 +90,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)))
 

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.

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:

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)