Set security_groups when create internal ports for nova server

Make sure nova server be created in correct security groups
if user specified subnet and security_groups when create/update
server.

Change-Id: Ic93cad4def90f3da25390d871d6a8c14ffe1c5ae
Closes-Bug: #1571975
(cherry picked from commit 66b6490705affa7e11b4fce43f8f1face9e8767b)

4 files changed, 156 insertions, 17 deletions

diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py
index 6a1473b63..064397804 100644
--- a/heat/engine/resources/openstack/nova/server.py
+++ b/heat/engine/resources/openstack/nova/server.py
@@ -826,7 +826,8 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
         scheduler_hints = self._scheduler_hints(
             self.properties[self.SCHEDULER_HINTS])
 
-        nics = self._build_nics(self.properties[self.NETWORKS])
+        nics = self._build_nics(self.properties[self.NETWORKS],
+                                security_groups=security_groups)
         block_device_mapping = self._build_block_device_mapping(
             self.properties[self.BLOCK_DEVICE_MAPPING])
         block_device_mapping_v2 = self._build_block_device_mapping_v2(
@@ -1097,12 +1098,13 @@ class Server(stack_user.StackUser, sh.SchedulerHintsMixin,
         updaters = []
         new_networks = prop_diff.get(self.NETWORKS)
         old_networks = self.properties[self.NETWORKS]
+        security_groups = self.properties[self.SECURITY_GROUPS]
 
         if not server:
             server = self.client().servers.get(self.resource_id)
         interfaces = server.interface_list()
         remove_ports, add_nets = self.calculate_networks(
-            old_networks, new_networks, interfaces)
+            old_networks, new_networks, interfaces, security_groups)
 
         for port in remove_ports:
             updaters.append(
diff --git a/heat/engine/resources/openstack/nova/server_network_mixin.py b/heat/engine/resources/openstack/nova/server_network_mixin.py
index f15579873..e6430c591 100644
--- a/heat/engine/resources/openstack/nova/server_network_mixin.py
+++ b/heat/engine/resources/openstack/nova/server_network_mixin.py
@@ -74,11 +74,12 @@ class ServerNetworkMixin(object):
                         'network': net}
                     raise exception.StackValidationFailed(message=msg)
 
-    def _create_internal_port(self, net_data, net_number):
+    def _create_internal_port(self, net_data, net_number,
+                              security_groups=None):
         name = _('%(server)s-port-%(number)s') % {'server': self.name,
                                                   'number': net_number}
 
-        kwargs = self._prepare_internal_port_kwargs(net_data)
+        kwargs = self._prepare_internal_port_kwargs(net_data, security_groups)
         kwargs['name'] = name
 
         port = self.client('neutron').create_port({'port': kwargs})['port']
@@ -89,7 +90,7 @@ class ServerNetworkMixin(object):
 
         return port['id']
 
-    def _prepare_internal_port_kwargs(self, net_data):
+    def _prepare_internal_port_kwargs(self, net_data, security_groups=None):
         kwargs = {'network_id': self._get_network_id(net_data)}
         fixed_ip = net_data.get(self.NETWORK_FIXED_IP)
         subnet = net_data.get(self.NETWORK_SUBNET)
@@ -102,10 +103,9 @@ class ServerNetworkMixin(object):
         if body:
             kwargs.update({'fixed_ips': [body]})
 
-        if net_data.get(self.SECURITY_GROUPS):
+        if security_groups:
             sec_uuids = self.client_plugin(
-                'neutron').get_secgroup_uuids(net_data.get(
-                    self.SECURITY_GROUPS))
+                'neutron').get_secgroup_uuids(security_groups)
             kwargs['security_groups'] = sec_uuids
 
         extra_props = net_data.get(self.NETWORK_PORT_EXTRA)
@@ -197,7 +197,7 @@ class ServerNetworkMixin(object):
         for port_id in new_ports:
             self._data_update_ports(port_id, 'add', port_type='external_ports')
 
-    def _build_nics(self, networks):
+    def _build_nics(self, networks, security_groups=None):
         if not networks:
             return None
         nics = []
@@ -208,7 +208,8 @@ class ServerNetworkMixin(object):
             if net.get(self.NETWORK_PORT):
                 nic_info['port-id'] = net[self.NETWORK_PORT]
             elif self.is_using_neutron() and net.get(self.NETWORK_SUBNET):
-                nic_info['port-id'] = self._create_internal_port(net, idx)
+                nic_info['port-id'] = self._create_internal_port(
+                    net, idx, security_groups)
 
             # if nic_info including 'port-id', do not set ip for nic
             if not nic_info.get('port-id'):
@@ -311,7 +312,8 @@ class ServerNetworkMixin(object):
             if net is not None:
                 net['port'] = props['port']
 
-    def calculate_networks(self, old_nets, new_nets, ifaces):
+    def calculate_networks(self, old_nets, new_nets, ifaces,
+                           security_groups=None):
         remove_ports = []
         add_nets = []
         attach_first_free_port = False
@@ -368,8 +370,8 @@ class ServerNetworkMixin(object):
             if net.get(self.NETWORK_PORT):
                 handler_kwargs['port_id'] = net.get(self.NETWORK_PORT)
             elif self.is_using_neutron() and net.get(self.NETWORK_SUBNET):
-                handler_kwargs['port_id'] = self._create_internal_port(net,
-                                                                       idx)
+                handler_kwargs['port_id'] = self._create_internal_port(
+                    net, idx, security_groups)
 
             if not handler_kwargs['port_id']:
                 handler_kwargs['net_id'] = self._get_network_id(net)
diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py
index 369c7b497..cec477f27 100644
--- a/heat/tests/openstack/nova/test_server.py
+++ b/heat/tests/openstack/nova/test_server.py
@@ -123,6 +123,20 @@ resources:
         - network: 4321
 """
 
+tmpl_server_with_sub_secu_group = """
+heat_template_version: 2015-10-15
+resources:
+  server:
+    type: OS::Nova::Server
+    properties:
+      flavor: m1.small
+      image: F17-x86_64-gold
+      networks:
+        - subnet: 2a60cbaa-3d33-4af6-a9ce-83594ac546fc
+      security_groups:
+        - my_seg
+"""
+
 server_with_sw_config_personality = """
 heat_template_version: 2014-10-16
 resources:
@@ -357,6 +371,46 @@ class ServersTest(common.HeatTestCase):
         args, kwargs = mock_create.call_args
         self.assertEqual(kwargs['meta'], {'a': "1"})
 
+    def test_server_create_with_subnet_security_group(self):
+        stack_name = 'server_with_subnet_security_group'
+        self.patchobject(nova.NovaClientPlugin, '_create',
+                         return_value=self.fc)
+        return_server = self.fc.servers.list()[1]
+        (tmpl, stack) = self._setup_test_stack(
+            stack_name, test_templ=tmpl_server_with_sub_secu_group)
+
+        resource_defns = tmpl.resource_definitions(stack)
+        server = servers.Server('server_with_sub_secu',
+                                resource_defns['server'], stack)
+        mock_find = self.patchobject(
+            neutron.NeutronClientPlugin,
+            'find_resourceid_by_name_or_id',
+            return_value='2a60cbaa-3d33-4af6-a9ce-83594ac546fc')
+
+        sec_uuids = ['86c0f8ae-23a8-464f-8603-c54113ef5467']
+        self.patchobject(neutron.NeutronClientPlugin,
+                         'get_secgroup_uuids', return_value=sec_uuids)
+        self.patchobject(server, 'store_external_ports')
+        self.patchobject(neutron.NeutronClientPlugin,
+                         'network_id_from_subnet_id',
+                         return_value='05d8e681-4b37-4570-bc8d-810089f706b2')
+        mock_create_port = self.patchobject(
+            neutronclient.Client, 'create_port')
+
+        self.patchobject(
+            self.fc.servers, 'create', return_value=return_server)
+
+        scheduler.TaskRunner(server.create)()
+
+        kwargs = {'network_id': '05d8e681-4b37-4570-bc8d-810089f706b2',
+                  'fixed_ips': [
+                      {'subnet_id': '2a60cbaa-3d33-4af6-a9ce-83594ac546fc'}],
+                  'security_groups': sec_uuids,
+                  'name': 'server_with_sub_secu-port-0',
+                  }
+        mock_create_port.assert_called_with({'port': kwargs})
+        self.assertEqual(1, mock_find.call_count)
+
     def test_server_create_with_image_id(self):
         return_server = self.fc.servers.list()[1]
         return_server.id = '5678'
@@ -2976,6 +3030,58 @@ class ServersTest(common.HeatTestCase):
         self.assertEqual(1, mock_detach.call_count)
         self.assertEqual(1, mock_attach.call_count)
 
+    def test_server_update_subnet_with_security_group(self):
+        return_server = self.fc.servers.list()[3]
+        return_server.id = '9102'
+
+        server = self._create_test_server(return_server, 'update_subnet')
+        # set old properties for 'networks' and 'security_groups'
+        server.t['Properties']['networks'] = [
+            {'subnet': 'aaa09d50-8c23-4498-a542-aa0deb24f73e'}]
+        server.t['Properties']['security_groups'] = ['the_sg']
+        # set new property 'networks'
+        new_networks = [{'subnet': '2a60cbaa-3d33-4af6-a9ce-83594ac546fc'}]
+        update_template = copy.deepcopy(server.t)
+        update_template['Properties']['networks'] = new_networks
+
+        sec_uuids = ['86c0f8ae-23a8-464f-8603-c54113ef5467']
+
+        self.patchobject(self.fc.servers, 'get', return_value=return_server)
+        self.patchobject(neutron.NeutronClientPlugin,
+                         'get_secgroup_uuids', return_value=sec_uuids)
+        # execute translation rules need to call find_resourceid_by_name_or_id
+        mock_find = self.patchobject(
+            neutron.NeutronClientPlugin,
+            'find_resourceid_by_name_or_id',
+            side_effect=['2a60cbaa-3d33-4af6-a9ce-83594ac546fc',
+                         'aaa09d50-8c23-4498-a542-aa0deb24f73e',
+                         '2a60cbaa-3d33-4af6-a9ce-83594ac546fc'])
+        self.patchobject(neutron.NeutronClientPlugin,
+                         'network_id_from_subnet_id',
+                         return_value='05d8e681-4b37-4570-bc8d-810089f706b2')
+        mock_create_port = self.patchobject(
+            neutronclient.Client, 'create_port')
+
+        iface = self.create_fake_iface('aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa',
+                                       '05d8e681-4b37-4570-bc8d-810089f706b2',
+                                       '1.2.3.4')
+        self.patchobject(return_server, 'interface_list', return_value=[iface])
+        mock_detach = self.patchobject(return_server, 'interface_detach')
+        mock_attach = self.patchobject(return_server, 'interface_attach')
+
+        scheduler.TaskRunner(server.update, update_template, before=server.t)()
+        self.assertEqual((server.UPDATE, server.COMPLETE), server.state)
+        self.assertEqual(1, mock_detach.call_count)
+        self.assertEqual(1, mock_attach.call_count)
+        self.assertEqual(3, mock_find.call_count)
+        kwargs = {'network_id': '05d8e681-4b37-4570-bc8d-810089f706b2',
+                  'fixed_ips': [
+                      {'subnet_id': '2a60cbaa-3d33-4af6-a9ce-83594ac546fc'}],
+                  'security_groups': sec_uuids,
+                  'name': 'update_subnet-port-0',
+                  }
+        mock_create_port.assert_called_with({'port': kwargs})
+
     def test_server_update_empty_networks_with_complex_parameters(self):
         return_server = self.fc.servers.list()[3]
         return_server.id = '9102'
@@ -3558,6 +3664,8 @@ class ServerInternalPortTest(common.HeatTestCase):
             properties:
               flavor: m1.small
               image: F17-x86_64-gold
+              security_groups:
+                - test_sec
               networks:
                 - network: 4321
                   subnet: 1234
@@ -3567,12 +3675,15 @@ class ServerInternalPortTest(common.HeatTestCase):
         t, stack, server = self._return_template_stack_and_rsrc_defn('test',
                                                                      tmpl)
         self.patchobject(server, '_validate_belonging_subnet_to_net')
+        self.patchobject(neutron.NeutronClientPlugin,
+                         'get_secgroup_uuids', return_value=['5566'])
         self.port_create.return_value = {'port': {'id': '111222'}}
         data_set = self.patchobject(resource.Resource, 'data_set')
 
         network = [{'network': '4321', 'subnet': '1234',
                     'fixed_ip': '127.0.0.1'}]
-        server._build_nics(network)
+        security_groups = ['test_sec']
+        server._build_nics(network, security_groups)
 
         self.port_create.assert_called_once_with(
             {'port': {'name': 'server-port-0',
@@ -3580,7 +3691,8 @@ class ServerInternalPortTest(common.HeatTestCase):
                       'fixed_ips': [{
                           'ip_address': '127.0.0.1',
                           'subnet_id': '1234'
-                      }]}})
+                      }],
+                      'security_groups': ['5566']}})
         data_set.assert_called_once_with('internal_ports',
                                          '[{"id": "111222"}]')
 
@@ -3630,8 +3742,13 @@ class ServerInternalPortTest(common.HeatTestCase):
                            {'mac_address': '00:00:00:00:00:00'}
                        ]
                    }}
-        kwargs = server._prepare_internal_port_kwargs(network)
+        sec_uuids = ['8d94c72093284da88caaef5e985d96f7']
+        self.patchobject(neutron.NeutronClientPlugin,
+                         'get_secgroup_uuids', return_value=sec_uuids)
+        kwargs = server._prepare_internal_port_kwargs(
+            network, security_groups=['test_sec'])
         self.assertEqual({'network_id': '4321',
+                          'security_groups': sec_uuids,
                           'fixed_ips': [
                               {'ip_address': '127.0.0.1', 'subnet_id': '1234'}
                           ],
diff --git a/heat_integrationtests/functional/test_nova_server_networks.py b/heat_integrationtests/functional/test_nova_server_networks.py
index ae550b282..f258f8317 100644
--- a/heat_integrationtests/functional/test_nova_server_networks.py
+++ b/heat_integrationtests/functional/test_nova_server_networks.py
@@ -31,6 +31,16 @@ resources:
     properties:
       network: {get_resource: net}
       cidr: 11.11.11.0/24
+  security_group:
+    type: OS::Neutron::SecurityGroup
+    properties:
+      name: the_sg
+      description: Ping and SSH
+      rules:
+      - protocol: icmp
+      - protocol: tcp
+        port_range_min: 22
+        port_range_max: 22
   server:
     type: OS::Nova::Server
     properties:
@@ -39,6 +49,8 @@ resources:
       networks:
         - subnet: {get_resource: subnet}
           fixed_ip: 11.11.11.11
+      security_groups:
+        - {get_resource: security_group}
 outputs:
   networks:
     value: {get_attr: [server, networks]}
@@ -55,7 +67,7 @@ class CreateServerTest(functional_base.FunctionalTestsBase):
         output = self._stack_output(stack, output_key)
         return output
 
-    def test_create_server_with_subnet_fixed_ip(self):
+    def test_create_server_with_subnet_fixed_ip_sec_group(self):
         parms = {'flavor': self.conf.minimal_instance_type,
                  'image': self.conf.minimal_image_ref}
         stack_identifier = self.stack_create(
@@ -65,6 +77,12 @@ class CreateServerTest(functional_base.FunctionalTestsBase):
         networks = self.get_outputs(stack_identifier, 'networks')
         self.assertEqual(['11.11.11.11'], networks['my_net'])
 
+        server_resource = self.client.resources.get(
+            stack_identifier, 'server')
+        server_id = server_resource.physical_resource_id
+        server = self.compute_client.servers.get(server_id)
+        self.assertEqual([{"name": "the_sg"}], server.security_groups)
+
     def test_create_update_server_with_subnet(self):
         parms = {'flavor': self.conf.minimal_instance_type,
                  'image': self.conf.minimal_image_ref}