ceph_argparse: increment matchcnt on kwargs

Currently when you pass a param in on the ceph cli as a kwarg
(--<param_name>) the matchcnt isn't incremented in the validate method
which is used to choose the right command signature.

This means currently things fail like:

  ceph orchestrator rgw rm --realm_name=default --zone_name=default
  no valid command found; 1 closest matches:
  orchestrator rgw rm <realm_name> <zone_name>
  Error EINVAL: invalid command

The '--realm_name' and '--zone_name' isn't counted to the matchcnt, so
'orchestrator rgw rm' isn't picked as the valid command.

This patch simply corrects this by incrementing matchcnt on the kwarg
validate path before calling shortcircuiting the loop.

Fixes: https://tracker.ceph.com/issues/43803
Signed-off-by: Matthew Oliver <moliver@suse.com>

diff --git a/src/pybind/ceph_argparse.py b/src/pybind/ceph_argparse.py
index 107f4d88fa4826f204bb0948a5e5b9ee8814959e..4c65761f51e3214b8f1042803f496f68c791be7f 100644 (file)
--- a/src/pybind/ceph_argparse.py
+++ b/src/pybind/ceph_argparse.py
@@ -1022,6 +1022,7 @@ def validate(args, signature, flags=0, partial=False):
 
                 if kwarg_desc:
                     validate_one(kwarg_v, kwarg_desc)
+                    matchcnt += 1
                     store_arg(kwarg_desc, d)
                     continue
 
@@ -1123,7 +1124,7 @@ def validate_command(sigdict, args, verbose=False):
     Parse positional arguments into a parameter dict, according to
     the command descriptions.
 
-    Writes advice about nearly-matching commands ``sys.stderr`` if 
+    Writes advice about nearly-matching commands ``sys.stderr`` if
     the arguments do not match any command.
 
     :param sigdict: A command description dictionary, as returned

diff --git a/src/test/pybind/test_ceph_argparse.py b/src/test/pybind/test_ceph_argparse.py
index ed38c59bd7989cff4e332bbc2b05aa748e8a65c7..2b66dbeaab707b9fd91f1f3c524f08e5d1cde079 100755 (executable)
--- a/src/test/pybind/test_ceph_argparse.py
+++ b/src/test/pybind/test_ceph_argparse.py
@@ -17,11 +17,16 @@
 
 from nose.tools import eq_ as eq
 from nose.tools import *
+from unittest import TestCase
 
-from ceph_argparse import validate_command, parse_json_funcsigs
+from ceph_argparse import validate_command, parse_json_funcsigs, validate, \
+    parse_funcsig, ArgumentError, ArgumentTooFew, ArgumentMissing, \
+    ArgumentNumber, ArgumentValid
 
 import os
+import random
 import re
+import string
 import sys
 import json
 try:
@@ -1230,6 +1235,74 @@ class TestConfigKey(TestArgparse):
     def test_list(self):
         self.check_no_arg('config-key', 'list')
 
+
+class TestValidate(TestCase):
+
+    ARGS = 0
+    KWARGS = 1
+    KWARGS_EQ = 2
+    MIXED = 3
+
+    def setUp(self):
+        self.prefix = ['some', 'random', 'cmd']
+        self.args_dict = [
+            {'name': 'variable_one', 'type': 'CephString'},
+            {'name': 'variable_two', 'type': 'CephString'},
+            {'name': 'variable_three', 'type': 'CephString'},
+            {'name': 'variable_four', 'type': 'CephInt'},
+            {'name': 'variable_five', 'type': 'CephString'}]
+        self.args = []
+        for d in self.args_dict:
+            if d['type'] == 'CephInt':
+                val = "{}".format(random.randint(0, 100))
+            elif d['type'] == 'CephString':
+                letters = string.ascii_letters
+                str_len = random.randint(5, 10)
+                val = ''.join(random.choice(letters) for _ in range(str_len))
+            else:
+                self.skipTest()
+
+            self.args.append((d['name'], val))
+
+        self.sig = parse_funcsig(self.prefix + self.args_dict)
+
+    def arg_kwarg_test(self, prefix, args, sig, arg_type=0):
+        """
+        Runs validate in different arg/kargs ways.
+
+        :param prefix: List of prefix commands (that can't be kwarged)
+        :param args: a list of kwarg, arg pairs: [(k1, v1), (k2, v2), ...]
+        :param sig: The sig to match
+        :param arg_type: how to build the args to send. As positional args (ARGS),
+                     as long kwargs (KWARGS [--k v]), other style long kwargs
+                     (KWARGS_EQ (--k=v]), and mixed (MIXED) where there will be
+                     a random mix of the above.
+        :return: None, the method will assert.
+        """
+        final_args = list(prefix)
+        for k, v in args:
+            a_type = arg_type
+            if a_type == self.MIXED:
+                a_type = random.choice((self.ARGS,
+                                          self.KWARGS,
+                                          self.KWARGS_EQ))
+            if a_type == self.ARGS:
+                final_args.append(v)
+            elif a_type == self.KWARGS:
+                final_args.extend(["--{}".format(k), v])
+            else:
+                final_args.append("--{}={}".format(k, v))
+
+        try:
+            validate(final_args, sig)
+        except (ArgumentError, ArgumentMissing,
+                ArgumentNumber, ArgumentTooFew, ArgumentValid) as ex:
+            self.fail("Validation failed: {}".format(str(ex)))
+
+    def test_args_and_kwargs_validate(self):
+        for arg_type in (self.ARGS, self.KWARGS, self.KWARGS_EQ, self.MIXED):
+            self.arg_kwarg_test(self.prefix, self.args, self.sig, arg_type)
+
 # Local Variables:
 # compile-command: "cd ../../..; cmake --build build --target get_command_descriptions -j4 &&
 #  CEPH_BIN=build/bin \