From: Ihar Hrachyshka Date: Fri, 10 Jul 2015 12:07:09 +0000 (+0200) Subject: Enforce specific order for firewall.(un)filtered_ports and devices X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=5b066a237ec0918d882ef2455aef4f2f9cb0606c;p=openstack-build%2Fneutron-build.git Enforce specific order for firewall.(un)filtered_ports and devices Lots of tests in the file rely on specific order of devices and ports with which they are iterated thru inside firewall implementation. This is needed to match a regexp against iptables output generated by the firewall driver. In production code, those .(un)filtered_ports dictionaries are unordered, and it would be not wise to enforce the order for them just for the sake of those unit tests. Instead, we 'patch' the agent firewall with ordered versions of dict for those attributes. Also enforce specific order for device_info dictionaries we pass into firewall. The failure was easily reproducible with PYTHONHASHSEED=111, and after the fix, it's gone. While at it, stop making assumptions about stable order of dict.values() between multiple dictionaries with the same keys. It may actually work for now, but it seems fragile. Overall simplified regex construction code a bit, f.e. killing some dead or redundant code. Closes-Bug: #1473413 Change-Id: I170087426bc961592b4c4923c64a5fea30d51c21 --- diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py index 0ff86ceb2..eb4f1ac83 100644 --- a/neutron/tests/unit/agent/test_securitygroups_rpc.py +++ b/neutron/tests/unit/agent/test_securitygroups_rpc.py @@ -1637,23 +1637,12 @@ COMMIT CHAINS_NAT = 'OUTPUT|POSTROUTING|PREROUTING|float-snat|snat' -# These Dicts use the same keys as devices2 and devices3 in -# TestSecurityGroupAgentWithIptables() to ensure that the ordering -# is consistent regardless of hashseed value -PORTS = {'tap_port1': 'port1', 'tap_port2': 'port2'} -MACS = {'tap_port1': '12:34:56:78:9A:BC', 'tap_port2': '12:34:56:78:9A:BD'} -IPS = {'tap_port1': '10.0.0.3/32', 'tap_port2': '10.0.0.4/32'} - -ports_values = list(PORTS.values()) -macs_values = list(MACS.values()) -ips_values = list(IPS.values()) - -IPTABLES_ARG['port1'] = ports_values[0] -IPTABLES_ARG['port2'] = ports_values[1] -IPTABLES_ARG['mac1'] = macs_values[0] -IPTABLES_ARG['mac2'] = macs_values[1] -IPTABLES_ARG['ip1'] = ips_values[0] -IPTABLES_ARG['ip2'] = ips_values[1] +IPTABLES_ARG['port1'] = 'port1' +IPTABLES_ARG['port2'] = 'port2' +IPTABLES_ARG['mac1'] = '12:34:56:78:9A:BC' +IPTABLES_ARG['mac2'] = '12:34:56:78:9A:BD' +IPTABLES_ARG['ip1'] = '10.0.0.3/32' +IPTABLES_ARG['ip2'] = '10.0.0.4/32' IPTABLES_ARG['chains'] = CHAINS_NAT IPTABLES_RAW_DEFAULT = """# Generated by iptables_manager @@ -2136,12 +2125,6 @@ COMMIT # Completed by iptables_manager """ % IPTABLES_ARG -# These Dicts use the same keys as devices2 and devices3 in -# TestSecurityGroupAgentWithIptables() to ensure that the ordering -# is consistent regardless of hashseed value -REVERSE_PORT_ORDER = {'tap_port1': False, 'tap_port2': True} -reverse_port_order_values = list(REVERSE_PORT_ORDER.values()) - IPTABLES_FILTER_2_2 = """# Generated by iptables_manager *filter :neutron-filter-top - [0:0] @@ -2174,10 +2157,6 @@ IPTABLES_FILTER_2_2 = """# Generated by iptables_manager --dport 68 -j RETURN [0:0] -A %(bn)s-i_%(port1)s -p tcp -m tcp --dport 22 -j RETURN """ % IPTABLES_ARG -if reverse_port_order_values[0]: - IPTABLES_FILTER_2_2 += ("[0:0] -A %(bn)s-i_%(port1)s -s %(ip2)s " - "-j RETURN\n" - % IPTABLES_ARG) IPTABLES_FILTER_2_2 += """[0:0] -A %(bn)s-i_%(port1)s -j %(bn)s-sg-fallback [0:0] -A %(bn)s-FORWARD %(physdev_mod)s --physdev-EGRESS tap_%(port1)s \ %(physdev_is_bridged)s -j %(bn)s-sg-chain @@ -2205,10 +2184,9 @@ IPTABLES_FILTER_2_2 += """[0:0] -A %(bn)s-i_%(port1)s -j %(bn)s-sg-fallback --dport 68 -j RETURN [0:0] -A %(bn)s-i_%(port2)s -p tcp -m tcp --dport 22 -j RETURN """ % IPTABLES_ARG -if not reverse_port_order_values[0]: - IPTABLES_FILTER_2_2 += ("[0:0] -A %(bn)s-i_%(port2)s -s %(ip1)s " - "-j RETURN\n" - % IPTABLES_ARG) +IPTABLES_FILTER_2_2 += ("[0:0] -A %(bn)s-i_%(port2)s -s %(ip1)s " + "-j RETURN\n" + % IPTABLES_ARG) IPTABLES_FILTER_2_2 += """[0:0] -A %(bn)s-i_%(port2)s -j %(bn)s-sg-fallback [0:0] -A %(bn)s-FORWARD %(physdev_mod)s --physdev-EGRESS tap_%(port2)s \ %(physdev_is_bridged)s -j %(bn)s-sg-chain @@ -2554,27 +2532,39 @@ class TestSecurityGroupAgentWithIptables(base.BaseTestCase): '10.0.0.3/32', '12:34:56:78:9a:bc', rule1)} - self.devices2 = {'tap_port1': self._device('tap_port1', - '10.0.0.3/32', - '12:34:56:78:9a:bc', - rule2), - 'tap_port2': self._device('tap_port2', - '10.0.0.4/32', - '12:34:56:78:9a:bd', - rule4)} - self.devices3 = {'tap_port1': self._device('tap_port1', - '10.0.0.3/32', - '12:34:56:78:9a:bc', - rule3), - 'tap_port2': self._device('tap_port2', - '10.0.0.4/32', - '12:34:56:78:9a:bd', - rule5)} + self.devices2 = collections.OrderedDict([ + ('tap_port1', self._device('tap_port1', + '10.0.0.3/32', + '12:34:56:78:9a:bc', + rule2)), + ('tap_port2', self._device('tap_port2', + '10.0.0.4/32', + '12:34:56:78:9a:bd', + rule4)) + ]) + self.devices3 = collections.OrderedDict([ + ('tap_port1', self._device('tap_port1', + '10.0.0.3/32', + '12:34:56:78:9a:bc', + rule3)), + ('tap_port2', self._device('tap_port2', + '10.0.0.4/32', + '12:34:56:78:9a:bd', + rule5)) + ]) + + @staticmethod + def _enforce_order_in_firewall(firewall): + # for the sake of the test, eliminate any order randomness: + # it helps to match iptables output against regexps consistently + for attr in ('filtered_ports', 'unfiltered_ports'): + setattr(firewall, attr, collections.OrderedDict()) def _init_agent(self, defer_refresh_firewall): self.agent = sg_rpc.SecurityGroupAgentRpc( context=None, plugin_rpc=self.rpc, defer_refresh_firewall=defer_refresh_firewall) + self._enforce_order_in_firewall(self.agent.firewall) def _device(self, device, ip, mac_address, rule): return {'device': device, @@ -2742,14 +2732,16 @@ class TestSecurityGroupAgentEnhancedRpcWithIptables( 'security_group1': { 'IPv4': ['10.0.0.3/32'], 'IPv6': []}}, 'devices': devices_info1} - devices_info2 = {'tap_port1': self._device('tap_port1', - '10.0.0.3/32', - '12:34:56:78:9a:bc', - []), - 'tap_port2': self._device('tap_port2', - '10.0.0.4/32', - '12:34:56:78:9a:bd', - [])} + devices_info2 = collections.OrderedDict([ + ('tap_port1', self._device('tap_port1', + '10.0.0.3/32', + '12:34:56:78:9a:bc', + [])), + ('tap_port2', self._device('tap_port2', + '10.0.0.4/32', + '12:34:56:78:9a:bd', + [])) + ]) self.devices_info2 = {'security_groups': {'security_group1': rule1}, 'sg_member_ips': { 'security_group1': { @@ -2943,6 +2935,7 @@ class TestSecurityGroupAgentWithOVSIptables( context=None, plugin_rpc=self.rpc, local_vlan_map=local_vlan_map, defer_refresh_firewall=defer_refresh_firewall) + self._enforce_order_in_firewall(self.agent.firewall) def test_prepare_remove_port(self): self.rpc.security_group_rules_for_devices.return_value = self.devices1