Merge "don't 500 on invalid security group format"

nova/api/openstack/compute/servers.py

@@ -517,8 +517,17 @@ class Controller(wsgi.Controller):
         if self.ext_mgr.is_loaded('os-security-groups'):
             security_groups = server_dict.get('security_groups')
             if security_groups is not None:
-                sg_names = [sg['name'] for sg in security_groups
-                            if sg.get('name')]
+                try:
+                    sg_names = [sg['name'] for sg in security_groups
+                                if sg.get('name')]
+                except AttributeError:
+                    msg = _("Invalid input for field/attribute %(path)s."
+                           " Value: %(value)s. %(message)s") % {
+                               'path': 'security_groups',
+                               'value': security_groups,
+                               'message': ''
+                           }
+                    raise exc.HTTPBadRequest(explanation=msg)
         if not sg_names:
             sg_names.append('default')
 

nova/tests/functional/wsgi/test_secgroup.py

@@ -0,0 +1,97 @@
+# Copyright 2015 Hewlett-Packard Development Company, L.P.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from oslo_log import log as logging
+import testscenarios
+
+from nova import test
+from nova.tests import fixtures as nova_fixtures
+from nova.tests.functional.v3 import api_paste_fixture
+import nova.tests.unit.image.fake
+from nova.tests.unit import policy_fixture
+
+LOG = logging.getLogger(__name__)
+
+
+class SecgroupsFullstack(testscenarios.WithScenarios, test.TestCase):
+    """Tests for security groups
+
+    TODO: describe security group API
+
+    TODO: define scope
+
+    """
+    REQUIRES_LOCKING = True
+    _image_ref_parameter = 'imageRef'
+    _flavor_ref_parameter = 'flavorRef'
+
+    # This test uses ``testscenarios`` which matrix multiplies the
+    # test across the scenarios listed below setting the attributres
+    # in the dictionary on ``self`` for each scenario.
+    scenarios = [
+        ('v2', {'_test': 'v2'}),  # regular base scenario, test the v2 api
+        ('v2_1', {'_test': 'v2.1'})  # test v2.1 api on the v2 api endpoint
+    ]
+
+    def setUp(self):
+        super(SecgroupsFullstack, self).setUp()
+        self.useFixture(policy_fixture.RealPolicyFixture())
+        if self._test == 'v2.1':
+            self.useFixture(api_paste_fixture.ApiPasteFixture())
+        api_fixture = self.useFixture(nova_fixtures.OSAPIFixture())
+
+        self.api = api_fixture.api
+
+        # the image fake backend needed for image discovery
+        nova.tests.unit.image.fake.stub_out_image_service(self.stubs)
+
+    # TODO(sdague): refactor this method into the API client, we're
+    # going to use it a lot
+    def _build_minimal_create_server_request(self, name):
+        server = {}
+
+        image = self.api.get_images()[0]
+        LOG.info("Image: %s" % image)
+
+        if self._image_ref_parameter in image:
+            image_href = image[self._image_ref_parameter]
+        else:
+            image_href = image['id']
+            image_href = 'http://fake.server/%s' % image_href
+
+        # We now have a valid imageId
+        server[self._image_ref_parameter] = image_href
+
+        # Set a valid flavorId
+        flavor = self.api.get_flavors()[1]
+        server[self._flavor_ref_parameter] = ('http://fake.server/%s'
+                                              % flavor['id'])
+        server['name'] = name
+        return server
+
+    def test_security_group_fuzz(self):
+        """Test security group doesn't explode with a 500 on bad input.
+
+        Originally reported with bug
+        https://bugs.launchpad.net/nova/+bug/1239723
+
+        """
+        server = self._build_minimal_create_server_request("sg-fuzz")
+        # security groups must be passed as a list, this is an invalid
+        # format. The jsonschema in v2.1 caught it automatically, but
+        # in v2 we used to throw a 500.
+        server['security_groups'] = {"name": "sec"}
+        resp = self.api.api_post('/servers', {'server': server},
+                                 check_response_status=False)
+        self.assertEqual(400, resp.status)