Specify compound matches as lists

Compound matches, such as nodegroups, can now be specified as lists.  This
avoids splitting on whitespace which may be part of a key or value.

Right now this only works for nodegroups.  Making the argument to "-C"
(compound list) a JSON argument is a future task.

doc/topics/targeting/nodegroups.rst

@@ -17,6 +17,10 @@ nodegroups. Here's an example nodegroup configuration within
       group1: 'L@foo.domain.com,bar.domain.com,baz.domain.com or bl*.domain.com'
       group2: 'G@os:Debian and foo.domain.com'
       group3: 'G@os:Debian and N@group1'
+      group4:
+        - 'G@foo:bar'
+        - 'or'
+        - 'G@foo:baz'
 
 .. note::
 
@@ -24,12 +28,22 @@ nodegroups. Here's an example nodegroup configuration within
     group2 is matching specific grains. See the :doc:`compound matchers
     <compound>` documentation for more details.
 
+.. versionadded:: Beryllium
+
 .. note::
 
     Nodgroups can reference other nodegroups as seen in ``group3``.  Ensure
     that you do not have circular references.  Circular references will be
     detected and cause partial expansion with a logged error message.
 
+.. versionadded:: Beryllium
+
+Compound nodegroups can be either string values or lists of string values.
+When the nodegroup is A string value will be tokenized by splitting on
+whitespace.  This may be a problem if whitespace is necessary as part of a
+pattern.  When a nodegroup is a list of strings then tokenization will
+happen for each list element as a whole.
+
 To match a nodegroup on the CLI, use the ``-N`` command-line option:
 
 .. code-block:: bash

salt/minion.py

@@ -74,7 +74,7 @@ import salt.utils.jid
 import salt.pillar
 import salt.utils.args
 import salt.utils.event
-import salt.utils.minion
+import salt.utils.minions
 import salt.utils.schedule
 import salt.utils.error
 import salt.utils.zeromq
@@ -2308,10 +2308,10 @@ class Matcher(object):
         '''
         Runs the compound target check
         '''
-        if not isinstance(tgt, six.string_types):
-            log.debug('Compound target received that is not a string')
+        if not isinstance(tgt, six.string_types) and not isinstance(tgt, (list, tuple)):
+            log.error('Compound target received that is neither string, list nor tuple')
             return False
-        log.debug('compound_match: {0} ? {1}'.format(tgt, self.opts['id']))
+        log.debug('compound_match: {0} ? {1}'.format(self.opts['id'], tgt))
         ref = {'G': 'grain',
                'P': 'grain_pcre',
                'I': 'pillar',
@@ -2325,7 +2325,12 @@ class Matcher(object):
 
         results = []
         opers = ['and', 'or', 'not', '(', ')']
-        words = tgt.split()
+
+        if isinstance(tgt, six.string_types):
+            words = tgt.split()
+        else:
+            words = tgt
+
         for word in words:
             target_info = salt.utils.minions.parse_target(word)
 
@@ -2346,7 +2351,7 @@ class Matcher(object):
                         return False
                     results.append(word)
 
-            elif target_info and target_info.get('engine'):
+            elif target_info and target_info['engine']:
                 if 'N' == target_info['engine']:
                     # Nodegroups should already be expanded/resolved to other engines
                     log.error('Detected nodegroup expansion failure of "{0}"'.format(word))
@@ -2358,13 +2363,13 @@ class Matcher(object):
                               ' target expression "{1}"'.format(
                                   target_info['engine'],
                                   word,
-                              )
-                    )
+                                )
+                        )
                     return False
 
                 engine_args = [target_info['pattern']]
                 engine_kwargs = {}
-                if target_info.get('delimiter'):
+                if target_info['delimiter']:
                     engine_kwargs['delimiter'] = target_info['delimiter']
 
                 results.append(

salt/utils/minions.py

@@ -16,7 +16,6 @@ import salt.payload
 import salt.utils
 from salt.defaults import DEFAULT_TARGET_DELIM
 from salt.exceptions import CommandExecutionError
-from salt._compat import string_types
 
 # Import 3rd-party libs
 import salt.ext.six as six
@@ -45,7 +44,16 @@ def parse_target(target_expression):
      `pattern` - returns a dict'''
 
     match = TARGET_REX.match(target_expression)
-    return match.groupdict() if match else None
+    if not match:
+        log.warning('Unable to parse target "{0}"'.format(target_expression))
+        ret = {
+            'engine': None,
+            'delimiter': None,
+            'pattern': target_expression,
+        }
+    else:
+        ret = match.groupdict()
+    return ret
 
 
 def get_minion_data(minion, opts):
@@ -103,25 +111,37 @@ def nodegroup_comp(nodegroup, nodegroups, skip=None):
         log.error('Failed nodegroup expansion: unknown nodegroup "{0}"'.format(nodegroup))
         return ''
 
-    skip.add(nodegroup)
     nglookup = nodegroups[nodegroup]
+    if isinstance(nglookup, six.string_types):
+        words = nglookup.split()
+    elif isinstance(nglookup, (list, tuple)):
+        words = nglookup
+    else:
+        log.error(
+            'Nodgroup is neither a string, list'
+            ' nor tuple: {0} = {1}'.format(nodegroup, nglookup)
+        )
+        return ''
 
+    skip.add(nodegroup)
     ret = []
     opers = ['and', 'or', 'not', '(', ')']
-    tokens = nglookup.split()
-    for match in tokens:
-        if match in opers:
-            ret.append(match)
-        elif len(match) >= 3 and match.startswith('N@'):
-            ret.append(nodegroup_comp(match[2:], nodegroups, skip=skip))
+    for word in words:
+        if word in opers:
+            ret.append(word)
+        elif len(word) >= 3 and word.startswith('N@'):
+            ret.extend(nodegroup_comp(word[2:], nodegroups, skip=skip))
         else:
-            ret.append(match)
+            ret.append(word)
+
+    if ret:
+        ret.insert(0, '(')
+        ret.append(')')
 
     skip.remove(nodegroup)
 
-    expanded = '( {0} )'.format(' '.join(ret)) if ret else ''
-    log.debug('nodegroup_comp("{0}") => {1}'.format(nodegroup, expanded))
-    return expanded
+    log.debug('nodegroup_comp({0}) => {1}'.format(nodegroup, ret))
+    return ret
 
 
 class CkMinions(object):
@@ -157,12 +177,12 @@ class CkMinions(object):
         '''
         Return the minions found by looking via a list
         '''
-        if isinstance(expr, string_types):
+        if isinstance(expr, six.string_types):
             expr = [m for m in expr.split(',') if m]
         ret = []
-        for m in expr:
-            if os.path.isfile(os.path.join(self.opts['pki_dir'], self.acc, m)):
-                ret.append(m)
+        for minion in expr:
+            if os.path.isfile(os.path.join(self.opts['pki_dir'], self.acc, minion)):
+                ret.append(minion)
         return ret
 
     def _check_pcre_minions(self, expr, greedy):  # pylint: disable=unused-argument
@@ -362,6 +382,9 @@ class CkMinions(object):
         Return the minions found by looking via compound matcher
         '''
         log.debug('_check_compound_minions({0}, {1}, {2}, {3})'.format(expr, delimiter, greedy, pillar_exact))
+        if not isinstance(expr, six.string_types) and not isinstance(expr, (list, tuple)):
+            log.error('Compound target that is neither string, list nor tuple')
+            return []
         minions = set(
             os.listdir(os.path.join(self.opts['pki_dir'], self.acc))
         )
@@ -384,7 +407,12 @@ class CkMinions(object):
             results = []
             unmatched = []
             opers = ['and', 'or', 'not', '(', ')']
-            words = expr.split()
+
+            if isinstance(expr, six.string_types):
+                words = expr.split()
+            else:
+                words = expr
+
             for word in words:
                 target_info = parse_target(word)
 
@@ -440,7 +468,7 @@ class CkMinions(object):
                             )
                             return []
 
-                elif target_info and target_info.get('engine'):
+                elif target_info and target_info['engine']:
                     if 'N' == target_info['engine']:
                         # Nodegroups should already be expanded/resolved to other engines
                         log.error('Detected nodegroup expansion failure of "{0}"'.format(word))
@@ -457,9 +485,9 @@ class CkMinions(object):
                         )
                         return []
 
-                    engine_args = [target_info.get('pattern')]
+                    engine_args = [target_info['pattern']]
                     if target_info['engine'] in ('G', 'P', 'I', 'J'):
-                        engine_args.append(target_info.get('delimiter') or ':')
+                        engine_args.append(target_info['delimiter'] or ':')
                     engine_args.append(True)
 
                     results.append(str(set(engine(*engine_args))))
@@ -474,8 +502,9 @@ class CkMinions(object):
                         results.append(')')
                         unmatched.pop()
 
-            for token in unmatched:
-                results.append(')')
+            # Add a closing ')' for each item left in unmatched
+            results.extend([')' for item in unmatched])
+
             results = ' '.join(results)
             log.debug('Evaluating final compound matching expr: {0}'
                       .format(results))
@@ -484,6 +513,7 @@ class CkMinions(object):
             except Exception:
                 log.error('Invalid compound target: {0}'.format(expr))
                 return []
+
         return list(minions)
 
     def connected_ids(self, subset=None, show_ipv4=False):
@@ -581,8 +611,8 @@ class CkMinions(object):
         if not target_info:
             log.error('Failed to parse valid target "{0}"'.format(valid))
 
-        v_matcher = ref.get(target_info.get('engine'))
-        v_expr = target_info.get('pattern')
+        v_matcher = ref.get(target_info['engine'])
+        v_expr = target_info['pattern']
 
         if v_matcher in infinite:
             # We can't be sure what the subset is, only match the identical
@@ -663,7 +693,7 @@ class CkMinions(object):
         try:
             for fun in funs:
                 for ind in auth_list:
-                    if isinstance(ind, string_types):
+                    if isinstance(ind, six.string_types):
                         # Allowed for all minions
                         if self.match_check(ind, fun):
                             return True
@@ -678,7 +708,7 @@ class CkMinions(object):
                                 tgt,
                                 tgt_type):
                             # Minions are allowed, verify function in allowed list
-                            if isinstance(ind[valid], string_types):
+                            if isinstance(ind[valid], six.string_types):
                                 if self.match_check(ind[valid], fun):
                                     return True
                             elif isinstance(ind[valid], list):
@@ -713,7 +743,7 @@ class CkMinions(object):
         mod = comps[0]
         fun = comps[1]
         for ind in auth_list:
-            if isinstance(ind, string_types):
+            if isinstance(ind, six.string_types):
                 if ind.startswith('@') and ind[1:] == mod:
                     return True
                 if ind == '@wheel':
@@ -725,7 +755,7 @@ class CkMinions(object):
                     continue
                 valid = next(six.iterkeys(ind))
                 if valid.startswith('@') and valid[1:] == mod:
-                    if isinstance(ind[valid], string_types):
+                    if isinstance(ind[valid], six.string_types):
                         if self.match_check(ind[valid], fun):
                             return True
                     elif isinstance(ind[valid], list):
@@ -744,7 +774,7 @@ class CkMinions(object):
         mod = comps[0]
         fun = comps[1]
         for ind in auth_list:
-            if isinstance(ind, string_types):
+            if isinstance(ind, six.string_types):
                 if ind.startswith('@') and ind[1:] == mod:
                     return True
                 if ind == '@runners':
@@ -756,7 +786,7 @@ class CkMinions(object):
                     continue
                 valid = next(six.iterkeys(ind))
                 if valid.startswith('@') and valid[1:] == mod:
-                    if isinstance(ind[valid], string_types):
+                    if isinstance(ind[valid], six.string_types):
                         if self.match_check(ind[valid], fun):
                             return True
                     elif isinstance(ind[valid], list):
@@ -778,7 +808,7 @@ class CkMinions(object):
         else:
             mod = fun
         for ind in auth_list:
-            if isinstance(ind, string_types):
+            if isinstance(ind, six.string_types):
                 if ind.startswith('@') and ind[1:] == mod:
                     return True
                 if ind == '@{0}'.format(form):
@@ -790,7 +820,7 @@ class CkMinions(object):
                     continue
                 valid = next(six.iterkeys(ind))
                 if valid.startswith('@') and valid[1:] == mod:
-                    if isinstance(ind[valid], string_types):
+                    if isinstance(ind[valid], six.string_types):
                         if self.match_check(ind[valid], fun):
                             return True
                     elif isinstance(ind[valid], list):

tests/integration/files/conf/master

@@ -63,6 +63,10 @@ nodegroups:
   min: minion
   sub_min: sub_minion
   mins: N@min or N@sub_min
+  multiline_nodegroup:
+    - 'minion'
+    - 'or'
+    - 'sub_minion'
   redundant_minions: N@min or N@mins
   nodegroup_loop_a: N@nodegroup_loop_b
   nodegroup_loop_b: N@nodegroup_loop_a

tests/integration/shell/matcher.py

@@ -115,6 +115,10 @@ class MatchTest(integration.ShellCase, integration.ShellCaseCommonTestsMixIn):
         time.sleep(2)
         data = '\n'.join(self.run_salt('-N nodegroup_loop_a test.ping'))
         self.assertIn('No minions matched', data)
+        time.sleep(2)
+        data = self.run_salt("-N multiline_nodegroup test.ping")
+        self.assertTrue(minion_in_returns('minion', data))
+        self.assertTrue(minion_in_returns('sub_minion', data))
 
     def test_glob(self):
         '''