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 <sfinucan@redhat.com>
Story: #2006628
Task: #36840
Related-Bug: #1845322
This commit is contained in:
Stephen Finucane 2019-09-27 12:19:29 +01:00
parent 70ab3f9dd5
commit ea27ebb0f9
2 changed files with 61 additions and 12 deletions

View File

@ -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='<key=value>',
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

View File

@ -860,7 +860,7 @@ class TestServerCreate(TestServer):
('key_name', 'keyname'),
('property', {'Beta': 'b'}),
('security_group', ['securitygroup']),
('hint', ['a=b', 'a=c']),
('hint', {'a': ['b', 'c']}),
('config_drive', False),
('server_name', self.new_server.name),
]
@ -2060,6 +2060,29 @@ class TestServerCreate(TestServer):
self.cmd.take_action,
parsed_args)
def test_server_create_invalid_hint(self):
# Not a key-value pair
arglist = [
'--image', 'image1',
'--flavor', 'flavor1',
'--hint', 'a0cf03a5-d921-4877-bb5c-86d26cf818e1',
self.new_server.name,
]
self.assertRaises(argparse.ArgumentTypeError,
self.check_parser,
self.cmd, arglist, [])
# Empty key
arglist = [
'--image', 'image1',
'--flavor', 'flavor1',
'--hint', '=a0cf03a5-d921-4877-bb5c-86d26cf818e1',
self.new_server.name,
]
self.assertRaises(argparse.ArgumentTypeError,
self.check_parser,
self.cmd, arglist, [])
def test_server_create_with_description_api_newer(self):
# Description is supported for nova api version 2.19 or above