If an ident member of an IdentKeyVal relationship starts with no_ or -, handle it the same way we do for a key. Some unittests are added to check that check_ident_key_val behaves as expected.
This patch also changes ForceDictType to, for now, fail on such an entry, and the same to happen when creating an instance or modifying its nics or disks. This behavior will be used later on to allow deletion of os entries in os parameters. Signed-off-by: Guido Trotter <[email protected]> --- lib/cli.py | 16 +++++++++++++--- lib/cmdlib.py | 7 +++++++ lib/utils.py | 4 ++++ scripts/gnt-backup | 8 +++++++- scripts/gnt-instance | 8 +++++++- test/ganeti.cli_unittest.py | 17 +++++++++++++++++ 6 files changed, 55 insertions(+), 5 deletions(-) diff --git a/lib/cli.py b/lib/cli.py index f4f32db..547d25e 100644 --- a/lib/cli.py +++ b/lib/cli.py @@ -54,6 +54,8 @@ __all__ = ["DEBUG_OPT", "NOHDR_OPT", "SEP_OPT", "GenericMain", "GetOnlineNodes", "JobExecutor", "SYNC_OPT", "CONFIRM_OPT", ] +NO_PREFIX = "no_" +UN_PREFIX = "-" def _ExtractTagsObject(opts, args): """Extract the tag type object. @@ -256,8 +258,6 @@ def _SplitKeyVal(opt, data): @raises errors.ParameterError: if there are duplicate keys """ - NO_PREFIX = "no_" - UN_PREFIX = "-" kv_dict = {} if data: for elem in data.split(","): @@ -282,9 +282,19 @@ def check_ident_key_val(option, opt, value): """ if ":" not in value: - retval = (value, {}) + ident, rest = value, '' else: ident, rest = value.split(":", 1) + + if ident.startswith(NO_PREFIX): + if rest: + raise errors.ParameterError("Invalid parameter: %s" % value) + retval = (ident[len(NO_PREFIX):], False) + elif ident.startswith(UN_PREFIX): + if rest: + raise errors.ParameterError("Invalid parameter: %s" % value) + retval = (ident[len(UN_PREFIX):], None) + else: kv_dict = _SplitKeyVal(opt, rest) retval = (ident, kv_dict) return retval diff --git a/lib/cmdlib.py b/lib/cmdlib.py index 91a35a3..7f47b45 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -5887,6 +5887,10 @@ class LUSetInstanceParams(LogicalUnit): else: if not isinstance(disk_op, int): raise errors.OpPrereqError("Invalid disk index") + if not isinstance(disk_dict, dict): + msg = "Invalid disk value: expected dict, got '%s'" % disk_dict + raise errors.OpPrereqError(msg) + if disk_op == constants.DDM_ADD: mode = disk_dict.setdefault('mode', constants.DISK_RDWR) if mode not in constants.DISK_ACCESS_SET: @@ -5921,6 +5925,9 @@ class LUSetInstanceParams(LogicalUnit): else: if not isinstance(nic_op, int): raise errors.OpPrereqError("Invalid nic index") + if not isinstance(nic_dict, dict): + msg = "Invalid nic value: expected dict, got '%s'" % nic_dict + raise errors.OpPrereqError(msg) # nic_dict should be a dict nic_ip = nic_dict.get('ip', None) diff --git a/lib/utils.py b/lib/utils.py index 44bd0e7..b552915 100644 --- a/lib/utils.py +++ b/lib/utils.py @@ -388,6 +388,10 @@ def ForceDictType(target, key_types, allowed_values=None): if allowed_values is None: allowed_values = [] + if not isinstance(target, dict): + msg = "Expected dictionary, got '%s'" % target + raise errors.TypeEnforcementError(msg) + for key in target: if key not in key_types: msg = "Unknown key '%s'" % key diff --git a/scripts/gnt-backup b/scripts/gnt-backup index e78abba..f2e710b 100755 --- a/scripts/gnt-backup +++ b/scripts/gnt-backup @@ -104,6 +104,9 @@ def ImportInstance(opts, args): nics = [{}] * nic_max for nidx, ndict in opts.nics.items(): nidx = int(nidx) + if not isinstance(ndict, dict): + msg = "Invalid nic/%d value: expected dict got %s" % (nidx, ndict) + raise errors.OpPrereqError(msg) nics[nidx] = ndict elif opts.no_nics: # no nics @@ -132,7 +135,10 @@ def ImportInstance(opts, args): disks = [{}] * disk_max for didx, ddict in opts.disks: didx = int(didx) - if "size" not in ddict: + if not isinstance(ddict, dict): + msg = "Invalid disk/%d value: expected dict got %s" % (didx, ddict) + raise errors.OpPrereqError(msg) + elif "size" not in ddict: raise errors.OpPrereqError("Missing size for disk %d" % didx) try: ddict["size"] = utils.ParseUnit(ddict["size"]) diff --git a/scripts/gnt-instance b/scripts/gnt-instance index 12152e8..72ee722 100755 --- a/scripts/gnt-instance +++ b/scripts/gnt-instance @@ -302,6 +302,9 @@ def AddInstance(opts, args): nics = [{}] * nic_max for nidx, ndict in opts.nics: nidx = int(nidx) + if not isinstance(ndict, dict): + msg = "Invalid nic/%d value: expected dict got %s" % (nidx, ndict) + raise errors.OpPrereqError(msg) nics[nidx] = ndict elif opts.no_nics: # no nics @@ -330,7 +333,10 @@ def AddInstance(opts, args): disks = [{}] * disk_max for didx, ddict in opts.disks: didx = int(didx) - if "size" not in ddict: + if not isinstance(ddict, dict): + msg = "Invalid disk/%d value: expected dict got %s" % (didx, ddict) + raise errors.OpPrereqError(msg) + elif "size" not in ddict: raise errors.OpPrereqError("Missing size for disk %d" % didx) try: ddict["size"] = utils.ParseUnit(ddict["size"]) diff --git a/test/ganeti.cli_unittest.py b/test/ganeti.cli_unittest.py index b86486b..cf3810d 100755 --- a/test/ganeti.cli_unittest.py +++ b/test/ganeti.cli_unittest.py @@ -79,6 +79,23 @@ class TestSplitKeyVal(unittest.TestCase): """Test how we handle splitting an empty string""" self.failUnlessEqual(cli._SplitKeyVal("option", ""), {}) +class TestIdentKeyVal(unittest.TestCase): + """Testing case for cli.check_ident_key_val""" + + def testIdentKeyVal(self): + """Test identkeyval""" + def cikv(value): + return cli.check_ident_key_val("option", "opt", value) + + self.assertEqual(cikv("foo:bar"), ("foo", {"bar": True})) + self.assertEqual(cikv("foo:bar=baz"), ("foo", {"bar": "baz"})) + self.assertEqual(cikv("bar:b=c,c=a"), ("bar", {"b": "c", "c": "a"})) + self.assertEqual(cikv("no_bar"), ("bar", False)) + self.assertRaises(ParameterError, cikv, "no_bar:foo") + self.assertRaises(ParameterError, cikv, "no_bar:foo=baz") + self.assertEqual(cikv("-foo"), ("foo", None)) + self.assertRaises(ParameterError, cikv, "-foo:a=c") + class TestToStream(unittest.TestCase): """Thes the ToStream functions""" -- 1.5.6.5
