Merge "Moves security group functionality into extension"

9 files changed, 271 insertions, 56 deletions

diff --git a/nova/api/openstack/compute/contrib/createserverext.py b/nova/api/openstack/compute/contrib/createserverext.py
index ebb24e559..116511fe5 100644
--- a/nova/api/openstack/compute/contrib/createserverext.py
+++ b/nova/api/openstack/compute/contrib/createserverext.py
@@ -14,36 +14,9 @@
 #    License for the specific language governing permissions and limitations
 #    under the License
 
-from nova.api.openstack.compute import servers
-from nova.api.openstack.compute import views
 from nova.api.openstack import extensions
 
 
-authorize = extensions.soft_extension_authorizer('compute', 'createserverext')
-
-
-class ViewBuilder(views.servers.ViewBuilder):
-    """Adds security group output when viewing server details."""
-
-    def show(self, request, instance):
-        """Detailed view of a single instance."""
-        server = super(ViewBuilder, self).show(request, instance)
-        context = request.environ['nova.context']
-        if authorize(context):
-            server["server"]["security_groups"] = self._get_groups(instance)
-        return server
-
-    def _get_groups(self, instance):
-        """Get a list of security groups for this instance."""
-        groups = instance.get('security_groups')
-        if groups is not None:
-            return [{"name": group["name"]} for group in groups]
-
-
-class Controller(servers.Controller):
-    _view_builder_class = ViewBuilder
-
-
 class Createserverext(extensions.ExtensionDescriptor):
     """Extended support to the Create Server v1.1 API"""
 
@@ -54,11 +27,6 @@ class Createserverext(extensions.ExtensionDescriptor):
     updated = "2011-07-19T00:00:00+00:00"
 
     def get_resources(self):
-        resources = []
-        controller = Controller(self.ext_mgr)
-
         res = extensions.ResourceExtension('os-create-server-ext',
-                                           controller=controller)
-        resources.append(res)
-
-        return resources
+                                           inherits='servers')
+        return [res]
diff --git a/nova/api/openstack/compute/contrib/disk_config.py b/nova/api/openstack/compute/contrib/disk_config.py
index d803035af..961457c46 100644
--- a/nova/api/openstack/compute/contrib/disk_config.py
+++ b/nova/api/openstack/compute/contrib/disk_config.py
@@ -106,7 +106,7 @@ class ServerDiskConfigController(wsgi.Controller):
             db_server = req.get_db_instance(server['id'])
             # server['id'] is guaranteed to be in the cache due to
             # the core API adding it in its 'show'/'detail' methods.
-            value = db_server[INTERNAL_DISK_CONFIG]
+            value = db_server.get(INTERNAL_DISK_CONFIG)
             server[API_DISK_CONFIG] = disk_config_to_api(value)
 
     def _show(self, req, resp_obj):
diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index 6d85d37a6..8bb9f3cf1 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -35,6 +35,7 @@ from nova.openstack.common import log as logging
 LOG = logging.getLogger(__name__)
 FLAGS = flags.FLAGS
 authorize = extensions.extension_authorizer('compute', 'security_groups')
+softauth = extensions.soft_extension_authorizer('compute', 'security_groups')
 
 
 def make_rule(elem):
@@ -454,6 +455,70 @@ class SecurityGroupActionController(wsgi.Controller):
                             context, id, group_name)
 
 
+class SecurityGroupsOutputController(wsgi.Controller):
+    def __init__(self, *args, **kwargs):
+        super(SecurityGroupsOutputController, self).__init__(*args, **kwargs)
+        self.compute_api = compute.API()
+
+    def _extend_servers(self, req, servers):
+        key = "security_groups"
+        for server in servers:
+            instance = req.get_db_instance(server['id'])
+            groups = instance.get(key)
+            if groups:
+                server[key] = [{"name": group["name"]} for group in groups]
+
+    def _show(self, req, resp_obj):
+        if not softauth(req.environ['nova.context']):
+            return
+        if 'server' in resp_obj.obj:
+            resp_obj.attach(xml=SecurityGroupServerTemplate())
+            self._extend_servers(req, [resp_obj.obj['server']])
+
+    @wsgi.extends
+    def show(self, req, resp_obj, id):
+        return self._show(req, resp_obj)
+
+    @wsgi.extends
+    def create(self, req, resp_obj, body):
+        return self._show(req, resp_obj)
+
+    @wsgi.extends
+    def detail(self, req, resp_obj):
+        if not softauth(req.environ['nova.context']):
+            return
+        resp_obj.attach(xml=SecurityGroupServersTemplate())
+        self._extend_servers(req, list(resp_obj.obj['servers']))
+
+
+class SecurityGroupsTemplateElement(xmlutil.TemplateElement):
+    def will_render(self, datum):
+        return "security_groups" in datum
+
+
+def make_server(elem):
+    secgrps = SecurityGroupsTemplateElement('security_groups')
+    elem.append(secgrps)
+    secgrp = xmlutil.SubTemplateElement(secgrps, 'security_group',
+                                        selector="security_groups")
+    secgrp.set('name')
+
+
+class SecurityGroupServerTemplate(xmlutil.TemplateBuilder):
+    def construct(self):
+        root = xmlutil.TemplateElement('server')
+        make_server(root)
+        return xmlutil.SlaveTemplate(root, 1)
+
+
+class SecurityGroupServersTemplate(xmlutil.TemplateBuilder):
+    def construct(self):
+        root = xmlutil.TemplateElement('servers')
+        elem = xmlutil.SubTemplateElement(root, 'server', selector='servers')
+        make_server(elem)
+        return xmlutil.SlaveTemplate(root, 1)
+
+
 class Security_groups(extensions.ExtensionDescriptor):
     """Security group support"""
     name = "SecurityGroups"
@@ -463,8 +528,10 @@ class Security_groups(extensions.ExtensionDescriptor):
 
     def get_controller_extensions(self):
         controller = SecurityGroupActionController()
-        extension = extensions.ControllerExtension(self, 'servers', controller)
-        return [extension]
+        actions = extensions.ControllerExtension(self, 'servers', controller)
+        controller = SecurityGroupsOutputController()
+        output = extensions.ControllerExtension(self, 'servers', controller)
+        return [actions, output]
 
     def get_resources(self):
         resources = []
diff --git a/nova/api/openstack/compute/contrib/volumes.py b/nova/api/openstack/compute/contrib/volumes.py
index 1f231d600..8dc16fcdc 100644
--- a/nova/api/openstack/compute/contrib/volumes.py
+++ b/nova/api/openstack/compute/contrib/volumes.py
@@ -598,7 +598,8 @@ class Volumes(extensions.ExtensionDescriptor):
         resources.append(res)
 
         controller = BootFromVolumeController(self.ext_mgr)
-        res = extensions.ResourceExtension('os-volumes_boot', controller)
+        res = extensions.ResourceExtension('os-volumes_boot', controller,
+                                           inherits='servers')
         resources.append(res)
 
         res = extensions.ResourceExtension('os-snapshots',
diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py
index e71f2bcea..d70323cbf 100644
--- a/nova/api/openstack/compute/servers.py
+++ b/nova/api/openstack/compute/servers.py
@@ -41,11 +41,6 @@ LOG = logging.getLogger(__name__)
 FLAGS = flags.FLAGS
 
 
-class SecurityGroupsTemplateElement(xmlutil.TemplateElement):
-    def will_render(self, datum):
-        return 'security_groups' in datum
-
-
 def make_fault(elem):
     fault = xmlutil.SubTemplateElement(elem, 'fault', selector='fault')
     fault.set('code')
@@ -90,13 +85,6 @@ def make_server(elem, detailed=False):
         # Attach addresses node
         elem.append(ips.AddressesTemplate())
 
-        # Attach security groups node
-        secgrps = SecurityGroupsTemplateElement('security_groups')
-        elem.append(secgrps)
-        secgrp = xmlutil.SubTemplateElement(secgrps, 'security_group',
-                                            selector='security_groups')
-        secgrp.set('name')
-
     xmlutil.make_links(elem, 'links')
 
 
@@ -630,9 +618,11 @@ class Controller(wsgi.Controller):
             injected_files = self._get_injected_files(personality)
 
         sg_names = []
-        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')]
+        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')]
         if not sg_names:
             sg_names.append('default')
 
diff --git a/nova/tests/api/openstack/compute/contrib/test_createserverext.py b/nova/tests/api/openstack/compute/contrib/test_createserverext.py
index aaef09f8b..d7d5be5c7 100644
--- a/nova/tests/api/openstack/compute/contrib/test_createserverext.py
+++ b/nova/tests/api/openstack/compute/contrib/test_createserverext.py
@@ -77,6 +77,10 @@ class CreateserverextTest(test.TestCase):
                 self.db = db
 
             def create(self, *args, **kwargs):
+                if 'security_group' in kwargs:
+                    self.security_group = kwargs['security_group']
+                else:
+                    self.security_group = None
                 if 'injected_files' in kwargs:
                     self.injected_files = kwargs['injected_files']
                 else:
@@ -367,6 +371,7 @@ class CreateserverextTest(test.TestCase):
         _run_create_inst = self._run_create_instance_with_mock_compute_api
         compute_api, response = _run_create_inst(request)
         self.assertEquals(response.status_int, 202)
+        self.assertEquals(compute_api.security_group, security_groups)
 
     def test_get_server_by_id_verify_security_groups_json(self):
         self.stubs.Set(nova.db, 'instance_get', fakes.fake_instance_get())
@@ -376,7 +381,7 @@ class CreateserverextTest(test.TestCase):
         self.assertEquals(response.status_int, 200)
         res_dict = jsonutils.loads(response.body)
         expected_security_group = [{"name": "test"}]
-        self.assertEquals(res_dict['server']['security_groups'],
+        self.assertEquals(res_dict['server'].get('security_groups'),
                           expected_security_group)
 
     def test_get_server_by_id_verify_security_groups_xml(self):
diff --git a/nova/tests/api/openstack/compute/contrib/test_disk_config.py b/nova/tests/api/openstack/compute/contrib/test_disk_config.py
index 9999d94da..a6c898f7c 100644
--- a/nova/tests/api/openstack/compute/contrib/test_disk_config.py
+++ b/nova/tests/api/openstack/compute/contrib/test_disk_config.py
@@ -95,6 +95,10 @@ class DiskConfigTestCase(test.TestCase):
             inst['name'] = 'instance-1'  # this is a property
             inst['task_state'] = ''
             inst['vm_state'] = ''
+            # NOTE(vish): db create translates security groups into model
+            #             objects. Translate here so tests pass
+            inst['security_groups'] = [{'name': group}
+                                       for group in inst['security_groups']]
 
             def fake_instance_get_for_create(context, id_, *args, **kwargs):
                 return (inst, inst)
diff --git a/nova/tests/api/openstack/compute/contrib/test_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
index ea7d21db1..f977fc04e 100644
--- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py
+++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
@@ -23,9 +23,12 @@ import webob
 
 from nova.api.openstack.compute.contrib import security_groups
 from nova.api.openstack import wsgi
+from nova.api.openstack import xmlutil
+from nova import compute
 import nova.db
 from nova import exception
 from nova import flags
+from nova.openstack.common import jsonutils
 from nova import test
 from nova.tests.api.openstack import fakes
 
@@ -1199,3 +1202,136 @@ class TestSecurityGroupXMLSerializer(unittest.TestCase):
         self.assertEqual(len(groups), len(tree))
         for idx, child in enumerate(tree):
             self._verify_security_group(groups[idx], child)
+
+
+UUID1 = '00000000-0000-0000-0000-000000000001'
+UUID2 = '00000000-0000-0000-0000-000000000002'
+UUID3 = '00000000-0000-0000-0000-000000000003'
+
+
+def fake_compute_get_all(*args, **kwargs):
+    return [
+        fakes.stub_instance(1, uuid=UUID1,
+                            security_groups=[{'name': 'fake-0-0'},
+                                             {'name': 'fake-0-1'}]),
+        fakes.stub_instance(2, uuid=UUID2,
+                            security_groups=[{'name': 'fake-1-0'},
+                                             {'name': 'fake-1-1'}])
+    ]
+
+
+def fake_compute_get(*args, **kwargs):
+    return fakes.stub_instance(1, uuid=UUID3,
+                               security_groups=[{'name': 'fake-2-0'},
+                                                {'name': 'fake-2-1'}])
+
+
+def fake_compute_create(*args, **kwargs):
+    return ([fake_compute_get()], '')
+
+
+class SecurityGroupsOutputTest(test.TestCase):
+    content_type = 'application/json'
+
+    def setUp(self):
+        super(SecurityGroupsOutputTest, self).setUp()
+        fakes.stub_out_nw_api(self.stubs)
+        self.stubs.Set(compute.api.API, 'get', fake_compute_get)
+        self.stubs.Set(compute.api.API, 'get_all', fake_compute_get_all)
+        self.stubs.Set(compute.api.API, 'create', fake_compute_create)
+
+    def _make_request(self, url, body=None):
+        req = webob.Request.blank(url)
+        if body:
+            req.method = 'POST'
+            req.body = self._encode_body(body)
+        req.content_type = self.content_type
+        req.headers['Accept'] = self.content_type
+        res = req.get_response(fakes.wsgi_app())
+        return res
+
+    def _encode_body(self, body):
+        return jsonutils.dumps(body)
+
+    def _get_server(self, body):
+        return jsonutils.loads(body).get('server')
+
+    def _get_servers(self, body):
+        return jsonutils.loads(body).get('servers')
+
+    def _get_groups(self, server):
+        return server.get('security_groups')
+
+    def test_create(self):
+        url = '/v2/fake/servers'
+        image_uuid = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
+        server = dict(name='server_test', imageRef=image_uuid, flavorRef=2)
+        res = self._make_request(url, {'server': server})
+        self.assertEqual(res.status_int, 202)
+        server = self._get_server(res.body)
+        for i, group in enumerate(self._get_groups(server)):
+            name = 'fake-2-%s' % i
+            self.assertEqual(group.get('name'), name)
+
+    def test_show(self):
+        url = '/v2/fake/servers/%s' % UUID3
+        res = self._make_request(url)
+
+        self.assertEqual(res.status_int, 200)
+        server = self._get_server(res.body)
+        for i, group in enumerate(self._get_groups(server)):
+            name = 'fake-2-%s' % i
+            self.assertEqual(group.get('name'), name)
+
+    def test_detail(self):
+        url = '/v2/fake/servers/detail'
+        res = self._make_request(url)
+
+        self.assertEqual(res.status_int, 200)
+        for i, server in enumerate(self._get_servers(res.body)):
+            for j, group in enumerate(self._get_groups(server)):
+                name = 'fake-%s-%s' % (i, j)
+                self.assertEqual(group.get('name'), name)
+
+    def test_no_instance_passthrough_404(self):
+
+        def fake_compute_get(*args, **kwargs):
+            raise exception.InstanceNotFound()
+
+        self.stubs.Set(compute.api.API, 'get', fake_compute_get)
+        url = '/v2/fake/servers/70f6db34-de8d-4fbd-aafb-4065bdfa6115'
+        res = self._make_request(url)
+
+        self.assertEqual(res.status_int, 404)
+
+
+class SecurityGroupsOutputXmlTest(SecurityGroupsOutputTest):
+    content_type = 'application/xml'
+
+    class MinimalCreateServerTemplate(xmlutil.TemplateBuilder):
+        def construct(self):
+            root = xmlutil.TemplateElement('server', selector='server')
+            root.set('name')
+            root.set('id')
+            root.set('imageRef')
+            root.set('flavorRef')
+            return xmlutil.MasterTemplate(root, 1,
+                                          nsmap={None: xmlutil.XMLNS_V11})
+
+    def _encode_body(self, body):
+        serializer = self.MinimalCreateServerTemplate()
+        return serializer.serialize(body)
+
+    def _get_server(self, body):
+        return etree.XML(body)
+
+    def _get_servers(self, body):
+        return etree.XML(body).getchildren()
+
+    def _get_groups(self, server):
+        # NOTE(vish): we are adding security groups without an extension
+        #             namespace so we don't break people using the existing
+        #             functionality, but that means we need to use find with
+        #             the existing server namespace.
+        namespace = server.nsmap[None]
+        return server.find('{%s}security_groups' % namespace).getchildren()
diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py
index 2cb146358..e482ca812 100644
--- a/nova/tests/api/openstack/compute/test_servers.py
+++ b/nova/tests/api/openstack/compute/test_servers.py
@@ -116,6 +116,7 @@ class ServersControllerTest(test.TestCase):
                 instance_update)
 
         self.ext_mgr = extensions.ExtensionManager()
+        self.ext_mgr.extensions = {}
         self.controller = servers.Controller(self.ext_mgr)
         self.ips_controller = ips.Controller()
 
@@ -1410,7 +1411,9 @@ class ServerStatusTest(test.TestCase):
         super(ServerStatusTest, self).setUp()
         fakes.stub_out_nw_api(self.stubs)
 
-        self.controller = servers.Controller()
+        self.ext_mgr = extensions.ExtensionManager()
+        self.ext_mgr.extensions = {}
+        self.controller = servers.Controller(self.ext_mgr)
 
     def _get_with_state(self, vm_state, task_state=None):
         self.stubs.Set(nova.db, 'instance_get_by_uuid',
@@ -1479,7 +1482,9 @@ class ServersControllerCreateTest(test.TestCase):
         self.instance_cache_by_id = {}
         self.instance_cache_by_uuid = {}
 
-        self.controller = servers.Controller()
+        self.ext_mgr = extensions.ExtensionManager()
+        self.ext_mgr.extensions = {}
+        self.controller = servers.Controller(self.ext_mgr)
 
         def instance_create(context, inst):
             inst_type = instance_types.get_instance_type_by_flavor_id(3)
@@ -1743,6 +1748,45 @@ class ServersControllerCreateTest(test.TestCase):
         fakes.stub_out_key_pair_funcs(self.stubs, have_key_pair=False)
         self._test_create_instance()
 
+    def _test_create_security_group(self, group):
+        image_uuid = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77'
+        body = dict(server=dict(
+            name='server_test', imageRef=image_uuid, flavorRef=2,
+            metadata={'hello': 'world', 'open': 'stack'},
+            security_groups=[{'name': group}],
+            personality={}))
+        req = fakes.HTTPRequest.blank('/v2/fake/servers')
+        req.method = 'POST'
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+        server = self.controller.create(req, body).obj['server']
+
+    def test_create_instance_with_security_group_enabled(self):
+        self.ext_mgr.extensions = {'os-security-groups': 'fake'}
+        group = 'foo'
+        old_create = nova.compute.api.API.create
+
+        def create(*args, **kwargs):
+            self.assertEqual(kwargs['security_group'], [group])
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(nova.compute.api.API, 'create', create)
+        self._test_create_security_group(group)
+
+    def test_create_instance_with_security_group_disabled(self):
+        group = 'foo'
+        old_create = nova.compute.api.API.create
+
+        def create(*args, **kwargs):
+            # NOTE(vish): if the security groups extension is not
+            #             enabled, then security groups passed in
+            #             are ignored.
+            self.assertEqual(kwargs['security_group'], ['default'])
+            return old_create(*args, **kwargs)
+
+        self.stubs.Set(nova.compute.api.API, 'create', create)
+        self._test_create_security_group(group)
+
     def test_create_instance_with_access_ip(self):
         # proper local hrefs must start with 'http://localhost/v2/'
         image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6'