L3 agent: match format used by iptables
This fixes the iptables rules generated by the L3 agent
(SNAT, DNAT, set-mark and metadata), and the DHCP agent
(checksum-fill) to match the format that will be returned
by iptables-save to prevent excessive extra replacement
work done by the iptables manager.
It also fixes the iptables test that was not passing the
expected arguments (-p PROTO -m PROTO) for block rules.
A simple test was added to the L3 agent to ensure that the
rules have converged during the normal lifecycle tests.
Closes-Bug: #1566007
Change-Id: I5e8e27cdbf0d0448011881614671efe53bb1b6a1
(cherry picked from commit b8d520ffe2
)
This commit is contained in:
parent
3a922a2dfe
commit
a43b11e0aa
|
@ -143,11 +143,11 @@ class RouterInfo(object):
|
||||||
return self.router.get(l3_constants.FLOATINGIP_KEY, [])
|
return self.router.get(l3_constants.FLOATINGIP_KEY, [])
|
||||||
|
|
||||||
def floating_forward_rules(self, floating_ip, fixed_ip):
|
def floating_forward_rules(self, floating_ip, fixed_ip):
|
||||||
return [('PREROUTING', '-d %s -j DNAT --to %s' %
|
return [('PREROUTING', '-d %s/32 -j DNAT --to-destination %s' %
|
||||||
(floating_ip, fixed_ip)),
|
(floating_ip, fixed_ip)),
|
||||||
('OUTPUT', '-d %s -j DNAT --to %s' %
|
('OUTPUT', '-d %s/32 -j DNAT --to-destination %s' %
|
||||||
(floating_ip, fixed_ip)),
|
(floating_ip, fixed_ip)),
|
||||||
('float-snat', '-s %s -j SNAT --to %s' %
|
('float-snat', '-s %s/32 -j SNAT --to-source %s' %
|
||||||
(fixed_ip, floating_ip))]
|
(fixed_ip, floating_ip))]
|
||||||
|
|
||||||
def process_floating_ip_nat_rules(self):
|
def process_floating_ip_nat_rules(self):
|
||||||
|
|
|
@ -1292,7 +1292,7 @@ class DeviceManager(object):
|
||||||
"""Ensure DHCP reply packets always have correct UDP checksums."""
|
"""Ensure DHCP reply packets always have correct UDP checksums."""
|
||||||
iptables_mgr = iptables_manager.IptablesManager(use_ipv6=False,
|
iptables_mgr = iptables_manager.IptablesManager(use_ipv6=False,
|
||||||
namespace=namespace)
|
namespace=namespace)
|
||||||
ipv4_rule = ('-p udp --dport %d -j CHECKSUM --checksum-fill'
|
ipv4_rule = ('-p udp -m udp --dport %d -j CHECKSUM --checksum-fill'
|
||||||
% constants.DHCP_RESPONSE_PORT)
|
% constants.DHCP_RESPONSE_PORT)
|
||||||
iptables_mgr.ipv4['mangle'].add_rule('POSTROUTING', ipv4_rule)
|
iptables_mgr.ipv4['mangle'].add_rule('POSTROUTING', ipv4_rule)
|
||||||
iptables_mgr.apply()
|
iptables_mgr.apply()
|
||||||
|
|
|
@ -65,7 +65,7 @@ class MetadataDriver(object):
|
||||||
return [('PREROUTING', '-d 169.254.169.254/32 '
|
return [('PREROUTING', '-d 169.254.169.254/32 '
|
||||||
'-i %(interface_name)s '
|
'-i %(interface_name)s '
|
||||||
'-p tcp -m tcp --dport 80 -j REDIRECT '
|
'-p tcp -m tcp --dport 80 -j REDIRECT '
|
||||||
'--to-port %(port)s' %
|
'--to-ports %(port)s' %
|
||||||
{'interface_name': namespaces.INTERNAL_DEV_PREFIX + '+',
|
{'interface_name': namespaces.INTERNAL_DEV_PREFIX + '+',
|
||||||
'port': port})]
|
'port': port})]
|
||||||
|
|
||||||
|
|
|
@ -31,7 +31,8 @@ class IptablesManagerTestCase(functional_base.BaseSudoTestCase):
|
||||||
DIRECTION_CHAIN_MAPPER = {'ingress': 'INPUT',
|
DIRECTION_CHAIN_MAPPER = {'ingress': 'INPUT',
|
||||||
'egress': 'OUTPUT'}
|
'egress': 'OUTPUT'}
|
||||||
PROTOCOL_BLOCK_RULE = '-p %s -j DROP'
|
PROTOCOL_BLOCK_RULE = '-p %s -j DROP'
|
||||||
PROTOCOL_PORT_BLOCK_RULE = '-p %s --dport %d -j DROP'
|
PROTOCOL_PORT_BLOCK_RULE = ('-p %(protocol)s -m %(protocol)s '
|
||||||
|
'--dport %(port)d -j DROP')
|
||||||
|
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
super(IptablesManagerTestCase, self).setUp()
|
super(IptablesManagerTestCase, self).setUp()
|
||||||
|
@ -73,7 +74,8 @@ class IptablesManagerTestCase(functional_base.BaseSudoTestCase):
|
||||||
def _get_chain_and_rule(self, direction, protocol, port):
|
def _get_chain_and_rule(self, direction, protocol, port):
|
||||||
chain = self.DIRECTION_CHAIN_MAPPER[direction]
|
chain = self.DIRECTION_CHAIN_MAPPER[direction]
|
||||||
if port:
|
if port:
|
||||||
rule = self.PROTOCOL_PORT_BLOCK_RULE % (protocol, port)
|
rule = self.PROTOCOL_PORT_BLOCK_RULE % {'protocol': protocol,
|
||||||
|
'port': port}
|
||||||
else:
|
else:
|
||||||
rule = self.PROTOCOL_BLOCK_RULE % protocol
|
rule = self.PROTOCOL_BLOCK_RULE % protocol
|
||||||
return chain, rule
|
return chain, rule
|
||||||
|
|
|
@ -270,6 +270,12 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
|
||||||
'INPUT',
|
'INPUT',
|
||||||
metadata_port_filter))
|
metadata_port_filter))
|
||||||
|
|
||||||
|
def _assert_iptables_rules_converged(self, router):
|
||||||
|
# if your code is failing on this line, it means you are not generating
|
||||||
|
# your iptables rules in the same format that iptables-save returns
|
||||||
|
# them. run iptables-save to see the format they should be in
|
||||||
|
self.assertFalse(router.iptables_manager.apply())
|
||||||
|
|
||||||
def _assert_internal_devices(self, router):
|
def _assert_internal_devices(self, router):
|
||||||
internal_devices = router.router[l3_constants.INTERFACE_KEY]
|
internal_devices = router.router[l3_constants.INTERFACE_KEY]
|
||||||
self.assertTrue(len(internal_devices))
|
self.assertTrue(len(internal_devices))
|
||||||
|
@ -680,6 +686,7 @@ class L3AgentTestCase(L3AgentTestFramework):
|
||||||
self.assertTrue(self.floating_ips_configured(router))
|
self.assertTrue(self.floating_ips_configured(router))
|
||||||
self._assert_snat_chains(router)
|
self._assert_snat_chains(router)
|
||||||
self._assert_floating_ip_chains(router)
|
self._assert_floating_ip_chains(router)
|
||||||
|
self._assert_iptables_rules_converged(router)
|
||||||
self._assert_extra_routes(router)
|
self._assert_extra_routes(router)
|
||||||
ip_versions = [4, 6] if (ip_version == 6 or dual_stack) else [4]
|
ip_versions = [4, 6] if (ip_version == 6 or dual_stack) else [4]
|
||||||
self._assert_onlink_subnet_routes(router, ip_versions)
|
self._assert_onlink_subnet_routes(router, ip_versions)
|
||||||
|
|
|
@ -1300,7 +1300,7 @@ class TestDeviceManager(base.BaseTestCase):
|
||||||
|
|
||||||
def test_setup_calls_fill_dhcp_udp_checksums(self):
|
def test_setup_calls_fill_dhcp_udp_checksums(self):
|
||||||
self._test_setup_helper(False)
|
self._test_setup_helper(False)
|
||||||
rule = ('-p udp --dport %d -j CHECKSUM --checksum-fill'
|
rule = ('-p udp -m udp --dport %d -j CHECKSUM --checksum-fill'
|
||||||
% const.DHCP_RESPONSE_PORT)
|
% const.DHCP_RESPONSE_PORT)
|
||||||
expected = [mock.call.add_rule('POSTROUTING', rule)]
|
expected = [mock.call.add_rule('POSTROUTING', rule)]
|
||||||
self.mangle_inst.assert_has_calls(expected)
|
self.mangle_inst.assert_has_calls(expected)
|
||||||
|
|
|
@ -34,7 +34,7 @@ class TestMetadataDriverRules(base.BaseTestCase):
|
||||||
|
|
||||||
def test_metadata_nat_rules(self):
|
def test_metadata_nat_rules(self):
|
||||||
rules = ('PREROUTING', '-d 169.254.169.254/32 -i qr-+ '
|
rules = ('PREROUTING', '-d 169.254.169.254/32 -i qr-+ '
|
||||||
'-p tcp -m tcp --dport 80 -j REDIRECT --to-port 8775')
|
'-p tcp -m tcp --dport 80 -j REDIRECT --to-ports 8775')
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
[rules],
|
[rules],
|
||||||
metadata_driver.MetadataDriver.metadata_nat_rules(8775))
|
metadata_driver.MetadataDriver.metadata_nat_rules(8775))
|
||||||
|
|
Loading…
Reference in New Issue