]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix handling of port-range-min 0 in secgroup RPC and agent
authorshihanzhang <shihanzhang@huawei.com>
Tue, 14 Jul 2015 11:20:46 +0000 (19:20 +0800)
committershihanzhang <shihanzhang@huawei.com>
Thu, 23 Jul 2015 09:28:04 +0000 (17:28 +0800)
For TCP/UDP protocol, port valid range is 0 to 65535, so for a
security group rule, its valid range is also 0 to 65535. this
patch makes two changes:
1. if a security group rule port_range_min is 0, l2 agent also can
   get port_range_min real value 0 when it gets this rule for a
   device via RPC.
2. For IptablesFirewallDriver, if port range is [0, xxxx], l2 agent
   also need add this rule to iptables.

Change-Id: If93c54a31d973187889ead2c2797ffdd40a4393d
Closes-bug: #1473965

neutron/agent/linux/iptables_firewall.py
neutron/db/securitygroups_rpc_base.py
neutron/tests/unit/agent/linux/test_iptables_firewall.py
neutron/tests/unit/agent/test_securitygroups_rpc.py

index e4e1f171172fd83efa7b652e6c63294c18159c74..014873370d3ad3f9a284b0d2fbdbab3a2904b932 100644 (file)
@@ -566,7 +566,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
 
     def _port_arg(self, direction, protocol, port_range_min, port_range_max):
         if (protocol not in ['udp', 'tcp', 'icmp', 'icmpv6']
-            or not port_range_min):
+            or port_range_min is None):
             return []
 
         if protocol in ['icmp', 'icmpv6']:
index 63212fad92cdd232be6376f04cdae9b93660edf5..7ae461d74c7902ab5acb45488e68b7564db528b1 100644 (file)
@@ -194,7 +194,7 @@ class SecurityGroupServerRpcMixin(sg_db.SecurityGroupDbMixin):
 
             for key in ('protocol', 'port_range_min', 'port_range_max',
                         'remote_ip_prefix', 'remote_group_id'):
-                if rule_in_db.get(key):
+                if rule_in_db.get(key) is not None:
                     if key == 'remote_ip_prefix':
                         direction_ip_prefix = DIRECTION_IP_PREFIX[direction]
                         rule_dict[direction_ip_prefix] = rule_in_db[key]
@@ -440,7 +440,7 @@ class SecurityGroupServerRpcMixin(sg_db.SecurityGroupDbMixin):
             }
             for key in ('protocol', 'port_range_min', 'port_range_max',
                         'remote_ip_prefix', 'remote_group_id'):
-                if rule_in_db.get(key):
+                if rule_in_db.get(key) is not None:
                     if key == 'remote_ip_prefix':
                         direction_ip_prefix = DIRECTION_IP_PREFIX[direction]
                         rule_dict[direction_ip_prefix] = rule_in_db[key]
index d43532df01027a361f89a37bc8944be8dbada746..7b05a3fe649272c8afb9ff79d2d78ff0d381cae0 100644 (file)
@@ -604,6 +604,25 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
         egress = None
         self._test_prepare_port_filter(rule, ingress, egress)
 
+    def _test_filter_ingress_tcp_min_port_0(self, ethertype):
+        rule = {'ethertype': ethertype,
+                'direction': 'ingress',
+                'protocol': 'tcp',
+                'port_range_min': 0,
+                'port_range_max': 100}
+        ingress = mock.call.add_rule(
+            'ifake_dev',
+            '-p tcp -m tcp -m multiport --dports 0:100 -j RETURN',
+            comment=None)
+        egress = None
+        self._test_prepare_port_filter(rule, ingress, egress)
+
+    def test_filter_ingress_tcp_min_port_0_for_ipv4(self):
+        self._test_filter_ingress_tcp_min_port_0('IPv4')
+
+    def test_filter_ingress_tcp_min_port_0_for_ipv6(self):
+        self._test_filter_ingress_tcp_min_port_0('IPv6')
+
     def test_filter_ipv6_ingress_tcp_mport_prefix(self):
         prefix = FAKE_PREFIX['IPv6']
         rule = {'ethertype': 'IPv6',
index eb4f1ac839cf8359ce2c0087147c4033e022d23e..030899cf7a677dd8e3d247ad17e4f323c50d241a 100644 (file)
@@ -182,7 +182,8 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
             '192.168.1.3')
         self.assertFalse(self.notifier.security_groups_provider_updated.called)
 
-    def test_security_group_rules_for_devices_ipv4_ingress(self):
+    def _test_sg_rules_for_devices_ipv4_ingress_port_range(
+            self, min_port, max_port):
         fake_prefix = FAKE_PREFIX[const.IPv4]
         with self.network() as n,\
                 self.subnet(n),\
@@ -190,8 +191,8 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
             sg1_id = sg1['security_group']['id']
             rule1 = self._build_security_group_rule(
                 sg1_id,
-                'ingress', const.PROTO_NAME_TCP, '22',
-                '22')
+                'ingress', const.PROTO_NAME_TCP, str(min_port),
+                str(max_port))
             rule2 = self._build_security_group_rule(
                 sg1_id,
                 'ingress', const.PROTO_NAME_TCP, '23',
@@ -221,9 +222,9 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
                         {'direction': 'ingress',
                          'protocol': const.PROTO_NAME_TCP,
                          'ethertype': const.IPv4,
-                         'port_range_max': 22,
+                         'port_range_max': max_port,
                          'security_group_id': sg1_id,
-                         'port_range_min': 22},
+                         'port_range_min': min_port},
                         {'direction': 'ingress',
                          'protocol': const.PROTO_NAME_TCP,
                          'ethertype': const.IPv4,
@@ -235,6 +236,12 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
                              expected)
             self._delete('ports', port_id1)
 
+    def test_sg_rules_for_devices_ipv4_ingress_port_range_min_port_0(self):
+        self._test_sg_rules_for_devices_ipv4_ingress_port_range(0, 10)
+
+    def test_sg_rules_for_devices_ipv4_ingress_port_range_min_port_1(self):
+        self._test_sg_rules_for_devices_ipv4_ingress_port_range(1, 10)
+
     @contextlib.contextmanager
     def _port_with_addr_pairs_and_security_group(self):
         plugin_obj = manager.NeutronManager.get_plugin()