Pass correct 'security_groups' value for port operations

Make sure pass the correct value of 'security_groups'
property to neutronClient for OS::Neutron::Port resource
operations.

Change-Id: I84fa2c149c18b0b85be7cf1ff5a8ab2641a5c276
Closes-bug: #1369933

heat/engine/resources/neutron/port.py

@@ -127,7 +127,6 @@ class Port(neutron.NeutronResource):
         SECURITY_GROUPS: properties.Schema(
             properties.Schema.LIST,
             _('Security group IDs to associate with this port.'),
-            default=[],
             update_allowed=True
         ),
         ALLOWED_ADDRESS_PAIRS: properties.Schema(
@@ -245,7 +244,7 @@ class Port(neutron.NeutronResource):
         port = self.neutron().create_port({'port': props})['port']
         self.resource_id_set(port['id'])
 
-    def _prepare_port_properties(self, props):
+    def _prepare_port_properties(self, props, prepare_for_update=False):
         for fixed_ip in props.get(self.FIXED_IPS, []):
             for key, value in fixed_ip.items():
                 if value is None:
@@ -260,11 +259,18 @@ class Port(neutron.NeutronResource):
                     pair[self.ALLOWED_ADDRESS_PAIR_MAC_ADDRESS] is None):
                 del pair[self.ALLOWED_ADDRESS_PAIR_MAC_ADDRESS]
 
-        if props.get(self.SECURITY_GROUPS):
+        # if without 'security_groups', don't set the 'security_groups'
+        # property when creating, neutron will create the port with the
+        # 'default' securityGroup. If has the 'security_groups' and the
+        # value is [], which means to create the port without securityGroup.
+        if props.get(self.SECURITY_GROUPS) is not None:
             props[self.SECURITY_GROUPS] = self.client_plugin().\
                 get_secgroup_uuids(props.get(self.SECURITY_GROUPS))
         else:
-            props.pop(self.SECURITY_GROUPS, None)
+            # And the update should has the same behavior.
+            if prepare_for_update:
+                props[self.SECURITY_GROUPS] = self.client_plugin().\
+                    get_secgroup_uuids(['default'])
 
         if not props[self.FIXED_IPS]:
             del(props[self.FIXED_IPS])
@@ -316,7 +322,7 @@ class Port(neutron.NeutronResource):
     def handle_update(self, json_snippet, tmpl_diff, prop_diff):
         props = self.prepare_update_properties(json_snippet)
 
-        self._prepare_port_properties(props)
+        self._prepare_port_properties(props, prepare_for_update=True)
         LOG.debug('updating port with %s' % props)
         self.neutron().update_port(self.resource_id, {'port': props})
 

heat/tests/test_neutron.py

@@ -2055,7 +2055,9 @@ class NeutronFloatingIPTest(HeatTestCase):
                     'admin_state_up': True,
                     'name': 'test_port',
                     'device_id': 'd6b4d3a5-c700-476f-b609-1493dd9dadc2',
-                    'device_owner': 'network:floatingip'
+                    'device_owner': 'network:floatingip',
+                    'security_groups': [
+                        '8a2f582a-e1cd-480f-b85d-b02631c10656']
                 }
             }
         ).AndReturn(None)
@@ -2089,7 +2091,8 @@ class NeutronFloatingIPTest(HeatTestCase):
             }],
             "name": "test_port",
             "device_id": "d6b4d3a5-c700-476f-b609-1493dd9dadc2",
-            'device_owner': 'network:floatingip'
+            'device_owner': 'network:floatingip',
+            'security_groups': ['8a2f582a-e1cd-480f-b85d-b02631c10656']
         }
         update_snippet = rsrc_defn.ResourceDefinition(p.name, p.type(), props)
 
@@ -2605,7 +2608,7 @@ class NeutronPortTest(HeatTestCase):
         scheduler.TaskRunner(port.create)()
         self.m.VerifyAll()
 
-    def test_security_groups(self):
+    def _mock_create_with_security_groups(self, port_prop):
         neutronV20.find_resourceid_by_name_or_id(
             mox.IsA(neutronclient.Client),
             'network',
@@ -2616,20 +2619,10 @@ class NeutronPortTest(HeatTestCase):
             'subnet',
             'sub1234'
         ).AndReturn('sub1234')
-        neutronclient.Client.create_port({'port': {
+        neutronclient.Client.create_port({'port': port_prop}).AndReturn(
-            'network_id': u'net1234',
+            {'port': {
-            'security_groups': ['8a2f582a-e1cd-480f-b85d-b02631c10656',
-                                '024613dc-b489-4478-b46f-ada462738740'],
-            'fixed_ips': [
-                {'subnet_id': u'sub1234', 'ip_address': u'10.0.3.21'}
-            ],
-            'name': utils.PhysName('test_stack', 'port'),
-            'admin_state_up': True,
-            'device_owner': u'network:dhcp'}}
-        ).AndReturn({'port': {
                 "status": "BUILD",
-            "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766"
+                "id": "fc68ea2c-b60b-4b4f-bd82-94ec81110766"}})
-        }})
         neutronclient.Client.show_port(
             'fc68ea2c-b60b-4b4f-bd82-94ec81110766'
         ).AndReturn({'port': {
@@ -2639,6 +2632,20 @@ class NeutronPortTest(HeatTestCase):
 
         self.m.ReplayAll()
 
+    def test_security_groups(self):
+        port_prop = {
+            'network_id': u'net1234',
+            'security_groups': ['8a2f582a-e1cd-480f-b85d-b02631c10656',
+                                '024613dc-b489-4478-b46f-ada462738740'],
+            'fixed_ips': [
+                {'subnet_id': u'sub1234', 'ip_address': u'10.0.3.21'}
+            ],
+            'name': utils.PhysName('test_stack', 'port'),
+            'admin_state_up': True,
+            'device_owner': u'network:dhcp'}
+
+        self._mock_create_with_security_groups(port_prop)
+
         t = template_format.parse(neutron_port_template)
         t['Resources']['port']['Properties']['security_groups'] = [
             '8a2f582a-e1cd-480f-b85d-b02631c10656',
@@ -2650,6 +2657,28 @@ class NeutronPortTest(HeatTestCase):
 
         self.m.VerifyAll()
 
+    def test_security_groups_empty_list(self):
+        port_prop = {
+            'network_id': u'net1234',
+            'security_groups': [],
+            'fixed_ips': [
+                {'subnet_id': u'sub1234', 'ip_address': u'10.0.3.21'}
+            ],
+            'name': utils.PhysName('test_stack', 'port'),
+            'admin_state_up': True,
+            'device_owner': u'network:dhcp'}
+
+        self._mock_create_with_security_groups(port_prop)
+
+        t = template_format.parse(neutron_port_template)
+        t['Resources']['port']['Properties']['security_groups'] = []
+        stack = utils.parse_stack(t)
+
+        port = stack['port']
+        scheduler.TaskRunner(port.create)()
+
+        self.m.VerifyAll()
+
     def test_create_and_update_port(self):
         props = {'network_id': u'net1234',
                  'name': utils.PhysName('test_stack', 'port'),
@@ -2657,9 +2686,17 @@ class NeutronPortTest(HeatTestCase):
                  'device_owner': u'network:dhcp'}
         new_props = props.copy()
         new_props['name'] = "new_name"
+        new_props['security_groups'] = [
+            '8a2f582a-e1cd-480f-b85d-b02631c10656']
         new_props_update = new_props.copy()
         new_props_update.pop('network_id')
 
+        new_props1 = new_props.copy()
+        new_props1.pop('security_groups')
+        new_props_update1 = new_props_update.copy()
+        new_props_update1['security_groups'] = [
+            '0389f747-7785-4757-b7bb-2ab07e4b09c3']
+
         neutronV20.find_resourceid_by_name_or_id(
             mox.IsA(neutronclient.Client),
             'network',
@@ -2686,6 +2723,25 @@ class NeutronPortTest(HeatTestCase):
             {'port': new_props_update}
         ).AndReturn(None)
 
+        fake_groups_list = {
+            'security_groups': [
+                {
+                    'tenant_id': 'dc4b074874244f7693dd65583733a758',
+                    'id': '0389f747-7785-4757-b7bb-2ab07e4b09c3',
+                    'name': 'default',
+                    'security_group_rules': [],
+                    'description': 'no protocol'
+                }
+            ]
+        }
+        self.m.StubOutWithMock(neutronclient.Client, 'list_security_groups')
+        neutronclient.Client.list_security_groups().AndReturn(
+            fake_groups_list)
+        neutronclient.Client.update_port(
+            'fc68ea2c-b60b-4b4f-bd82-94ec81110766',
+            {'port': new_props_update1}
+        ).AndReturn(None)
+
         self.m.ReplayAll()
 
         # create port
@@ -2700,6 +2756,10 @@ class NeutronPortTest(HeatTestCase):
         update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(),
                                                       new_props)
         self.assertIsNone(port.handle_update(update_snippet, {}, {}))
+        # update again to test port without security group
+        update_snippet = rsrc_defn.ResourceDefinition(port.name, port.type(),
+                                                      new_props1)
+        self.assertIsNone(port.handle_update(update_snippet, {}, {}))
 
         self.m.VerifyAll()