openstack/deb-heat
Code Issues Proposed changes

Match tenant_id when name ambiguity in get_secgroup_uuids()

Browse Source 
Neutron allows securityGroup creation with same name
between one/more users, and the admin roles can get/use
the other users' securityGroup. So if there are several
securityGroups return, we should match the one which is
belongs to the stack owner, and then to raise
'PhysicalResourceNameAmbiguity' exception if can't match.

Change-Id: Icb7eae35ba40a142a4d398b7c9d58c8483b9588e
Closes-Bug: #1348064
This commit is contained in:
huangtianhua 2014-07-24 15:52:11 +08:00
parent 4adbf91e6f
commit 945f3fe95a
8 changed files with 229 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
heat/engine/clients/os/neutron.py
View File

@ -15,8 +15,10 @@ from neutronclient.common import exceptions
from neutronclient.neutron import v2_0 as neutronV20
from neutronclient.v2_0 import client as nc
from heat.common import exception
from heat.engine.clients import client_plugin
from heat.engine import constraints
from heat.openstack.common import uuidutils
class NeutronClientPlugin(client_plugin.ClientPlugin):
@ -81,6 +83,39 @@ class NeutronClientPlugin(client_plugin.ClientPlugin):
subnet_info = self.client().show_subnet(subnet_id)
return subnet_info['subnet']['network_id']
def get_secgroup_uuids(self, security_groups):
'''
Returns a list of security group UUIDs.
Args:
security_groups: List of security group names or UUIDs
'''
seclist = []
all_groups = None
for sg in security_groups:
if uuidutils.is_uuid_like(sg):
seclist.append(sg)
else:
if not all_groups:
response = self.client().list_security_groups()
all_groups = response['security_groups']
same_name_groups = [g for g in all_groups if g['name'] == sg]
groups = [g['id'] for g in same_name_groups]
if len(groups) == 0:
raise exception.PhysicalResourceNotFound(resource_id=sg)
elif len(groups) == 1:
seclist.append(groups[0])
else:
# for admin roles, can get the other users'
# securityGroups, so we should match the tenant_id with
# the groups, and return the own one
own_groups = [g['id'] for g in same_name_groups
if g['tenant_id'] == self.context.tenant_id]
if len(own_groups) == 1:
seclist.append(own_groups[0])
else:
raise exception.PhysicalResourceNameAmbiguity(name=sg)
return seclist
class NetworkConstraint(constraints.BaseCustomConstraint):

5
heat/engine/resources/instance.py
View File

@ -21,7 +21,6 @@ from heat.engine import attributes
from heat.engine import constraints
from heat.engine import properties
from heat.engine import resource
from heat.engine.resources.neutron import neutron
from heat.engine.resources import volume
from heat.engine import scheduler
from heat.engine import signal_responder
@ -483,8 +482,8 @@ class Instance(resource.Resource):
if security_groups:
props['security_groups'] = \
neutron.NeutronResource.get_secgroup_uuids(
security_groups, self.neutron())
self.client_plugin('neutron').get_secgroup_uuids(
security_groups)
port = neutronclient.create_port({'port': props})['port']
nics = [{'port-id': port['id']}]

5
heat/engine/resources/network_interface.py
View File

@ -14,7 +14,6 @@
from heat.engine import attributes
from heat.engine import properties
from heat.engine import resource
from heat.engine.resources.neutron import neutron
class NetworkInterface(resource.Resource):
@ -118,8 +117,8 @@ class NetworkInterface(resource.Resource):
}
if self.properties[self.GROUP_SET]:
sgs = neutron.NeutronResource.get_secgroup_uuids(
self.properties.get(self.GROUP_SET), self.neutron())
sgs = self.client_plugin().get_secgroup_uuids(
self.properties.get(self.GROUP_SET))
props['security_groups'] = sgs
port = client.create_port({'port': props})['port']
self.resource_id_set(port['id'])

26
heat/engine/resources/neutron/neutron.py
View File

@ -11,6 +11,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import warnings
from heat.common import exception
from heat.engine import resource
from heat.engine import scheduler
@ -140,13 +142,17 @@ class NeutronResource(resource.Resource):
return unicode(self.resource_id)
@staticmethod
def get_secgroup_uuids(security_groups, client):
def get_secgroup_uuids(security_groups, client, tenant_id):
'''
Returns a list of security group UUIDs.
Args:
security_groups: List of security group names or UUIDs
client: reference to neutronclient
tenant_id: the tenant id to match the security_groups
'''
warnings.warn('neutron.NeutronResource.get_secgroup_uuids is '
'deprecated. Use '
'self.client_plugin("neutron").get_secgroup_uuids')
seclist = []
all_groups = None
for sg in security_groups:
@ -156,12 +162,22 @@ class NeutronResource(resource.Resource):
if not all_groups:
response = client.list_security_groups()
all_groups = response['security_groups']
groups = [g['id'] for g in all_groups if g['name'] == sg]
same_name_groups = [g for g in all_groups if g['name'] == sg]
groups = [g['id'] for g in same_name_groups]
if len(groups) == 0:
raise exception.PhysicalResourceNotFound(resource_id=sg)
if len(groups) > 1:
raise exception.PhysicalResourceNameAmbiguity(name=sg)
seclist.append(groups[0])
elif len(groups) == 1:
seclist.append(groups[0])
else:
# for admin roles, can get the other users'
# securityGroups, so we should match the tenant_id with
# the groups, and return the own one
own_groups = [g['id'] for g in same_name_groups
if g['tenant_id'] == tenant_id]
if len(own_groups) == 1:
seclist.append(own_groups[0])
else:
raise exception.PhysicalResourceNameAmbiguity(name=sg)
return seclist
def _delete_task(self):

4
heat/engine/resources/neutron/port.py
View File

@ -247,8 +247,8 @@ class Port(neutron.NeutronResource):
del pair[self.ALLOWED_ADDRESS_PAIR_MAC_ADDRESS]
if props.get(self.SECURITY_GROUPS):
props[self.SECURITY_GROUPS] = self.get_secgroup_uuids(
props.get(self.SECURITY_GROUPS), self.neutron())
props[self.SECURITY_GROUPS] = self.client_plugin().\
get_secgroup_uuids(props.get(self.SECURITY_GROUPS))
else:
props.pop(self.SECURITY_GROUPS, None)

6
heat/tests/test_instance.py
View File

@ -1281,7 +1281,7 @@ class InstancesTest(HeatTestCase):
neutronclient.Client.create_port(
{'port': props}).MultipleTimes().AndReturn(
{'port': {'id': 'fake_port_id'}})
self.stub_keystoneclient()
self.m.ReplayAll()
if get_secgroup_raises:
@ -1302,24 +1302,28 @@ class InstancesTest(HeatTestCase):
fake_groups_list = {
'security_groups': [
{
'tenant_id': 'test_tenant_id',
'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
},
{
'tenant_id': 'test_tenant_id',
'id': '384ccd91-447c-4d83-832c-06974a7d3d05',
'name': 'security_group_2',
'security_group_rules': [],
'description': 'no protocol'
},
{
'tenant_id': 'test_tenant_id',
'id': 'e91a0007-06a6-4a4a-8edb-1d91315eb0ef',
'name': 'duplicate_group_name',
'security_group_rules': [],
'description': 'no protocol'
},
{
'tenant_id': 'test_tenant_id',
'id': '8be37f3c-176d-4826-aa17-77d1d9df7b2e',
'name': 'duplicate_group_name',
'security_group_rules': [],

90
heat/tests/test_neutron.py
View File

@ -537,6 +537,96 @@ class NeutronTest(HeatTestCase):
self.assertRaises(KeyError, res._resolve_attribute, 'attr3')
self.assertIsNone(res._resolve_attribute('attr2'))
def test_get_secgroup_uuids(self):
# test get_secgroup_uuids with uuid
security_groups = ['b62c3079-6946-44f5-a67b-6b9091884d4f',
'9887157c-d092-40f5-b547-6361915fce7d']
self.assertEqual(security_groups,
qr.get_secgroup_uuids(security_groups, None, None))
# test get_secgroup_uuids with name
secgroups = ['security_group_1']
expected_groups = ['0389f747-7785-4757-b7bb-2ab07e4b09c3']
ctx = utils.dummy_context(
tenant_id='dc4b074874244f7693dd65583733a758')
fake_groups_list = {
'security_groups': [
{
'tenant_id': 'dc4b074874244f7693dd65583733a758',
'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
}
]
}
nclient = neutronclient.Client()
self.m.StubOutWithMock(neutronclient.Client, 'list_security_groups')
neutronclient.Client.list_security_groups().AndReturn(
fake_groups_list)
self.m.ReplayAll()
self.assertEqual(expected_groups,
qr.get_secgroup_uuids(secgroups, nclient,
ctx.tenant_id))
self.m.VerifyAll()
self.m.UnsetStubs()
# test there are two securityGroups with same name, but there is
# one belongs to the tenant
fake_groups_list = {
'security_groups': [
{
'tenant_id': 'dc4b074874244f7693dd65583733a758',
'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
},
{
'tenant_id': '64395a8e5beb4930a18245f76a5b1570',
'id': '384ccd91-447c-4d83-832c-06974a7d3d05',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
}
]
}
self.m.StubOutWithMock(neutronclient.Client, 'list_security_groups')
neutronclient.Client.list_security_groups().AndReturn(
fake_groups_list)
self.m.ReplayAll()
self.assertEqual(expected_groups,
qr.get_secgroup_uuids(secgroups, nclient,
ctx.tenant_id))
self.m.VerifyAll()
self.m.UnsetStubs()
# test there are two securityGroups with same name, and the two
# all belong to the tenant
fake_groups_list = {
'security_groups': [
{
'tenant_id': 'dc4b074874244f7693dd65583733a758',
'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
},
{
'tenant_id': 'dc4b074874244f7693dd65583733a758',
'id': '384ccd91-447c-4d83-832c-06974a7d3d05',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
}
]
}
self.m.StubOutWithMock(neutronclient.Client, 'list_security_groups')
neutronclient.Client.list_security_groups().AndReturn(fake_groups_list)
self.m.ReplayAll()
self.assertRaises(exception.PhysicalResourceNameAmbiguity,
qr.get_secgroup_uuids,
secgroups, nclient, ctx.tenant_id)
self.m.VerifyAll()
self.m.UnsetStubs()
class NeutronNetTest(HeatTestCase):

73
heat/tests/test_neutron_client.py
View File

@ -13,7 +13,7 @@
import mock
from heat.common import exception
from heat.engine.clients.os import neutron
from heat.tests.common import HeatTestCase
from heat.tests import utils
@ -77,3 +77,74 @@ class NeutronClientPluginTests(NeutronClientPluginTestCase):
# in this case find_resourceid_by_name_or_id is not called
self.mock_find.assert_called_once_with(self.neutron_client, 'subnet',
'test_subnet')
def test_get_secgroup_uuids(self):
# test get from uuids
sgs_uuid = ['b62c3079-6946-44f5-a67b-6b9091884d4f',
'9887157c-d092-40f5-b547-6361915fce7d']
sgs_list = self.neutron_plugin.get_secgroup_uuids(sgs_uuid)
self.assertEqual(sgs_list, sgs_uuid)
# test get from name, return only one
sgs_non_uuid = ['security_group_1']
expected_groups = ['0389f747-7785-4757-b7bb-2ab07e4b09c3']
fake_list = {
'security_groups': [
{
'tenant_id': 'test_tenant_id',
'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
}
]
}
self.neutron_client.list_security_groups.return_value = fake_list
self.assertEqual(expected_groups,
self.neutron_plugin.get_secgroup_uuids(sgs_non_uuid))
# test only one belong to the tenant
fake_list = {
'security_groups': [
{
'tenant_id': 'test_tenant_id',
'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
},
{
'tenant_id': 'not_test_tenant_id',
'id': '384ccd91-447c-4d83-832c-06974a7d3d05',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
}
]
}
self.neutron_client.list_security_groups.return_value = fake_list
self.assertEqual(expected_groups,
self.neutron_plugin.get_secgroup_uuids(sgs_non_uuid))
# test there are two securityGroups with same name, and the two
# all belong to the tenant
fake_list = {
'security_groups': [
{
'tenant_id': 'test_tenant_id',
'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
},
{
'tenant_id': 'test_tenant_id',
'id': '384ccd91-447c-4d83-832c-06974a7d3d05',
'name': 'security_group_1',
'security_group_rules': [],
'description': 'no protocol'
}
]
}
self.neutron_client.list_security_groups.return_value = fake_list
self.assertRaises(exception.PhysicalResourceNameAmbiguity,
self.neutron_plugin.get_secgroup_uuids,
sgs_non_uuid)