Server secgroups and network/port together invalid

Nova silently ignores security_groups properties for a server if the
server has a 'networks' property that references a neutron port, the
intent being that security groups are assigned to the port(s).

Adds note to Server properties to this effect.
Validation fails for a Server if 'security_groups' is defined AND
the server has neutron Ports assigned.

Change-Id: I883761a0bb0e73a3fcc7c362d8e6dc5a099806e0
Closes-Bug: #1270313

heat/engine/resources/server.py

@@ -158,7 +158,9 @@ class Server(resource.Resource):
         ),
         SECURITY_GROUPS: properties.Schema(
             properties.Schema.LIST,
-            _('List of security group names or IDs.'),
+            _('List of security group names or IDs. Cannot be used if '
+              'neutron ports are associated with this server; assign '
+              'security groups to the ports instead.'),
             default=[]
         ),
         NETWORKS: properties.Schema(
@@ -554,10 +556,15 @@ class Server(resource.Resource):
             msg = _('Neither image nor bootable volume is specified for'
                     ' instance %s') % self.name
             raise exception.StackValidationFailed(message=msg)
+
         # network properties 'uuid' and 'network' shouldn't be used
         # both at once for all networks
         networks = self.properties.get(self.NETWORKS) or []
+        # record if any networks include explicit ports
+        networks_with_port = False
         for network in networks:
+            networks_with_port = networks_with_port or \
+                network.get(self.NETWORK_PORT)
             if network.get(self.NETWORK_UUID) and network.get(self.NETWORK_ID):
                 msg = _('Properties "%(uuid)s" and "%(id)s" are both set '
                         'to the network "%(network)s" for the server '
@@ -584,6 +591,13 @@ class Server(resource.Resource):
         if metadata is not None or personality is not None:
             limits = nova_utils.absolute_limits(self.nova())
 
+        # if 'security_groups' present for the server and explict 'port'
+        # in one or more entries in 'networks', raise validation error
+        if networks_with_port and self.properties.get(self.SECURITY_GROUPS):
+            raise exception.ResourcePropertyConflict(
+                self.SECURITY_GROUPS,
+                "/".join([self.NETWORKS, self.NETWORK_PORT]))
+
         # verify that the number of metadata entries is not greater
         # than the maximum number allowed in the provider's absolute
         # limits

heat/tests/test_server.py

@@ -578,6 +578,33 @@ class ServersTest(HeatTestCase):
                       str(ex))
         self.m.VerifyAll()
 
+    def test_server_validate_net_security_groups(self):
+        # Test that if network 'ports' are assigned security groups are
+        # not, because they'll be ignored
+        stack_name = 'srv_net_secgroups'
+        (t, stack) = self._setup_test_stack(stack_name)
+
+        t['Resources']['WebServer']['Properties']['networks'] = [
+            {'port': 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'}]
+        t['Resources']['WebServer']['Properties']['security_groups'] = \
+            ['my_security_group']
+
+        server = servers.Server('server_validate_net_security_groups',
+                                t['Resources']['WebServer'], stack)
+
+        self.m.StubOutWithMock(server, 'nova')
+        server.nova().MultipleTimes().AndReturn(self.fc)
+        self.m.StubOutWithMock(clients.OpenStackClients, 'nova')
+        clients.OpenStackClients.nova().MultipleTimes().AndReturn(self.fc)
+        self.m.ReplayAll()
+
+        error = self.assertRaises(exception.ResourcePropertyConflict,
+                                  server.validate)
+        self.assertEqual("Cannot define the following properties at the same "
+                         "time: security_groups, networks/port.",
+                         str(error))
+        self.m.VerifyAll()
+
     def test_server_delete(self):
         return_server = self.fc.servers.list()[1]
         server = self._create_test_server(return_server,