Detect globbing patterns as query arguments

Short: this patch enables the use of “gnt-instance list '*.site'”.

Detailed description: This patch changes the command line interface code
to try to deduce the kind of filter from the arguments to a “list”
command. If it's a list of plain names an old-style name filter is used.
If filtering is forced or the single argument is potentially a filter,
it is parsed as a query filter string. Any name looking like a globbing
pattern (e.g. “*.site” or “web?.example.com”) is treated as such.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

diff --git a/lib/cli.py b/lib/cli.py
index 52d5780..4aeb162 100644 (file)
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -2661,18 +2661,7 @@ def GenericList(resource, fields, names, unit, separator, header, cl=None,
   if not names:
     names = None
 
-  if (force_filter or
-      (names and len(names) == 1 and qlang.MaybeFilter(names[0]))):
-    try:
-      (filter_text, ) = names
-    except ValueError:
-      raise errors.OpPrereqError("Exactly one argument must be given as a"
-                                 " filter")
-
-    logging.debug("Parsing '%s' as filter", filter_text)
-    filter_ = qlang.ParseFilter(filter_text)
-  else:
-    filter_ = qlang.MakeSimpleFilter("name", names)
+  filter_ = qlang.MakeFilter(names, force_filter)
 
   response = cl.Query(resource, fields, filter_)
 

diff --git a/lib/qlang.py b/lib/qlang.py
index 0c5169e..b0bc07d 100644 (file)
--- a/lib/qlang.py
+++ b/lib/qlang.py
@@ -33,12 +33,14 @@ converted to callable functions by L{query._CompileFilter}.
 
 import re
 import string # pylint: disable-msg=W0402
+import logging
 
 import pyparsing as pyp
 
 from ganeti import errors
 from ganeti import netutils
 from ganeti import utils
+from ganeti import compat
 
 
 # Logic operators with one or more operands, each of which is a filter on its
@@ -61,7 +63,10 @@ OP_CONTAINS = "=[]"
 
 
 #: Characters used for detecting user-written filters (see L{MaybeFilter})
-FILTER_DETECTION_CHARS = frozenset("()=/!~" + string.whitespace)
+FILTER_DETECTION_CHARS = frozenset("()=/!~'\"\\" + string.whitespace)
+
+#: Characters used to detect globbing filters (see L{MaybeGlobbing})
+GLOB_DETECTION_CHARS = frozenset("*?")
 
 
 def MakeSimpleFilter(namefield, values):
@@ -233,6 +238,8 @@ def ParseFilter(text, parser=None):
   @rtype: list
 
   """
+  logging.debug("Parsing as query filter: %s", text)
+
   if parser is None:
     parser = BuildFilterParser()
 
@@ -243,25 +250,75 @@ def ParseFilter(text, parser=None):
                                        " '%s': %s" % (text, err), err)
 
 
-def MaybeFilter(text):
-  """Try to determine if a string is a filter or a name.
+def _IsHostname(text):
+  """Checks if a string could be a hostname.
 
-  If in doubt, this function treats a text as a name.
-
-  @type text: string
-  @param text: String to be examined
   @rtype: bool
 
   """
-  # Quick check for punctuation and whitespace
-  if frozenset(text) & FILTER_DETECTION_CHARS:
-    return True
-
   try:
     netutils.Hostname.GetNormalizedName(text)
   except errors.OpPrereqError:
-    # Not a valid hostname, treat as filter
+    return False
+  else:
     return True
 
-  # Most probably a name
-  return False
+
+def _CheckFilter(text):
+  """CHecks if a string could be a filter.
+
+  @rtype: bool
+
+  """
+  return bool(frozenset(text) & FILTER_DETECTION_CHARS)
+
+
+def _CheckGlobbing(text):
+  """Checks if a string could be a globbing pattern.
+
+  @rtype: bool
+
+  """
+  return bool(frozenset(text) & GLOB_DETECTION_CHARS)
+
+
+def _MakeFilterPart(namefield, text):
+  """Generates filter for one argument.
+
+  """
+  if _CheckGlobbing(text):
+    return [OP_REGEXP, namefield, utils.DnsNameGlobPattern(text)]
+  else:
+    return [OP_EQUAL, namefield, text]
+
+
+def MakeFilter(args, force_filter):
+  """Try to make a filter from arguments to a command.
+
+  If the name could be a filter it is parsed as such. If it's just a globbing
+  pattern, e.g. "*.site", such a filter is constructed. As a last resort the
+  names are treated just as a plain name filter.
+
+  @type args: list of string
+  @param args: Arguments to command
+  @type force_filter: bool
+  @param force_filter: Whether to force treatment as a full-fledged filter
+  @rtype: list
+  @return: Query filter
+
+  """
+  if (force_filter or
+      (args and len(args) == 1 and _CheckFilter(args[0]))):
+    try:
+      (filter_text, ) = args
+    except (TypeError, ValueError):
+      raise errors.OpPrereqError("Exactly one argument must be given as a"
+                                 " filter")
+
+    result = ParseFilter(filter_text)
+  elif args:
+    result = [OP_OR] + map(compat.partial(_MakeFilterPart, "name"), args)
+  else:
+    result = None
+
+  return result

diff --git a/test/ganeti.qlang_unittest.py b/test/ganeti.qlang_unittest.py
index ed8f77f..c781289 100755 (executable)
--- a/test/ganeti.qlang_unittest.py
+++ b/test/ganeti.qlang_unittest.py
@@ -54,13 +54,7 @@ class TestParseFilter(unittest.TestCase):
     self.parser = qlang.BuildFilterParser()
 
   def _Test(self, filter_, expected, expect_filter=True):
-    if expect_filter:
-      self.assertTrue(qlang.MaybeFilter(filter_),
-                      msg="'%s' was not recognized as a filter" % filter_)
-    else:
-      self.assertFalse(qlang.MaybeFilter(filter_),
-                       msg=("'%s' should not be recognized as a filter" %
-                            filter_))
+    self.assertEqual(qlang.MakeFilter([filter_], not expect_filter), expected)
     self.assertEqual(qlang.ParseFilter(filter_, parser=self.parser), expected)
 
   def test(self):
@@ -182,21 +176,62 @@ class TestParseFilter(unittest.TestCase):
         self.fail("Invalid filter '%s' did not raise exception" % filter_)
 
 
-class TestMaybeFilter(unittest.TestCase):
-  def test(self):
-    self.assertTrue(qlang.MaybeFilter(""))
-    self.assertTrue(qlang.MaybeFilter("foo/bar"))
-    self.assertTrue(qlang.MaybeFilter("foo==bar"))
-
-    for i in set("()!~" + string.whitespace) | qlang.FILTER_DETECTION_CHARS:
-      self.assertTrue(qlang.MaybeFilter(i),
-                      msg="%r not recognized as filter" % i)
-
-    self.assertFalse(qlang.MaybeFilter("node1"))
-    self.assertFalse(qlang.MaybeFilter("n-o-d-e"))
-    self.assertFalse(qlang.MaybeFilter("n_o_d_e"))
-    self.assertFalse(qlang.MaybeFilter("node1.example.com"))
-    self.assertFalse(qlang.MaybeFilter("node1.example.com."))
+class TestMakeFilter(unittest.TestCase):
+  def testNoNames(self):
+    self.assertEqual(qlang.MakeFilter([], False), None)
+    self.assertEqual(qlang.MakeFilter(None, False), None)
+
+  def testPlainNames(self):
+    self.assertEqual(qlang.MakeFilter(["web1", "web2"], False),
+                     [qlang.OP_OR, [qlang.OP_EQUAL, "name", "web1"],
+                                   [qlang.OP_EQUAL, "name", "web2"]])
+
+  def testForcedFilter(self):
+    for i in [None, [], ["1", "2"], ["", "", ""], ["a", "b", "c", "d"]]:
+      self.assertRaises(errors.OpPrereqError, qlang.MakeFilter, i, True)
+
+    # Glob pattern shouldn't parse as filter
+    self.assertRaises(errors.QueryFilterParseError,
+                      qlang.MakeFilter, ["*.site"], True)
+
+    # Plain name parses as boolean filter
+    self.assertEqual(qlang.MakeFilter(["web1"], True), [qlang.OP_TRUE, "web1"])
+
+  def testFilter(self):
+    self.assertEqual(qlang.MakeFilter(["foo/bar"], False),
+                     [qlang.OP_TRUE, "foo/bar"])
+    self.assertEqual(qlang.MakeFilter(["foo=='bar'"], False),
+                     [qlang.OP_EQUAL, "foo", "bar"])
+    self.assertEqual(qlang.MakeFilter(["field=*'*.site'"], False),
+                     [qlang.OP_REGEXP, "field",
+                      utils.DnsNameGlobPattern("*.site")])
+
+    # Plain name parses as name filter, not boolean
+    for name in ["node1", "n-o-d-e", "n_o_d_e", "node1.example.com",
+                 "node1.example.com."]:
+      self.assertEqual(qlang.MakeFilter([name], False),
+                       [qlang.OP_OR, [qlang.OP_EQUAL, "name", name]])
+
+    # Invalid filters
+    for i in ["foo==bar", "foo+=1"]:
+      self.assertRaises(errors.QueryFilterParseError,
+                        qlang.MakeFilter, [i], False)
+
+  def testGlob(self):
+    self.assertEqual(qlang.MakeFilter(["*.site"], False),
+                     [qlang.OP_OR, [qlang.OP_REGEXP, "name",
+                                    utils.DnsNameGlobPattern("*.site")]])
+    self.assertEqual(qlang.MakeFilter(["web?.example"], False),
+                     [qlang.OP_OR, [qlang.OP_REGEXP, "name",
+                                    utils.DnsNameGlobPattern("web?.example")]])
+    self.assertEqual(qlang.MakeFilter(["*.a", "*.b", "?.c"], False),
+                     [qlang.OP_OR,
+                      [qlang.OP_REGEXP, "name",
+                       utils.DnsNameGlobPattern("*.a")],
+                      [qlang.OP_REGEXP, "name",
+                       utils.DnsNameGlobPattern("*.b")],
+                      [qlang.OP_REGEXP, "name",
+                       utils.DnsNameGlobPattern("?.c")]])
 
 
 if __name__ == "__main__":