Rewrite get_secgroup_uuids to avoid resource_by_refid

get_secgroup_uuids has changed in the following ways:
* No longer attempts to find a security group name via
  a resource_by_refid lookup. resource_by_refid only exists
  as a workaround for AWS compatibility resource which are
  not backed by an API. Usage of resource_by_refid should be
  avoided in native resources if at all possible.
* If the security group looks like a UUID, assume it is
  a UUID for a security group that exists
* Call client.list_security_groups only once, and only if
  there is a non-uuid security group specified
* Do not attempt to reduce duplicate security groups to a unique
  list. If that is important for resources it can be done in
  validation.
* Raise PhysicalResourceNotFound if name maps to no group
* Raise PhysicalResourceNameAmbiguity if name maps to more than one
  group

Change-Id: I106adc15a7882192884d37c0806947470d139434
Closes-Bug: #1250286

heat/engine/resources/network_interface.py

@@ -76,8 +76,7 @@ def handle_create(self):
 
         if self.properties['GroupSet']:
             sgs = neutron.NeutronResource.get_secgroup_uuids(
-                self.stack, self.properties, 'GroupSet', self.name,
-                self.neutron())
+                self.properties.get('GroupSet', []), self.neutron())
             props['security_groups'] = sgs
         port = client.create_port({'port': props})['port']
         self.resource_id_set(port['id'])

heat/engine/resources/neutron/neutron.py

@@ -20,6 +20,7 @@
 from heat.engine import resource
 
 from heat.openstack.common import log as logging
+from heat.openstack.common import uuidutils
 
 logger = logging.getLogger(__name__)
 
@@ -140,31 +141,26 @@ def FnGetRefId(self):
         return unicode(self.resource_id)
 
     @staticmethod
-    def get_secgroup_uuids(stack, props, props_name, rsrc_name, client):
+    def get_secgroup_uuids(security_groups, client):
         '''
-        Returns security group names in UUID form.
-
+        Returns a list of security group UUIDs.
         Args:
-            stack: stack associated with given resource
-            props: properties described in the template
-            props_name: name of security group property
-            rsrc_name: name of the given resource
+            security_groups: List of security group names or UUIDs
             client: reference to neutronclient
         '''
         seclist = []
-        for sg in props.get(props_name):
-            resource = stack.resource_by_refid(sg)
-            if resource is not None:
-                seclist.append(resource.resource_id)
+        all_groups = None
+        for sg in security_groups:
+            if uuidutils.is_uuid_like(sg):
+                seclist.append(sg)
             else:
-                try:
-                    client.show_security_group(sg)
-                    seclist.append(sg)
-                except NeutronClientException as e:
-                    if e.status_code == 404:
-                        raise exception.InvalidTemplateAttribute(
-                            resource=rsrc_name,
-                            key=props_name)
-                    else:
-                        raise
+                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]
+                if len(groups) == 0:
+                    raise exception.PhysicalResourceNotFound(resource_id=sg)
+                if len(groups) > 1:
+                    raise exception.PhysicalResourceNameAmbiguity(name=sg)
+                seclist.append(groups[0])
         return seclist

heat/engine/resources/neutron/port.py

@@ -114,8 +114,7 @@ def _prepare_list_properties(self, props):
 
         if props.get('security_groups'):
             props['security_groups'] = self.get_secgroup_uuids(
-                self.stack, props, 'security_groups', props.get('name'),
-                self.neutron())
+                props.get('security_groups'), self.neutron())
 
     def _show_resource(self):
         return self.neutron().show_port(

heat/tests/test_vpc.py

@@ -56,6 +56,7 @@ def setUp(self):
         self.m.StubOutWithMock(neutronclient.Client, 'show_router')
         self.m.StubOutWithMock(neutronclient.Client, 'create_security_group')
         self.m.StubOutWithMock(neutronclient.Client, 'show_security_group')
+        self.m.StubOutWithMock(neutronclient.Client, 'list_security_groups')
         self.m.StubOutWithMock(neutronclient.Client, 'delete_security_group')
         self.m.StubOutWithMock(
             neutronclient.Client, 'create_security_group_rule')
@@ -186,7 +187,7 @@ def mock_create_security_group(self):
                 'name': self.sg_name,
                 'description': 'SSH access',
                 'security_group_rules': [],
-                'id': 'eeee'
+                'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3'
             }
         })
 
@@ -199,7 +200,7 @@ def mock_create_security_group(self):
                 'ethertype': 'IPv4',
                 'port_range_max': '22',
                 'protocol': 'tcp',
-                'security_group_id': 'eeee'
+                'security_group_id': '0389f747-7785-4757-b7bb-2ab07e4b09c3'
             }
         }).AndReturn({
             'security_group_rule': {
@@ -210,14 +211,15 @@ def mock_create_security_group(self):
                 'ethertype': 'IPv4',
                 'port_range_max': '22',
                 'protocol': 'tcp',
-                'security_group_id': 'eeee',
+                'security_group_id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
                 'id': 'bbbb'
             }
         })
 
-    def mock_show_security_group(self, group='eeee'):
+    def mock_show_security_group(self, group=None):
         sg_name = utils.PhysName('test_stack', 'the_sg')
-        if group == 'eeee':
+        group = group or '0389f747-7785-4757-b7bb-2ab07e4b09c3'
+        if group == '0389f747-7785-4757-b7bb-2ab07e4b09c3':
             neutronclient.Client.show_security_group(group).AndReturn({
                 'security_group': {
                     'tenant_id': 'c1210485b2424d48804aad5d39c61b8f',
@@ -229,24 +231,28 @@ def mock_show_security_group(self, group='eeee'):
                         'port_range_max': '22',
                         'id': 'bbbb',
                         'ethertype': 'IPv4',
-                        'security_group_id': 'eeee',
+                        'security_group_id': ('0389f747-7785-4757-b7bb-'
+                                              '2ab07e4b09c3'),
                         'remote_group_id': None,
                         'remote_ip_prefix': '0.0.0.0/0',
                         'tenant_id': 'c1210485b2424d48804aad5d39c61b8f',
                         'port_range_min': '22'
                     }],
-                    'id': 'eeee'}})
+                    'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3'}})
         elif group == 'INVALID-NO-REF':
             neutronclient.Client.show_security_group(group).AndRaise(
                 NeutronClientException(status_code=404))
         elif group == 'RaiseException':
-            neutronclient.Client.show_security_group('eeee').AndRaise(
-                NeutronClientException(status_code=403))
+            neutronclient.Client.show_security_group(
+                '0389f747-7785-4757-b7bb-2ab07e4b09c3').AndRaise(
+                    NeutronClientException(status_code=403))
 
     def mock_delete_security_group(self):
         self.mock_show_security_group()
-        neutronclient.Client.delete_security_group_rule('bbbb').AndReturn(None)
-        neutronclient.Client.delete_security_group('eeee').AndReturn(None)
+        neutronclient.Client.delete_security_group_rule(
+            'bbbb').AndReturn(None)
+        neutronclient.Client.delete_security_group(
+            '0389f747-7785-4757-b7bb-2ab07e4b09c3').AndReturn(None)
 
     def mock_router_for_vpc(self):
         neutronclient.Client.list_routers(name=self.vpc_name).AndReturn({
@@ -338,9 +344,6 @@ def assertResourceState(self, resource, ref_id):
         self.assertEqual((resource.CREATE, resource.COMPLETE), resource.state)
         self.assertEqual(ref_id, resource.FnGetRefId())
 
-    def mock_rsrc_by_refid(self, sg):
-        parser.Stack.resource_by_refid(sg).AndReturn(None)
-
 
 class VPCTest(VPCTestBase):
 
@@ -526,7 +529,9 @@ class NetworkInterfaceTest(VPCTestBase):
       - INVALID-NO-REF
 '''
 
-    def mock_create_network_interface(self, security_groups=['eeee']):
+    def mock_create_network_interface(
+            self, security_groups=['0389f747-7785-4757-b7bb-2ab07e4b09c3']):
+
         self.nic_name = utils.PhysName('test_stack', 'the_nic')
         port = {'network_id': 'aaaa',
                 'fixed_ips': [{
@@ -591,15 +596,13 @@ def test_network_interface(self):
 
     def test_network_interface_existing_groupset(self):
         self.m.StubOutWithMock(parser.Stack, 'resource_by_refid')
-        self.mock_rsrc_by_refid(sg='eeee')
 
         self.mock_keystone()
         self.mock_create_security_group()
         self.mock_create_network()
         self.mock_create_subnet()
         self.mock_show_subnet()
         self.mock_create_network_interface()
-        self.mock_show_security_group()
         self.mock_delete_network_interface()
         self.mock_delete_subnet()
         self.mock_delete_network()
@@ -638,30 +641,6 @@ def test_network_interface_no_groupset(self):
 
         self.m.VerifyAll()
 
-    def test_network_interface_exception(self):
-        self.m.StubOutWithMock(parser.Stack, 'resource_by_refid')
-        self.mock_rsrc_by_refid(sg='eeee')
-
-        self.mock_keystone()
-        self.mock_create_security_group()
-        self.mock_create_network()
-        self.mock_create_subnet()
-        self.mock_show_subnet()
-        self.mock_show_security_group(group='RaiseException')
-
-        self.m.ReplayAll()
-
-        try:
-            stack = self.create_stack(self.test_template)
-            rsrc = stack['the_nic']
-            self.assertEqual((rsrc.CREATE, rsrc.FAILED), rsrc.state)
-            reason = rsrc.status_reason
-            self.assertTrue(reason.startswith('NeutronClientException:'))
-        finally:
-            stack.delete()
-
-        self.m.VerifyAll()
-
     def test_network_interface_error(self):
         real_exception = self.assertRaises(
             exception.InvalidTemplateReference,
@@ -673,30 +652,6 @@ def test_network_interface_error(self):
 
         self.assertEqual(str(expected_exception), str(real_exception))
 
-    def test_network_interface_error_no_ref(self):
-        self.mock_keystone()
-        self.mock_create_network()
-        self.mock_create_subnet()
-        self.mock_show_subnet()
-        self.mock_show_security_group(group='INVALID-NO-REF')
-        self.mock_delete_subnet()
-        neutronclient.Client.delete_port(None).AndReturn(None)
-        self.mock_delete_network()
-
-        self.m.ReplayAll()
-
-        stack = self.create_stack(self.test_template_error_no_ref)
-        try:
-            self.assertEqual((stack.CREATE, stack.FAILED), stack.state)
-            rsrc = stack['the_nic']
-            self.assertEqual((rsrc.CREATE, rsrc.FAILED), rsrc.state)
-            reason = rsrc.status_reason
-            self.assertTrue(reason.startswith('InvalidTemplateAttribute:'))
-        finally:
-            scheduler.TaskRunner(stack.delete)()
-
-        self.m.VerifyAll()
-
 
 class InternetGatewayTest(VPCTestBase):
 
@@ -741,12 +696,13 @@ def mock_create_internet_gateway(self):
                 'tenant_id': 'c1210485b2424d48804aad5d39c61b8f',
                 'admin_state_up': True,
                 'shared': True,
-                'id': 'eeee'
+                'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3'
             }]})
 
     def mock_create_gateway_attachment(self):
         neutronclient.Client.add_gateway_router(
-            'ffff', {'network_id': 'eeee'}).AndReturn(None)
+            'ffff', {'network_id': '0389f747-7785-4757-b7bb-2ab07e4b09c3'}
+        ).AndReturn(None)
 
     def mock_delete_gateway_attachment(self):
         neutronclient.Client.remove_gateway_router('ffff').AndReturn(None)