added keyed_group construction (#28578)

* added keyed_group construction

also added strict config to allow skipping bad templating
more precise error msgs
to_native better than to_text
fixed truthyness
added safe names

* allow keyed expressions to return lists

* PEPE should eat less, he is getting fat

lib/ansible/inventory/manager.py

@@ -28,7 +28,7 @@ from ansible import constants as C
 from ansible.errors import AnsibleError, AnsibleOptionsError, AnsibleParserError
 from ansible.inventory.data import InventoryData
 from ansible.module_utils.six import string_types
-from ansible.module_utils._text import to_bytes, to_text
+from ansible.module_utils._text import to_bytes, to_native
 from ansible.parsing.utils.addresses import parse_address
 from ansible.plugins.loader import PluginLoader
 from ansible.utils.path import unfrackpath
@@ -232,7 +232,7 @@ class InventoryManager(object):
 
                 # recursively deal with directory entries
                 fullpath = os.path.join(b_source, i)
-                parsed_this_one = self.parse_source(to_text(fullpath))
+                parsed_this_one = self.parse_source(to_native(fullpath))
                 display.debug(u'parsed %s as %s' % (fullpath, parsed_this_one))
                 if not parsed:
                     parsed = parsed_this_one
@@ -249,29 +249,30 @@ class InventoryManager(object):
             # try source with each plugin
             failures = []
             for plugin in self._inventory_plugins:
-                plugin_name = to_text(getattr(plugin, '_load_name', getattr(plugin, '_original_path', '')))
+                plugin_name = to_native(getattr(plugin, '_load_name', getattr(plugin, '_original_path', '')))
-                display.debug(u'Attempting to use plugin %s' % plugin_name)
+                display.debug(u'Attempting to use plugin %s (%s)' % (plugin_name, plugin._original_path))
 
                 # initialize
                 if plugin.verify_file(source):
                     try:
                         plugin.parse(self._inventory, self._loader, source, cache=cache)
                         parsed = True
-                        display.vvv(u'Parsed %s inventory source with %s plugin' % (to_text(source), plugin_name))
+                        display.vvv('Parsed %s inventory source with %s plugin' % (to_native(source), plugin_name))
                         break
                     except AnsibleParserError as e:
+                        display.debug('%s did not meet %s requirements' % (to_native(source), plugin_name))
                         failures.append({'src': source, 'plugin': plugin_name, 'exc': e})
                 else:
-                    display.debug(u'%s did not meet %s requirements' % (to_text(source), plugin_name))
+                    display.debug('%s did not meet %s requirements' % (to_native(source), plugin_name))
             else:
-                if failures:
+                if not parsed and failures:
                     # only if no plugin processed files should we show errors.
                     for fail in failures:
-                        display.warning(u'\n* Failed to parse %s with %s inventory plugin: %s' % (to_text(fail['src']), fail['plugin'], to_text(fail['exc'])))
+                        display.warning('\n* Failed to parse %s with %s plugin: %s' % (to_native(fail['src']), fail['plugin'], to_native(fail['exc'])))
                         display.vvv(fail['exc'].tb)
 
         if not parsed:
-            display.warning(u"Unable to parse %s as an inventory source" % to_text(source))
+            display.warning("Unable to parse %s as an inventory source" % to_native(source))
 
         # clear up, jic
         self._inventory.current_source = None
@@ -322,10 +323,10 @@ class InventoryManager(object):
             pattern_hash = pattern
 
         if not ignore_limits and self._subset:
-            pattern_hash += u":%s" % to_text(self._subset)
+            pattern_hash += ":%s" % to_native(self._subset)
 
         if not ignore_restrictions and self._restriction:
-            pattern_hash += u":%s" % to_text(self._restriction)
+            pattern_hash += ":%s" % to_native(self._restriction)
 
         if pattern_hash not in self._hosts_patterns_cache:
 

lib/ansible/parsing/dataloader.py

@@ -371,17 +371,10 @@ class DataLoader:
                 b_mydir = os.path.dirname(b_upath)
 
                 # if path is in role and 'tasks' not there already, add it into the search
-                if is_role or self._is_role(path):
+                if (is_role or self._is_role(path)) and b_mydir.endswith(b'tasks'):
-                    if b_mydir.endswith(b'tasks'):
                         search.append(os.path.join(os.path.dirname(b_mydir), b_dirname, b_source))
                         search.append(os.path.join(b_mydir, b_source))
-                    else:
+                else:
-                        # don't add dirname if user already is using it in source
-                        if b_source.split(b'/')[0] != b_dirname:
-                            search.append(os.path.join(b_upath, b_dirname, b_source))
-                        search.append(os.path.join(b_upath, b_source))
-
-                elif b_dirname not in b_source.split(b'/'):
                     # don't add dirname if user already is using it in source
                     if b_source.split(b'/')[0] != dirname:
                         search.append(os.path.join(b_upath, b_dirname, b_source))

lib/ansible/plugins/inventory/__init__.py

@@ -19,12 +19,15 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import os
 import hashlib
+import os
+import re
 import string
 
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleError, AnsibleOptionsError
-from ansible.module_utils._text import to_bytes
+from ansible.module_utils._text import to_bytes, to_native
+from ansible.module_utils.parsing.convert_bool import boolean
+from ansible.module_utils.six import string_types
 from ansible.template import Templar
 
 try:
@@ -33,6 +36,8 @@ except ImportError:
     from ansible.utils.display import Display
     display = Display()
 
+_SAFE_GROUP = re.compile("[^A-Za-z0-9\_]")
+
 
 class BaseInventoryPlugin(object):
     """ Parses an Inventory Source"""
@@ -86,27 +91,62 @@ class BaseInventoryPlugin(object):
         t.set_available_variables(variables)
         return t.do_template('%s%s%s' % (t.environment.variable_start_string, template, t.environment.variable_end_string), disable_lookups=True)
 
-    def _set_composite_vars(self, compose, variables, host):
+    def _set_composite_vars(self, compose, variables, host, strict=False):
         ''' loops over compose entries to create vars for hosts '''
         if compose and isinstance(compose, dict):
             for varname in compose:
-                composite = self._compose(compose[varname], variables)
+                try:
+                    composite = self._compose(compose[varname], variables)
+                except Exception as e:
+                    if strict:
+                        raise AnsibleOptionsError("Could set %s: %s" % (varname, to_native(e)))
+                    continue
                 self.inventory.set_variable(host, varname, composite)
 
-    def _add_host_to_composed_groups(self, groups, variables, host):
+    def _add_host_to_composed_groups(self, groups, variables, host, strict=False):
         ''' helper to create complex groups for plugins based on jinaj2 conditionals, hosts that meet the conditional are added to group'''
         # process each 'group entry'
         if groups and isinstance(groups, dict):
             self.templar.set_available_variables(variables)
             for group_name in groups:
                 conditional = "{%% if %s %%} True {%% else %%} False {%% endif %%}" % groups[group_name]
-                result = self.templar.template(conditional)
+                try:
-                if result and bool(result):
+                    result = boolean(self.templar.template(conditional))
+                except Exception as e:
+                    if strict:
+                        raise AnsibleOptionsError("Could not add to group %s: %s" % (group_name, to_native(e)))
+                    continue
+                if result:
                     # ensure group exists
                     self.inventory.add_group(group_name)
                     # add host to group
                     self.inventory.add_child(group_name, host)
 
+    def _add_host_to_keyed_groups(self, keys, variables, host, strict=False):
+        ''' helper to create groups for plugins based on variable values and add the corresponding hosts to it'''
+        if keys and isinstance(keys, list):
+            for keyed in keys:
+                if keyed and isinstance(keyed, dict):
+                    prefix = keyed.get('prefix', '')
+                    key = keyed.get('key')
+                    if key is not None:
+                        try:
+                            groups = to_safe_group_name('%s_%s' % (prefix, self._compose(key, variables)))
+                        except Exception as e:
+                            if strict:
+                                raise AnsibleOptionsError("Could not generate group on %s: %s" % (key, to_native(e)))
+                            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)
+                                self.inventory.add_child(group_name, host)
+                    else:
+                        raise AnsibleOptionsError("No key supplied, invalid entry")
+                else:
+                    raise AnsibleOptionsError("Invalid keyed group entry, it must be a dictionary: %s " % keyed)
+
 
 class BaseFileInventoryPlugin(BaseInventoryPlugin):
     """ Parses a File based Inventory Source"""
@@ -120,6 +160,11 @@ class BaseFileInventoryPlugin(BaseInventoryPlugin):
 
 
 # Helper methods
+def to_safe_group_name(name):
+    ''' Converts 'bad' characters in a string to underscores so they can be used as Ansible hosts or groups '''
+    return _SAFE_GROUP.sub("_", name)
+
+
 def detect_range(line=None):
     '''
     A helper function that checks a given host line to see if it contains

lib/ansible/plugins/inventory/constructed.py

@@ -27,6 +27,13 @@ DOCUMENTATION:
         - The JInja2 exprpessions are calculated and assigned to the variables
         - Only variables already available from previous inventories can be used for templating.
         - Failed expressions will be ignored (assumes vars were missing).
+    strict:
+        description:
+            - If true make invalid entries a fatal error, otherwise skip and continue
+            - Since it is possible to use facts in the expressions they might not always be available
+              and we ignore those errors by default.
+        type: boolean
+        default: False
     compose:
         description: create vars from jinja2 expressions
         type: dictionary
@@ -35,6 +42,10 @@ DOCUMENTATION:
         description: add hosts to group based on Jinja2 conditionals
         type: dictionary
         default: {}
+    keyed_groups:
+        description: add hosts to group based on the values of a variable
+        type: list
+        default: []
 EXAMPLES: | # inventory.config file in YAML format
     plugin: comstructed
     compose:
@@ -51,6 +62,15 @@ EXAMPLES: | # inventory.config file in YAML format
 
         # complex group membership
         multi_group: (group_names|intersection(['alpha', 'beta', 'omega']))|length >= 2
+
+    keyed_groups:
+        # this creates a group per distro (distro_CentOS, distro_Debian) and assigns the hosts that have matching values to it
+        - prefix: distro
+          key: ansible_distribution
+
+        # this creates a group per ec2 architecture and assign hosts to the matching ones (arch_x86_64, arch_sparc, etc)
+        - prefix: arch
+          key: ec2_architecture
 '''
 
 from __future__ import (absolute_import, division, print_function)
@@ -94,9 +114,12 @@ class InventoryModule(BaseInventoryPlugin):
         except Exception as e:
             raise AnsibleParserError("Unable to parse %s: %s" % (to_native(path), to_native(e)))
 
-        if not data or data.get('plugin') != self.NAME:
+        if not data:
-            raise AnsibleParserError("%s is empty or not a constructed groups config file" % (to_native(path)))
+            raise AnsibleParserError("%s is empty" % (to_native(path)))
+        elif data.get('plugin') != self.NAME:
+            raise AnsibleParserError("%s is not a constructed groups config file, plugin entry must be 'constructed'" % (to_native(path)))
 
+        strict = data.get('strict', False)
         try:
             # Go over hosts (less var copies)
             for host in inventory.hosts:
@@ -107,10 +130,13 @@ class InventoryModule(BaseInventoryPlugin):
                     hostvars = combine_vars(hostvars, inventory.cache[host])
 
                 # create composite vars
-                self._set_composite_vars(data.get('compose'), hostvars, host)
+                self._set_composite_vars(data.get('compose'), hostvars, host, strict=strict)
 
                 # constructed groups based on conditionals
-                self._add_host_to_composed_groups(data.get('groups'), hostvars, host)
+                self._add_host_to_composed_groups(data.get('groups'), hostvars, host, strict=strict)
+
+                # constructed groups based variable values
+                self._add_host_to_keyed_groups(data.get('keyed_groups'), hostvars, host, strict=strict)
 
         except Exception as e:
             raise AnsibleParserError("failed to parse %s: %s " % (to_native(path), to_native(e)))