From ea27ebb0f918db9eab2f5751a1b065818faa0e6d Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 27 Sep 2019 12:19:29 +0100 Subject: Stop silently ignoring invalid 'server create --hint' options The '--hint' option for 'server create' expects a key-value pair like so: openstack server create --hint group=245e1dfe-2d0e-4139-80a9-fce124948896 ... However, the command doesn't complain if this isn't the case, meaning typos like the below aren't indicated to the user: openstack server create --hint 245e1dfe-2d0e-4139-80a9-fce124948896 Due to how we'd implemented this here, this ultimately results in us POSTing the following as part of the body to 'os-servers': { ... "OS-SCH-HNT:scheduler_hints": { "245e1dfe-2d0e-4139-80a9-fce124948896": null } ... } Which is unfortunately allowed and ignored by nova due to the use of 'additionalProperties' in the schema [1] Do what we do for loads of other options and explicitly fail on invalid values. This involves adding a new argparse action since none of those defined in osc-lib work for us. This is included here to ease backporting of the fix but will be moved to osc-lib in a future patch. [1] https://github.com/openstack/nova/blob/19.0.0/nova/api/openstack/compute/schemas/servers.py#L142-L146 Change-Id: I9e96d2978912c8dfeadae4a782c481a17cd7e348 Signed-off-by: Stephen Finucane Story: #2006628 Task: #36840 Related-Bug: #1845322 --- openstackclient/compute/v2/server.py | 48 +++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 11 deletions(-) (limited to 'openstackclient/compute') diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index b5c420fe..a4cca661 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -201,6 +201,36 @@ def _prep_server_detail(compute_client, image_client, server, refresh=True): return info +# TODO(stephenfin): Migrate this to osc-lib +class KeyValueAppendAction(argparse.Action): + """A custom action to parse arguments as key=value pairs + + Ensures that ``dest`` is a dict and values are lists of strings. + """ + + def __call__(self, parser, namespace, values, option_string=None): + # Make sure we have an empty dict rather than None + if getattr(namespace, self.dest, None) is None: + setattr(namespace, self.dest, {}) + + # Add value if an assignment else remove it + if '=' in values: + key, value = values.split('=', 1) + # NOTE(qtang): Prevent null key setting in property + if '' == key: + msg = _("Property key must be specified: %s") + raise argparse.ArgumentTypeError(msg % str(values)) + + dest = getattr(namespace, self.dest) + if key in dest: + dest[key].append(value) + else: + dest[key] = [value] + else: + msg = _("Expected 'key=value' type, but got: %s") + raise argparse.ArgumentTypeError(msg % str(values)) + + class AddFixedIP(command.Command): _description = _("Add fixed IP address to server") @@ -689,8 +719,8 @@ class CreateServer(command.ShowOne): parser.add_argument( '--hint', metavar='', - action='append', - default=[], + action=KeyValueAppendAction, + default={}, help=_('Hints for the scheduler (optional extension)'), ) parser.add_argument( @@ -986,16 +1016,12 @@ class CreateServer(command.ShowOne): security_group_names.append(sg['name']) hints = {} - for hint in parsed_args.hint: - key, _sep, value = hint.partition('=') - # NOTE(vish): multiple copies of the same hint will - # result in a list of values - if key in hints: - if isinstance(hints[key], six.string_types): - hints[key] = [hints[key]] - hints[key] += [value] + for key, values in parsed_args.hint.items(): + # only items with multiple values will result in a list + if len(values) == 1: + hints[key] = values[0] else: - hints[key] = value + hints[key] = values # What does a non-boolean value for config-drive do? # --config-drive argument is either a volume id or -- cgit v1.2.1