Return HTTP 400 for invalid server-group uuid

Nova API checks that the value of the group scheduler hint
shall be a valid server-group uuid, however it was done by custom
code not jsonschema. Moreover the exception was not handled by the API
so both v2.0 and v.2.1 returned HTTP 500 if the group hint wasn't a valid
uuid of an existing server group.

The custom code to check for the validity of the group uuid is kept in
nova.compute.api as it is still needed in v2.0.

In v2.0 InstanceGroupNotFound exception are now translated to HTTPBadRequest.

In v2.1 the scheduler_hint jsonschema is now extended to check the format
of the group hint and the api is now translates the InstanceGroupNotFound
to HTTPBadRequest.

Closes-bug: #1535759
Change-Id: I38d98c74f6cceed5b4becf9ed67f7189cba479fa

nova/api/openstack/compute/legacy_v2/servers.py

@@ -84,6 +84,7 @@ CREATE_EXCEPTIONS = {
     exception.InstanceExists: exc.HTTPConflict,
     exception.NoUniqueMatch: exc.HTTPConflict,
     exception.Invalid: exc.HTTPBadRequest,
+    exception.InstanceGroupNotFound: exc.HTTPBadRequest,
 }
 
 CREATE_EXCEPTIONS_MSGS = {

nova/api/openstack/compute/schemas/scheduler_hints.py

@@ -19,9 +19,8 @@ _hints = {
     'type': 'object',
     'properties': {
         'group': {
-            # NOTE: The value of 'group' is stored to value which is
-            # defined as varchar(255) in instance_system_metadata table.
-            'type': 'string', 'maxLength': 255,
+            'type': 'string',
+            'format': 'uuid'
         },
         'different_host': {
             # NOTE: The value of 'different_host' is the set of server

nova/api/openstack/compute/servers.py

@@ -688,7 +688,8 @@ class ServersController(wsgi.Controller):
                 exception.ImageNUMATopologyCPUOutOfRange,
                 exception.ImageNUMATopologyCPUDuplicates,
                 exception.ImageNUMATopologyCPUsUnassigned,
-                exception.ImageNUMATopologyMemoryOutOfRange) as error:
+                exception.ImageNUMATopologyMemoryOutOfRange,
+                exception.InstanceGroupNotFound) as error:
             raise exc.HTTPBadRequest(explanation=error.format_message())
         except (exception.PortInUse,
                 exception.InstanceExists,

nova/compute/api.py

@@ -1031,6 +1031,8 @@ class API(base.Base):
         if not group_hint:
             return
 
+        # TODO(gibi): We need to remove the following validation code when
+        # removing legacy v2 code.
         if not uuidutils.is_uuid_like(group_hint):
             msg = _('Server group scheduler hint must be a UUID.')
             raise exception.InvalidInput(reason=msg)

nova/tests/unit/api/openstack/compute/test_scheduler_hints.py

@@ -15,6 +15,7 @@
 
 import datetime
 
+import mock
 from oslo_config import cfg
 from oslo_serialization import jsonutils
 
@@ -25,6 +26,7 @@ from nova.api.openstack.compute import servers as servers_v21
 from nova.api.openstack import extensions
 import nova.compute.api
 from nova.compute import flavors
+from nova import exception
 from nova import test
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit import fake_instance
@@ -97,7 +99,11 @@ class SchedulerHintsTestCaseV21(test.TestCase):
         self.assertEqual(202, res.status_int)
 
     def test_create_server_with_group_hint(self):
-        self._test_create_server_with_hint({'group': 'foo'})
+        self._test_create_server_with_hint({'group': UUID})
+
+    def test_create_server_with_non_uuid_group_hint(self):
+        self._create_server_with_scheduler_hints_bad_request(
+                {'group': 'non-uuid'})
 
     def test_create_server_with_different_host_hint(self):
         self._test_create_server_with_hint(
@@ -160,6 +166,13 @@ class SchedulerHintsTestCaseV2(SchedulerHintsTestCaseV21):
         # We skip this test for v2.0.
         pass
 
+    @mock.patch(
+            'nova.api.openstack.compute.legacy_v2.servers.Controller.create')
+    def test_create_server_with_non_uuid_group_hint(self, mock_create):
+        mock_create.side_effect = exception.InvalidInput(reason='')
+        self._create_server_with_scheduler_hints_bad_request(
+                {'group': 'non-uuid'})
+
 
 class ServersControllerCreateTestV21(test.TestCase):
 

nova/tests/unit/api/openstack/compute/test_serversV21.py

@@ -3116,6 +3116,24 @@ class ServersControllerCreateTest(test.TestCase):
         test_group = objects.InstanceGroup.get_by_uuid(ctxt, test_group.uuid)
         self.assertIn(server['id'], test_group.members)
 
+    def test_create_instance_with_group_hint_group_not_found(self):
+        def fake_instance_destroy(context, uuid, constraint):
+            return fakes.stub_instance(1)
+
+        self.stub_out('nova.db.instance_destroy', fake_instance_destroy)
+        self.body['os:scheduler_hints'] = {
+            'group': '5b674f73-c8cf-40ef-9965-3b6fe4b304b1'}
+        self.req.body = jsonutils.dump_as_bytes(self.body)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create, self.req, body=self.body)
+
+    def test_create_instance_with_group_hint_wrong_uuid_format(self):
+        self.body['os:scheduler_hints'] = {
+            'group': 'non-uuid'}
+        self.req.body = jsonutils.dump_as_bytes(self.body)
+        self.assertRaises(exception.ValidationError,
+                          self.controller.create, self.req, body=self.body)
+
     def test_create_instance_with_neutronv2_port_in_use(self):
         network = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
         port = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'

nova/tests/unit/compute/test_compute.py

@@ -7831,9 +7831,27 @@ class ComputeAPITestCase(BaseTestCase):
         self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show)
 
         inst_type = flavors.get_default_flavor()
-        self.assertRaises(exception.InvalidInput, self.compute_api.create,
-            self.context, inst_type, self.fake_image['id'],
-            scheduler_hints={'group': 'groupname'})
+        self.assertRaises(
+                exception.InvalidInput,
+                self.compute_api.create,
+                self.context,
+                inst_type,
+                self.fake_image['id'],
+                scheduler_hints={'group': 'non-uuid'})
+
+    def test_instance_create_with_group_uuid_fails_group_not_exist(self):
+        self.stub_out('nova.tests.unit.image.fake._FakeImageService.show',
+                      self.fake_show)
+
+        inst_type = flavors.get_default_flavor()
+        self.assertRaises(
+                exception.InstanceGroupNotFound,
+                self.compute_api.create,
+                self.context,
+                inst_type,
+                self.fake_image['id'],
+                scheduler_hints={'group':
+                                     '5b674f73-c8cf-40ef-9965-3b6fe4b304b1'})
 
     def test_destroy_instance_disassociates_security_groups(self):
         # Make sure destroying disassociates security groups.