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 | 12 +++++++++--- lib/cmdlib.py | 5 +++++ lib/utils.py | 4 ++++ scripts/gnt-backup | 6 +++++- scripts/gnt-instance | 6 +++++- test/ganeti.cli_unittest.py | 17 +++++++++++++++++ 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/lib/cli.py b/lib/cli.py index f4f32db..59f99fd 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,15 @@ 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): + retval = (ident[len(NO_PREFIX):], False) + elif ident.startswith(UN_PREFIX): + 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 03660c1..fdebd42 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -5887,6 +5887,9 @@ class LUSetInstanceParams(LogicalUnit): else: if not isinstance(disk_op, int): raise errors.OpPrereqError("Invalid disk index") + if not isinstance(disk_dict, dict): + raise errors.OpPrereqError("Invalid disk values dict") + if disk_op == constants.DDM_ADD: mode = disk_dict.setdefault('mode', constants.DISK_RDWR) if mode not in constants.DISK_ACCESS_SET: @@ -5921,6 +5924,8 @@ class LUSetInstanceParams(LogicalUnit): else: if not isinstance(nic_op, int): raise errors.OpPrereqError("Invalid nic index") + if not isinstance(nic_dict, dict): + raise errors.OpPrereqError("Invalid nic values dict") # 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..543073d 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 = "Invalid dictionary '%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..d279458 100755 --- a/scripts/gnt-backup +++ b/scripts/gnt-backup @@ -104,6 +104,8 @@ def ImportInstance(opts, args): nics = [{}] * nic_max for nidx, ndict in opts.nics.items(): nidx = int(nidx) + if not isinstance(ndict, dict): + raise errors.OpPrereqError("Invalid nic dict for nic %d" % nidx) nics[nidx] = ndict elif opts.no_nics: # no nics @@ -132,7 +134,9 @@ 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): + raise errors.OpPrereqError("Invalid disk dict for disk %d" % didx) + 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..e785409 100755 --- a/scripts/gnt-instance +++ b/scripts/gnt-instance @@ -302,6 +302,8 @@ def AddInstance(opts, args): nics = [{}] * nic_max for nidx, ndict in opts.nics: nidx = int(nidx) + if not isinstance(ndict, dict): + raise errors.OpPrereqError("Invalid nic dict for nic %d" % nidx) nics[nidx] = ndict elif opts.no_nics: # no nics @@ -330,7 +332,9 @@ 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): + raise errors.OpPrereqError("Invalid disk dict for disk %d" % didx) + 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..633d908 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.assertEqual(cikv("no_bar:foo"), ("bar", False)) + self.assertEqual(cikv("no_bar:foo=baz"), ("bar", False)) + self.assertEqual(cikv("-foo"), ("foo", None)) + self.assertEqual(cikv("-foo:a=c"), ("foo", None)) + class TestToStream(unittest.TestCase): """Thes the ToStream functions""" -- 1.5.6.5
