Merge security groups extension response into server view builder

As nova extensions has been deprecated already and goal is to
merge all scattered code into main controller side.
Currently schema and request/response extended code are there
among all extensions.

This commit merge the security groups extension resposne into server
view builder.

Partially implements: blueprint api-extensions-merge-stein

Change-Id: I57141fc6b1ee87ad3933edd9dc255294d03b5651
This commit is contained in:
ghanshyam 2018-08-15 14:19:43 +00:00 committed by Matt Riedemann
parent 6a74828621
commit 2e5fd8c8b8
7 changed files with 211 additions and 94 deletions

View File

@ -269,7 +269,6 @@ server_controller = functools.partial(_create_controller,
extended_status.ExtendedStatusController,
extended_volumes.ExtendedVolumesController,
hide_server_addresses.Controller,
security_groups.SecurityGroupsOutputController,
],
[
admin_actions.AdminActionsController,

View File

@ -16,7 +16,6 @@
"""The security groups extension."""
from oslo_log import log as logging
from oslo_serialization import jsonutils
from webob import exc
from nova.api.openstack.api_version_request \
@ -35,7 +34,6 @@ from nova.virt import netutils
LOG = logging.getLogger(__name__)
ATTRIBUTE_NAME = 'security_groups'
SG_NOT_FOUND = object()
@ -474,66 +472,3 @@ class SecurityGroupActionController(wsgi.Controller):
raise exc.HTTPConflict(explanation=exp.format_message())
except exception.SecurityGroupNotExistsForInstance as exp:
raise exc.HTTPBadRequest(explanation=exp.format_message())
class SecurityGroupsOutputController(wsgi.Controller):
def __init__(self, *args, **kwargs):
super(SecurityGroupsOutputController, self).__init__(*args, **kwargs)
self.compute_api = compute.API()
self.security_group_api = (
openstack_driver.get_openstack_security_group_driver())
def _extend_servers(self, req, servers):
# TODO(arosen) this function should be refactored to reduce duplicate
# code and use get_instance_security_groups instead of get_db_instance.
if not len(servers):
return
key = "security_groups"
context = req.environ['nova.context']
if not openstack_driver.is_neutron_security_groups():
for server in servers:
instance = req.get_db_instance(server['id'])
groups = instance.get(key)
if groups:
server[ATTRIBUTE_NAME] = [{"name": group.name}
for group in groups]
else:
# If method is a POST we get the security groups intended for an
# instance from the request. The reason for this is if using
# neutron security groups the requested security groups for the
# instance are not in the db and have not been sent to neutron yet.
if req.method != 'POST':
sg_instance_bindings = (
self.security_group_api
.get_instances_security_groups_bindings(context,
servers))
for server in servers:
groups = sg_instance_bindings.get(server['id'])
if groups:
server[ATTRIBUTE_NAME] = groups
# In this section of code len(servers) == 1 as you can only POST
# one server in an API request.
else:
# try converting to json
req_obj = jsonutils.loads(req.body)
# Add security group to server, if no security group was in
# request add default since that is the group it is part of
servers[0][ATTRIBUTE_NAME] = req_obj['server'].get(
ATTRIBUTE_NAME, [{'name': 'default'}])
def _show(self, req, resp_obj):
if 'server' in resp_obj.obj:
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):
self._extend_servers(req, list(resp_obj.obj['servers']))

View File

@ -723,7 +723,8 @@ class ServersController(wsgi.Controller):
show_extended_attr=False,
show_host_status=False,
show_keypair=False,
show_srv_usg=False)
show_srv_usg=False,
show_sec_grp=False)
except exception.InstanceNotFound:
msg = _("Instance could not be found")
raise exc.HTTPNotFound(explanation=msg)
@ -997,7 +998,8 @@ class ServersController(wsgi.Controller):
show_extended_attr=False,
show_host_status=False,
show_keypair=show_keypair,
show_srv_usg=False)
show_srv_usg=False,
show_sec_grp=False)
# Add on the admin_password attribute since the view doesn't do it
# unless instance passwords are disabled

View File

@ -15,6 +15,7 @@
# under the License.
from oslo_log import log as logging
from oslo_serialization import jsonutils
from nova.api.openstack import api_version_request
from nova.api.openstack import common
@ -25,6 +26,7 @@ from nova import availability_zones as avail_zone
from nova import compute
from nova import context as nova_context
from nova import exception
from nova.network.security_group import openstack_driver
from nova import objects
from nova.policies import extended_server_attributes as esa_policies
from nova.policies import flavor_extra_specs as fes_policies
@ -65,10 +67,13 @@ class ViewBuilder(common.ViewBuilder):
self._image_builder = views_images.ViewBuilder()
self._flavor_builder = views_flavors.ViewBuilder()
self.compute_api = compute.API()
self.security_group_api = (
openstack_driver.get_openstack_security_group_driver())
def create(self, request, instance):
"""View that should be returned when an instance is created."""
return {
server = {
"server": {
"id": instance["uuid"],
"links": self._get_links(request,
@ -81,9 +86,13 @@ class ViewBuilder(common.ViewBuilder):
'AUTO' if instance.get('auto_disk_config') else 'MANUAL'),
},
}
self._add_security_grps(request, [server["server"]], [instance])
return server
def basic(self, request, instance, show_extra_specs=False,
show_extended_attr=None, show_host_status=None):
show_extended_attr=None, show_host_status=None,
show_sec_grp=None):
"""Generic, non-detailed view of an instance."""
return {
"server": {
@ -118,7 +127,7 @@ class ViewBuilder(common.ViewBuilder):
def show(self, request, instance, extend_address=True,
show_extra_specs=None, show_AZ=True, show_config_drive=True,
show_extended_attr=None, show_host_status=None,
show_keypair=True, show_srv_usg=True):
show_keypair=True, show_srv_usg=True, show_sec_grp=True):
"""Detailed view of a single instance."""
ip_v4 = instance.get('access_ip_v4')
ip_v6 = instance.get('access_ip_v6')
@ -191,6 +200,8 @@ class ViewBuilder(common.ViewBuilder):
# the tzinfo from the stamp and str() it.
server["server"][key] = (instance[k].replace(tzinfo=None)
if instance[k] else None)
if show_sec_grp:
self._add_security_grps(request, [server["server"]], [instance])
if show_extended_attr is None:
show_extended_attr = context.can(
@ -270,13 +281,23 @@ class ViewBuilder(common.ViewBuilder):
if (api_version_request.is_supported(request, min_version='2.16')):
show_host_status = context.can(
servers_policies.SERVERS % 'show:host_status', fatal=False)
return self._list_view(self.show, request, instances, coll_name,
show_extra_specs,
show_extended_attr=show_extended_attr,
show_host_status=show_host_status)
# NOTE(gmann): pass show_sec_grp=False in _list_view() because
# security groups for detail method will be added by separate
# call to self._add_security_grps by passing the all servers
# together. That help to avoid multiple neutron call for each server.
servers_dict = self._list_view(self.show, request, instances,
coll_name, show_extra_specs,
show_extended_attr=show_extended_attr,
show_host_status=show_host_status,
show_sec_grp=False)
self._add_security_grps(request, list(servers_dict["servers"]),
instances)
return servers_dict
def _list_view(self, func, request, servers, coll_name, show_extra_specs,
show_extended_attr=None, show_host_status=None):
show_extended_attr=None, show_host_status=None,
show_sec_grp=False):
"""Provide a view for a list of servers.
:param func: Function used to format the server data
@ -288,12 +309,15 @@ class ViewBuilder(common.ViewBuilder):
included in the response dict.
:param show_host_status: If the host status should be included in
the response dict.
:param show_sec_grp: If the security group should be included in
the response dict.
:returns: Server data in dictionary format
"""
server_list = [func(request, server,
show_extra_specs=show_extra_specs,
show_extended_attr=show_extended_attr,
show_host_status=show_host_status)["server"]
show_host_status=show_host_status,
show_sec_grp=show_sec_grp)["server"]
for server in servers]
servers_links = self._get_collection_links(request,
servers,
@ -422,3 +446,42 @@ class ViewBuilder(common.ViewBuilder):
fault_dict['details'] = fault["details"]
return fault_dict
def _add_security_grps(self, req, servers, instances):
# TODO(arosen) this function should be refactored to reduce duplicate
# code and use get_instance_security_groups instead of get_db_instance.
if not len(servers):
return
if not openstack_driver.is_neutron_security_groups():
instances = {inst['uuid']: inst for inst in instances}
for server in servers:
instance = instances[server['id']]
groups = instance.get('security_groups')
if groups:
server['security_groups'] = [{"name": group.name}
for group in groups]
else:
# If method is a POST we get the security groups intended for an
# instance from the request. The reason for this is if using
# neutron security groups the requested security groups for the
# instance are not in the db and have not been sent to neutron yet.
if req.method != 'POST':
context = req.environ['nova.context']
sg_instance_bindings = (
self.security_group_api
.get_instances_security_groups_bindings(context,
servers))
for server in servers:
groups = sg_instance_bindings.get(server['id'])
if groups:
server['security_groups'] = groups
# This section is for POST request. There can be only one security
# group for POST request.
else:
# try converting to json
req_obj = jsonutils.loads(req.body)
# Add security group to server, if no security group was in
# request add default since that is the group it is part of
servers[0]['security_groups'] = req_obj['server'].get(
'security_groups', [{'name': 'default'}])

View File

@ -33,7 +33,8 @@ class AccessIPsAPIValidationTestV21(test.TestCase):
def fake_rebuild(*args, **kwargs):
pass
# Neutron security groups are tested in test_neutron_security_groups.py
self.flags(use_neutron=False)
fakes.stub_out_nw_api(self)
self._set_up_controller()
fake.stub_out_image_service(self)

View File

@ -168,6 +168,8 @@ class ServersControllerCreateTestV21(test.TestCase):
super(ServersControllerCreateTestV21, self).setUp()
self.instance_cache_num = 0
# Neutron security groups are tested in test_neutron_security_groups.py
self.flags(use_neutron=False)
fakes.stub_out_nw_api(self)
self._set_up_controller()

View File

@ -162,11 +162,22 @@ class ControllerTest(test.TestCase):
def setUp(self):
super(ControllerTest, self).setUp()
self.flags(use_ipv6=False)
# Neutron security groups are tested in test_neutron_security_groups.py
self.flags(use_neutron=False)
fakes.stub_out_nw_api(self)
fakes.stub_out_key_pair_funcs(self)
fake.stub_out_image_service(self)
security_groups = [
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}]
return_server = fakes.fake_compute_get(id=2, availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=security_groups)
return_servers = fakes.fake_compute_get_all()
# Server sort keys extension is enabled in v21 so sort data is passed
# to the instance API and the sorted DB API is invoked
@ -391,7 +402,9 @@ class ServersControllerTest(ControllerTest):
"OS-EXT-SRV-ATTR:instance_name": "instance-00000002",
"key_name": '',
"OS-SRV-USG:launched_at": None,
"OS-SRV-USG:terminated_at": None
"OS-SRV-USG:terminated_at": None,
"security_groups": [{'name': 'fake-0-0'},
{'name': 'fake-0-1'}]
}
}
@ -1509,7 +1522,14 @@ class ServersControllerTestV23(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
def _get_server_data_dict(self, uuid, image_bookmark, flavor_bookmark,
status="ACTIVE", progress=100):
@ -1560,7 +1580,18 @@ class ServersControllerTestV23(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1,
'description': 'foo',
'user_id': 'bar', 'project_id': 'baz',
'deleted': False, 'deleted_at': None,
'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1,
'description': 'foo',
'user_id': 'bar', 'project_id': 'baz',
'deleted': False, 'deleted_at': None,
'updated_at': None, 'created_at': None}])
obj_list.append(server)
return objects.InstanceList(objects=obj_list)
@ -1595,7 +1626,14 @@ class ServersControllerTestV29(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
def _get_server_data_dict(self, uuid, image_bookmark, flavor_bookmark,
status="ACTIVE", progress=100):
@ -1631,7 +1669,14 @@ class ServersControllerTestV29(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
req = self.req('/fake/servers/%s' % FAKE_UUID)
res_dict = self.controller.show(req, FAKE_UUID)
@ -1671,7 +1716,14 @@ class ServersControllerTestV29(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
req = self.req('/fake/servers/detail')
servers_list = self.controller.detail(req)
@ -1729,7 +1781,14 @@ class ServersControllerTestV216(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
self.useFixture(fixtures.MockPatchObject(
compute_api.API, 'get_instance_host_status',
return_value='UP')).mock
@ -1784,7 +1843,18 @@ class ServersControllerTestV216(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1,
'description': 'foo',
'user_id': 'bar', 'project_id': 'baz',
'deleted': False, 'deleted_at': None,
'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1,
'description': 'foo',
'user_id': 'bar', 'project_id': 'baz',
'deleted': False, 'deleted_at': None,
'updated_at': None, 'created_at': None}])
obj_list.append(server)
return objects.InstanceList(objects=obj_list)
@ -1819,7 +1889,14 @@ class ServersControllerTestV219(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
self.useFixture(fixtures.MockPatchObject(
compute_api.API, 'get_instance_host_status',
return_value='UP')).mock
@ -1861,7 +1938,14 @@ class ServersControllerTestV219(ServersControllerTest):
metadata={"seq": "2"},
availability_zone='nova',
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
req = self.req('/fake/servers/%s' % FAKE_UUID)
res_dict = self.controller.show(req, FAKE_UUID)
@ -3271,6 +3355,7 @@ class ServerStatusTest(test.TestCase):
def setUp(self):
super(ServerStatusTest, self).setUp()
self.flags(use_neutron=False)
fakes.stub_out_nw_api(self)
self.controller = servers.ServersController()
@ -3392,6 +3477,15 @@ class ServersControllerCreateTest(test.TestCase):
"task_state": "",
"vm_state": "",
"root_device_name": inst.get('root_device_name', 'vda'),
"security_groups": [
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None,
'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None,
'created_at': None}]
})
self.instance_cache_by_id[instance['id']] = instance
@ -3462,6 +3556,9 @@ class ServersControllerCreateTest(test.TestCase):
self.req = fakes.HTTPRequest.blank('/fake/servers')
self.req.method = 'POST'
self.req.headers["content-type"] = "application/json"
server = dict(name='server_test', imageRef=FAKE_UUID, flavorRef=2)
body = {'server': server}
self.req.body = encodeutils.safe_encode(jsonutils.dumps(body))
def _check_admin_password_len(self, server_dict):
"""utility function - check server_dict for admin_password length."""
@ -6159,6 +6256,9 @@ class ServersViewBuilderTest(test.TestCase):
def setUp(self):
super(ServersViewBuilderTest, self).setUp()
self.flags(use_ipv6=True)
# Neutron security groups are tested in test_neutron_security_groups.py
self.flags(use_neutron=False)
fakes.stub_out_nw_api(self)
self.flags(group='glance', api_servers=['http://localhost:9292'])
nw_cache_info = self._generate_nw_cache_info()
db_inst = fakes.stub_instance(
@ -6170,7 +6270,14 @@ class ServersViewBuilderTest(test.TestCase):
availability_zone='nova',
nw_cache=nw_cache_info,
launched_at=None,
terminated_at=None)
terminated_at=None,
security_groups=[
{'name': 'fake-0-0', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None},
{'name': 'fake-0-1', 'id': 1, 'description': 'foo',
'user_id': 'bar', 'project_id': 'baz', 'deleted': False,
'deleted_at': None, 'updated_at': None, 'created_at': None}])
privates = ['172.19.0.1']
publics = ['192.168.0.3']
@ -6351,7 +6458,9 @@ class ServersViewBuilderTest(test.TestCase):
"OS-EXT-SRV-ATTR:instance_name": "instance-00000001",
"key_name": '',
"OS-SRV-USG:launched_at": None,
"OS-SRV-USG:terminated_at": None
"OS-SRV-USG:terminated_at": None,
"security_groups": [{'name': 'fake-0-0'},
{'name': 'fake-0-1'}]
}
}
@ -6438,7 +6547,9 @@ class ServersViewBuilderTest(test.TestCase):
"OS-EXT-SRV-ATTR:instance_name": "instance-00000001",
"key_name": '',
"OS-SRV-USG:launched_at": None,
"OS-SRV-USG:terminated_at": None
"OS-SRV-USG:terminated_at": None,
"security_groups": [{'name': 'fake-0-0'},
{'name': 'fake-0-1'}]
}
}
@ -6625,7 +6736,9 @@ class ServersViewBuilderTest(test.TestCase):
"OS-EXT-SRV-ATTR:instance_name": "instance-00000001",
"key_name": '',
"OS-SRV-USG:launched_at": None,
"OS-SRV-USG:terminated_at": None
"OS-SRV-USG:terminated_at": None,
"security_groups": [{'name': 'fake-0-0'},
{'name': 'fake-0-1'}]
}
}
@ -6709,7 +6822,9 @@ class ServersViewBuilderTest(test.TestCase):
"OS-EXT-SRV-ATTR:instance_name": "instance-00000001",
"key_name": '',
"OS-SRV-USG:launched_at": None,
"OS-SRV-USG:terminated_at": None
"OS-SRV-USG:terminated_at": None,
"security_groups": [{'name': 'fake-0-0'},
{'name': 'fake-0-1'}]
}
}