Populate default explicit allow rules for egress

This way, the default behavior becomes clear and
symmetric to ingress processing.

Fixes bug 1143283

Change-Id: Id6496819aaceda50def597739f7872653d5b2e00

diff --git a/quantum/db/securitygroups_db.py b/quantum/db/securitygroups_db.py
index 4c66a49774ee46a9b0e15addb151870006c586e9..c2b42b50fd5ceb7f174af6220af86999b6c37bf1 100644 (file)
--- a/quantum/db/securitygroups_db.py
+++ b/quantum/db/securitygroups_db.py
@@ -102,16 +102,23 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
                                               tenant_id=tenant_id,
                                               name=s['name'])
             context.session.add(security_group_db)
-            if s.get('name') == 'default':
-                for ethertype in ext_sg.sg_supported_ethertypes:
+            for ethertype in ext_sg.sg_supported_ethertypes:
+                if s.get('name') == 'default':
                     # Allow intercommunication
-                    db = SecurityGroupRule(
+                    ingress_rule = SecurityGroupRule(
                         id=uuidutils.generate_uuid(), tenant_id=tenant_id,
                         security_group=security_group_db,
                         direction='ingress',
                         ethertype=ethertype,
                         source_group=security_group_db)
-                    context.session.add(db)
+                    context.session.add(ingress_rule)
+
+                egress_rule = SecurityGroupRule(
+                    id=uuidutils.generate_uuid(), tenant_id=tenant_id,
+                    security_group=security_group_db,
+                    direction='egress',
+                    ethertype=ethertype)
+                context.session.add(egress_rule)
 
         return self._make_security_group_dict(security_group_db)
 

diff --git a/quantum/db/securitygroups_rpc_base.py b/quantum/db/securitygroups_rpc_base.py
index e41a24983aaa25b16e012a344e376598f32a2ded..5894f74cb10dfdf531edc4c3690cd9a485abaec9 100644 (file)
--- a/quantum/db/securitygroups_rpc_base.py
+++ b/quantum/db/securitygroups_rpc_base.py
@@ -224,21 +224,6 @@ class SecurityGroupServerRpcCallbackMixin(object):
             port['security_group_rules'] = updated_rule
         return ports
 
-    def _add_default_egress_rule(self, port, ethertype, ips):
-        """ Adding default egress rule which allows all egress traffic. """
-        egress_rule = [r for r in port['security_group_rules']
-                       if (r['direction'] == 'egress' and
-                           r['ethertype'] == ethertype)]
-        if len(egress_rule) > 0:
-            return
-        for ip in port['fixed_ips']:
-            version = netaddr.IPAddress(ip).version
-            if "IPv%s" % version == ethertype:
-                default_egress_rule = {'direction': 'egress',
-                                       'ethertype': ethertype}
-                port['security_group_rules'].append(default_egress_rule)
-                return
-
     def _add_ingress_dhcp_rule(self, port, ips):
         dhcp_ips = ips.get(port['network_id'])
         for dhcp_ip in dhcp_ips:
@@ -273,8 +258,6 @@ class SecurityGroupServerRpcCallbackMixin(object):
         network_ids = self._select_network_ids(ports)
         ips = self._select_dhcp_ips_for_network_ids(context, network_ids)
         for port in ports.values():
-            self._add_default_egress_rule(port, q_const.IPv4, ips)
-            self._add_default_egress_rule(port, q_const.IPv6, ips)
             self._add_ingress_ra_rule(port, ips)
             self._add_ingress_dhcp_rule(port, ips)
 

diff --git a/quantum/tests/unit/test_extension_security_group.py b/quantum/tests/unit/test_extension_security_group.py
index 547c063c8c39e164f28cc3588126f5dbd8e0d65d..ccb724a1f1479a5980e41d99044c4e6d5c219a6b 100644 (file)
--- a/quantum/tests/unit/test_extension_security_group.py
+++ b/quantum/tests/unit/test_extension_security_group.py
@@ -148,6 +148,25 @@ class SecurityGroupsTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
                 self._delete('security-group-rules',
                              security_group_rule['security_group_rule']['id'])
 
+    def _delete_default_security_group_egress_rules(self, security_group_id):
+        """Deletes default egress rules given a security group ID"""
+        res = self._list(
+            'security-group-rules',
+            query_params='security_group_id=%s' % security_group_id)
+
+        for r in res['security_group_rules']:
+            if (r['direction'] == 'egress' and not r['port_range_max'] and
+                    not r['port_range_min'] and not r['protocol']
+                    and not r['remote_ip_prefix']):
+                self._delete('security-group-rules', r['id'])
+
+    def _assert_sg_rule_has_kvs(self, security_group_rule, expected_kvs):
+        """Asserts that the sg rule has expected key/value pairs passed
+           in as expected_kvs dictionary
+        """
+        for k, v in expected_kvs.iteritems():
+            self.assertEquals(security_group_rule[k], v)
+
 
 class SecurityGroupsTestCaseXML(SecurityGroupsTestCase):
     fmt = 'xml'
@@ -232,6 +251,35 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
             for k, v, in keys:
                 self.assertEqual(security_group['security_group'][k], v)
 
+        # Verify that default egress rules have been created
+
+        sg_rules = security_group['security_group']['security_group_rules']
+        self.assertEquals(len(sg_rules), 2)
+
+        v4_rules = filter(lambda x: x['ethertype'] == 'IPv4', sg_rules)
+        self.assertEquals(len(v4_rules), 1)
+        v4_rule = v4_rules[0]
+        expected = {'direction': 'egress',
+                    'ethertype': 'IPv4',
+                    'remote_group_id': None,
+                    'remote_ip_prefix': None,
+                    'protocol': None,
+                    'port_range_max': None,
+                    'port_range_min': None}
+        self._assert_sg_rule_has_kvs(v4_rule, expected)
+
+        v6_rules = filter(lambda x: x['ethertype'] == 'IPv6', sg_rules)
+        self.assertEquals(len(v6_rules), 1)
+        v6_rule = v6_rules[0]
+        expected = {'direction': 'egress',
+                    'ethertype': 'IPv6',
+                    'remote_group_id': None,
+                    'remote_ip_prefix': None,
+                    'protocol': None,
+                    'port_range_max': None,
+                    'port_range_min': None}
+        self._assert_sg_rule_has_kvs(v6_rule, expected)
+
     def test_default_security_group(self):
         with self.network():
             res = self.new_list_request('security-groups')
@@ -372,7 +420,9 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                 sg_rule = group['security_group']['security_group_rules']
                 self.assertEqual(group['security_group']['id'],
                                  remote_group_id)
-                self.assertEqual(len(sg_rule), 1)
+                self.assertEqual(len(sg_rule), 3)
+                sg_rule = filter(lambda x: x['direction'] == 'ingress',
+                                 sg_rule)
                 for k, v, in keys:
                     self.assertEqual(sg_rule[0][k], v)
 
@@ -395,15 +445,79 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
             res = self.new_list_request('security-groups')
             groups = self.deserialize(self.fmt, res.get_response(self.ext_api))
             self.assertEqual(len(groups['security_groups']), 1)
+            security_group_id = groups['security_groups'][0]['id']
             res = self.new_list_request('security-group-rules')
             rules = self.deserialize(self.fmt, res.get_response(self.ext_api))
-            self.assertEqual(len(rules['security_group_rules']), 2)
-            # just generic rules to allow default egress and
-            # intergroup communicartion
-            for rule in rules['security_group_rules']:
-                self.assertEqual(rule['port_range_max'], None)
-                self.assertEqual(rule['port_range_min'], None)
-                self.assertEqual(rule['protocol'], None)
+            self.assertEqual(len(rules['security_group_rules']), 4)
+
+            # Verify default rule for v4 egress
+            sg_rules = rules['security_group_rules']
+            rules = filter(
+                lambda x: (
+                    x['direction'] == 'egress' and x['ethertype'] == 'IPv4'),
+                sg_rules)
+            self.assertEqual(len(rules), 1)
+            v4_egress = rules[0]
+
+            expected = {'direction': 'egress',
+                        'ethertype': 'IPv4',
+                        'remote_group_id': None,
+                        'remote_ip_prefix': None,
+                        'protocol': None,
+                        'port_range_max': None,
+                        'port_range_min': None}
+            self._assert_sg_rule_has_kvs(v4_egress, expected)
+
+            # Verify default rule for v6 egress
+            rules = filter(
+                lambda x: (
+                    x['direction'] == 'egress' and x['ethertype'] == 'IPv6'),
+                sg_rules)
+            self.assertEqual(len(rules), 1)
+            v6_egress = rules[0]
+
+            expected = {'direction': 'egress',
+                        'ethertype': 'IPv6',
+                        'remote_group_id': None,
+                        'remote_ip_prefix': None,
+                        'protocol': None,
+                        'port_range_max': None,
+                        'port_range_min': None}
+            self._assert_sg_rule_has_kvs(v6_egress, expected)
+
+            # Verify default rule for v4 ingress
+            rules = filter(
+                lambda x: (
+                    x['direction'] == 'ingress' and x['ethertype'] == 'IPv4'),
+                sg_rules)
+            self.assertEqual(len(rules), 1)
+            v4_ingress = rules[0]
+
+            expected = {'direction': 'ingress',
+                        'ethertype': 'IPv4',
+                        'remote_group_id': security_group_id,
+                        'remote_ip_prefix': None,
+                        'protocol': None,
+                        'port_range_max': None,
+                        'port_range_min': None}
+            self._assert_sg_rule_has_kvs(v4_ingress, expected)
+
+            # Verify default rule for v6 ingress
+            rules = filter(
+                lambda x: (
+                    x['direction'] == 'ingress' and x['ethertype'] == 'IPv6'),
+                sg_rules)
+            self.assertEqual(len(rules), 1)
+            v6_ingress = rules[0]
+
+            expected = {'direction': 'ingress',
+                        'ethertype': 'IPv6',
+                        'remote_group_id': security_group_id,
+                        'remote_ip_prefix': None,
+                        'protocol': None,
+                        'port_range_max': None,
+                        'port_range_min': None}
+            self._assert_sg_rule_has_kvs(v6_ingress, expected)
 
     def test_create_security_group_rule_remote_ip_prefix(self):
         name = 'webservers'
@@ -625,9 +739,16 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                                                             port_range_min=24,
                                                             port_range_max=24)
                                    ) as (sgr1, sgr2, sgr3):
+
+                # Delete default rules as they would fail the following
+                # assertion at the end.
+                self._delete_default_security_group_egress_rules(
+                    security_group_id)
+
+                q = 'direction=egress&security_group_id=' + security_group_id
                 self._test_list_resources('security-group-rule',
                                           [sgr1, sgr2, sgr3],
-                                          query_params="direction=egress")
+                                          query_params=q)
 
     def test_list_security_group_rules_with_sort(self):
         with self.security_group(name='sg') as sg:
@@ -645,10 +766,17 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                                                             port_range_min=24,
                                                             port_range_max=24)
                                    ) as (sgr1, sgr2, sgr3):
+
+                # Delete default rules as they would fail the following
+                # assertion at the end.
+                self._delete_default_security_group_egress_rules(
+                    security_group_id)
+
+                q = 'direction=egress&security_group_id=' + security_group_id
                 self._test_list_with_sort('security-group-rule',
                                           (sgr3, sgr2, sgr1),
                                           [('port_range_max', 'desc')],
-                                          query_params='direction=egress')
+                                          query_params=q)
 
     def test_list_security_group_rules_with_pagination(self):
         with self.security_group(name='sg') as sg:
@@ -666,10 +794,17 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                                                             port_range_min=24,
                                                             port_range_max=24)
                                    ) as (sgr1, sgr2, sgr3):
+
+                # Delete default rules as they would fail the following
+                # assertion at the end.
+                self._delete_default_security_group_egress_rules(
+                    security_group_id)
+
+                q = 'direction=egress&security_group_id=' + security_group_id
                 self._test_list_with_pagination(
                     'security-group-rule', (sgr3, sgr2, sgr1),
                     ('port_range_max', 'desc'), 2, 2,
-                    query_params='direction=egress')
+                    query_params=q)
 
     def test_list_security_group_rules_with_pagination_reverse(self):
         with self.security_group(name='sg') as sg:

diff --git a/quantum/tests/unit/test_security_groups_rpc.py b/quantum/tests/unit/test_security_groups_rpc.py
index 68a8288c8780c580969e7fe6b80b725d7f0984b2..21d61b4d0276741cc4f58b76eeb19a9f8dfe053b 100644 (file)
--- a/quantum/tests/unit/test_security_groups_rpc.py
+++ b/quantum/tests/unit/test_security_groups_rpc.py
@@ -84,7 +84,11 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                 ports_rpc = self.rpc.security_group_rules_for_devices(
                     ctx, devices=devices)
                 port_rpc = ports_rpc[port_id1]
-                expected = [{'direction': 'ingress',
+                expected = [{'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg1_id},
+                            {'direction': 'ingress',
                              'protocol': 'tcp', 'ethertype': 'IPv4',
                              'port_range_max': 22,
                              'security_group_id': sg1_id,
@@ -94,7 +98,6 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                              'port_range_max': 23, 'security_group_id': sg1_id,
                              'port_range_min': 23,
                              'source_ip_prefix': fake_prefix},
-                            {'ethertype': 'IPv4', 'direction': 'egress'},
                             ]
                 self.assertEqual(port_rpc['security_group_rules'],
                                  expected)
@@ -133,7 +136,11 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                 ports_rpc = self.rpc.security_group_rules_for_devices(
                     ctx, devices=devices)
                 port_rpc = ports_rpc[port_id1]
-                expected = [{'direction': 'egress',
+                expected = [{'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress',
                              'protocol': 'tcp', 'ethertype': 'IPv4',
                              'port_range_max': 22,
                              'security_group_id': sg1_id,
@@ -186,13 +193,20 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                 ports_rpc = self.rpc.security_group_rules_for_devices(
                     ctx, devices=devices)
                 port_rpc = ports_rpc[port_id1]
-                expected = [{'direction': u'ingress',
+                expected = [{'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg2_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg2_id},
+                            {'direction': u'ingress',
                              'source_ip_prefix': u'10.0.0.3/32',
                              'protocol': u'tcp', 'ethertype': u'IPv4',
                              'port_range_max': 25, 'port_range_min': 24,
                              'remote_group_id': sg2_id,
                              'security_group_id': sg1_id},
-                            {'ethertype': 'IPv4', 'direction': 'egress'},
                             ]
                 self.assertEqual(port_rpc['security_group_rules'],
                                  expected)
@@ -237,7 +251,11 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                 ports_rpc = self.rpc.security_group_rules_for_devices(
                     ctx, devices=devices)
                 port_rpc = ports_rpc[port_id1]
-                expected = [{'direction': 'ingress',
+                expected = [{'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg1_id},
+                            {'direction': 'ingress',
                              'protocol': 'tcp', 'ethertype': 'IPv6',
                              'port_range_max': 22,
                              'security_group_id': sg1_id,
@@ -247,7 +265,6 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                              'port_range_max': 23, 'security_group_id': sg1_id,
                              'port_range_min': 23,
                              'source_ip_prefix': fake_prefix},
-                            {'ethertype': 'IPv6', 'direction': 'egress'},
                             ]
                 self.assertEqual(port_rpc['security_group_rules'],
                                  expected)
@@ -292,7 +309,11 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                 ports_rpc = self.rpc.security_group_rules_for_devices(
                     ctx, devices=devices)
                 port_rpc = ports_rpc[port_id1]
-                expected = [{'direction': 'egress',
+                expected = [{'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress',
                              'protocol': 'tcp', 'ethertype': 'IPv6',
                              'port_range_max': 22,
                              'security_group_id': sg1_id,
@@ -352,13 +373,20 @@ class SGServerRpcCallBackMixinTestCase(test_sg.SecurityGroupDBTestCase):
                 ports_rpc = self.rpc.security_group_rules_for_devices(
                     ctx, devices=devices)
                 port_rpc = ports_rpc[port_id1]
-                expected = [{'direction': 'ingress',
+                expected = [{'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg1_id},
+                            {'direction': 'egress', 'ethertype': 'IPv4',
+                             'security_group_id': sg2_id},
+                            {'direction': 'egress', 'ethertype': 'IPv6',
+                             'security_group_id': sg2_id},
+                            {'direction': 'ingress',
                              'source_ip_prefix': 'fe80::3/128',
                              'protocol': 'tcp', 'ethertype': 'IPv6',
                              'port_range_max': 25, 'port_range_min': 24,
                              'remote_group_id': sg2_id,
                              'security_group_id': sg1_id},
-                            {'ethertype': 'IPv6', 'direction': 'egress'},
                             ]
                 self.assertEqual(port_rpc['security_group_rules'],
                                  expected)