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 0ff86ceb212e5d985c3ac181e9ec69acf0616d84..eb4f1ac839cf8359ce2c0087147c4033e022d23e 100644 (file)
--- 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