File bsc904389-stack-with-multiple-security-groups.p... of Package openstack-heat

Overview Repositories Revisions Requests Users Attributes Meta

File bsc904389-stack-with-multiple-security-groups.patch of Package openstack-heat (Revision 6ea86282ec77a41fdf6be5e82853988b)

Currently displaying revision 6ea86282ec77a41fdf6be5e82853988b , Show latest

From ee648d260e22c0bf943faaac8ad72f360c30e9f3 Mon Sep 17 00:00:00 2001
From: Alberto Planas <aplanas@gmail.com>
Date: Mon, 2 Mar 2015 16:52:24 +0100
Subject: [PATCH] Match tenant_id when name ambiguity in get_secgroup_uuids()

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.

Backport of:
https://git.openstack.org/cgit/openstack/heat/commit/?id=945f3fe95a5a00993dfa1a549b57279696d1606e
---
 heat/engine/resources/instance.py          |  3 +-
 heat/engine/resources/network_interface.py |  3 +-
 heat/engine/resources/neutron/neutron.py   | 21 +++++--
 heat/engine/resources/neutron/port.py      |  3 +-
 heat/tests/test_instance.py                |  4 ++
 heat/tests/test_neutron.py                 | 90 ++++++++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 8 deletions(-)

diff --git a/heat/engine/resources/instance.py b/heat/engine/resources/instance.py
index 3020dd8..a8672f4 100644
--- a/heat/engine/resources/instance.py
+++ b/heat/engine/resources/instance.py
@@ -385,7 +385,8 @@ def _build_nics(self, network_interfaces,
                     if security_groups:
                         props['security_groups'] = \
                             neutron.NeutronResource.get_secgroup_uuids(
-                                security_groups, self.neutron())
+                                security_groups, self.neutron(),
+                                self.context.tenant_id)
 
                     port = neutronclient.create_port({'port': props})['port']
                     nics = [{'port-id': port['id']}]
diff --git a/heat/engine/resources/network_interface.py b/heat/engine/resources/network_interface.py
index 7af5e76..b8c3bd5 100644
--- a/heat/engine/resources/network_interface.py
+++ b/heat/engine/resources/network_interface.py
@@ -111,7 +111,8 @@ def handle_create(self):
 
         if self.properties[self.GROUP_SET]:
             sgs = neutron.NeutronResource.get_secgroup_uuids(
-                self.properties.get(self.GROUP_SET), self.neutron())
+                self.properties.get(self.GROUP_SET), self.neutron(),
+                self.context.tenant_id)
             props['security_groups'] = sgs
         port = client.create_port({'port': props})['port']
         self.resource_id_set(port['id'])
diff --git a/heat/engine/resources/neutron/neutron.py b/heat/engine/resources/neutron/neutron.py
index 868b192..82203c6 100644
--- a/heat/engine/resources/neutron/neutron.py
+++ b/heat/engine/resources/neutron/neutron.py
@@ -143,12 +143,13 @@ def FnGetRefId(self):
         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
         '''
         seclist = []
         all_groups = None
@@ -159,12 +160,22 @@ def get_secgroup_uuids(security_groups, client):
                 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):
diff --git a/heat/engine/resources/neutron/port.py b/heat/engine/resources/neutron/port.py
index c6f9b95..68448d2 100644
--- a/heat/engine/resources/neutron/port.py
+++ b/heat/engine/resources/neutron/port.py
@@ -193,7 +193,8 @@ def _prepare_list_properties(self, props):
 
         if props.get(self.SECURITY_GROUPS):
             props[self.SECURITY_GROUPS] = self.get_secgroup_uuids(
-                props.get(self.SECURITY_GROUPS), self.neutron())
+                props.get(self.SECURITY_GROUPS), self.neutron(),
+                self.context.tenant_id)
         else:
             props.pop(self.SECURITY_GROUPS, None)
 
diff --git a/heat/tests/test_instance.py b/heat/tests/test_instance.py
index 3923378..e1a8076 100644
--- a/heat/tests/test_instance.py
+++ b/heat/tests/test_instance.py
@@ -795,24 +795,28 @@ def _get_fake_properties(self, sg='one'):
         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': [],
diff --git a/heat/tests/test_neutron.py b/heat/tests/test_neutron.py
index 75c1fcb..e82a5f4 100644
--- a/heat/tests/test_neutron.py
+++ b/heat/tests/test_neutron.py
@@ -358,6 +358,96 @@ def test_is_built(self):
             'status': 'FROBULATING'
         })
 
+    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()
+
 
 @skipIf(neutronclient is None, 'neutronclient unavailable')
 class NeutronNetTest(HeatTestCase):

Places

File bsc904389-stack-with-multiple-security-groups.patch of Package openstack-heat (Revision 6ea86282ec77a41fdf6be5e82853988b)

Places