Skip to content

Commit

Permalink
Raise if sec-groups and port id are provided on boot
Browse files Browse the repository at this point in the history
Currently in case port_id is provided on instance boot
security groups parameter is ignored. Need to clearly state
that specifying both parameters is not allowed and that Neutron
should be used for configuring security groups on port

Closes-bug: #1373774

Change-Id: I701faba1b37a7106cf86f7abf8e55f7289e1ff3b
  • Loading branch information
Oleg Bondarev committed Jan 26, 2015
1 parent 4511c8e commit d8cafb9
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 0 deletions.
6 changes: 6 additions & 0 deletions nova/exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,12 @@ class SecurityGroupRuleExists(Invalid):
msg_fmt = _("Rule already exists in group: %(rule)s")


class SecurityGroupNotAllowedTogetherWithPort(Invalid):
msg_fmt = _("It's not allowed to specify security groups if port_id "
"is provided on instance boot. Neutron should be used to "
"configure security groups on port.")


class NoUniqueMatch(NovaException):
msg_fmt = _("No Unique Match Found.")
code = 409
Expand Down
3 changes: 3 additions & 0 deletions nova/network/neutronv2/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,9 @@ def allocate_for_instance(self, context, instance, **kwargs):
ports_in_requested_order = []
nets_in_requested_order = []
for request in ordered_networks:
if security_groups and request.port_id:
raise exception.SecurityGroupNotAllowedTogetherWithPort()

# Network lookup for available network_id
network = None
for net in nets:
Expand Down
34 changes: 34 additions & 0 deletions nova/tests/unit/network/test_neutronv2.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ def setUp(self):
'port_id': self.port_data2[1]['id'],
'fixed_ip_address': fixed_ip_address,
'router_id': 'router_id1'}
self.sec_group_data = [{'id': 'test_secgroup_id1',
'name': 'test_secgroup_name'}]
self._returned_nw_info = []
self.mox.StubOutWithMock(neutronapi, 'get_client')
self.moxed_client = self.mox.CreateMock(client.Client)
Expand Down Expand Up @@ -455,9 +457,20 @@ def _stub_allocate_for_instance(self, net_idx=1, **kwargs):
self.mox.ReplayAll()
return api

security_groups = kwargs.get('security_groups')
if security_groups:
search_opts = {'tenant_id': self.instance.project_id}
self.moxed_client.list_security_groups(
**search_opts).AndReturn(
{'security_groups': self.sec_group_data})

ports_in_requested_net_order = []
nets_in_requested_net_order = []
for request in ordered_networks:
if (request.port_id and security_groups):
self.mox.ReplayAll()
return api

port_req_body = {
'port': {
'device_id': self.instance.uuid,
Expand Down Expand Up @@ -511,6 +524,8 @@ def _stub_allocate_for_instance(self, net_idx=1, **kwargs):
res_port = {'port': {'id': 'fake'}}
if has_extra_dhcp_opts:
port_req_body['port']['extra_dhcp_opts'] = dhcp_options
if security_groups:
port_req_body['port']['security_groups'] = security_groups
if kwargs.get('_break') == 'mac' + request.network_id:
self.mox.ReplayAll()
return api
Expand Down Expand Up @@ -879,6 +894,25 @@ def test_allocate_for_instance_accepts_only_portid(self):
objects=[objects.NetworkRequest(port_id='my_portid1')]))
self.assertEqual(self.port_data1, result)

def test_allocate_for_instance_with_sec_groups(self):
# Test that allocate_for_instance handles security groups if provided
self._allocate_for_instance(
1, security_groups=[self.sec_group_data[0]['id']])

def test_allocate_for_instance_with_sec_groups_and_port_id(self):
# Test that allocate_for_instance raises if security groups are
# provided together with port_id
requested_networks = objects.NetworkRequestList(
objects=[objects.NetworkRequest(port_id='my_portid1')])
security_groups = [self.sec_group_data[0]['id']]
api = self._stub_allocate_for_instance(
requested_networks=requested_networks,
security_groups=security_groups)
self.assertRaises(exception.SecurityGroupNotAllowedTogetherWithPort,
api.allocate_for_instance, self.context,
self.instance, requested_networks=requested_networks,
security_groups=security_groups)

def test_allocate_for_instance_not_enough_macs_via_ports(self):
# using a hypervisor MAC via a pre-created port will stop it being
# used to dynamically create a port on a network. We put the network
Expand Down

0 comments on commit d8cafb9

Please sign in to comment.