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
This commit is contained in:
parent
2d5d634f6c
commit
5b066a237e
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue