Avoid re-adding iptables rules for instances that have disappeared
The remove_filters_for_instance() method fails silently if the instance's chain is gone (i.e. it's been deleted). If this happens while we're refreshing security group rules, we will not notice this case and re-add stale rules for an old instance, breaking our firewall for new instances. This adds a quick check after we've captured the lock to see if the associated chain exists, and bails if it doesn't. Change-Id: Ic75988939f82de49735d85fe99a9eecd4baf45c9 Related-bug: #1182131
This commit is contained in:
parent
c4080c9760
commit
6aa368b992
|
@ -191,6 +191,12 @@ class IptablesTable(object):
|
||||||
self.remove_chains = set()
|
self.remove_chains = set()
|
||||||
self.dirty = True
|
self.dirty = True
|
||||||
|
|
||||||
|
def has_chain(self, name, wrap=True):
|
||||||
|
if wrap:
|
||||||
|
return name in self.chains
|
||||||
|
else:
|
||||||
|
return name in self.unwrapped_chains
|
||||||
|
|
||||||
def add_chain(self, name, wrap=True):
|
def add_chain(self, name, wrap=True):
|
||||||
"""Adds a named chain to the table.
|
"""Adds a named chain to the table.
|
||||||
|
|
||||||
|
|
|
@ -7728,6 +7728,8 @@ class IptablesFirewallTestCase(test.TestCase):
|
||||||
self.mox.StubOutWithMock(self.fw,
|
self.mox.StubOutWithMock(self.fw,
|
||||||
'add_filters_for_instance',
|
'add_filters_for_instance',
|
||||||
use_mock_anything=True)
|
use_mock_anything=True)
|
||||||
|
self.mox.StubOutWithMock(self.fw.iptables.ipv4['filter'],
|
||||||
|
'has_chain')
|
||||||
|
|
||||||
self.fw.instance_rules(instance_ref,
|
self.fw.instance_rules(instance_ref,
|
||||||
mox.IgnoreArg()).AndReturn((None, None))
|
mox.IgnoreArg()).AndReturn((None, None))
|
||||||
|
@ -7735,6 +7737,8 @@ class IptablesFirewallTestCase(test.TestCase):
|
||||||
mox.IgnoreArg(), mox.IgnoreArg())
|
mox.IgnoreArg(), mox.IgnoreArg())
|
||||||
self.fw.instance_rules(instance_ref,
|
self.fw.instance_rules(instance_ref,
|
||||||
mox.IgnoreArg()).AndReturn((None, None))
|
mox.IgnoreArg()).AndReturn((None, None))
|
||||||
|
self.fw.iptables.ipv4['filter'].has_chain(mox.IgnoreArg()
|
||||||
|
).AndReturn(True)
|
||||||
self.fw.add_filters_for_instance(instance_ref, mox.IgnoreArg(),
|
self.fw.add_filters_for_instance(instance_ref, mox.IgnoreArg(),
|
||||||
mox.IgnoreArg(), mox.IgnoreArg())
|
mox.IgnoreArg(), mox.IgnoreArg())
|
||||||
self.mox.ReplayAll()
|
self.mox.ReplayAll()
|
||||||
|
@ -7743,6 +7747,26 @@ class IptablesFirewallTestCase(test.TestCase):
|
||||||
self.fw.instance_info[instance_ref['id']] = (instance_ref, None)
|
self.fw.instance_info[instance_ref['id']] = (instance_ref, None)
|
||||||
self.fw.do_refresh_security_group_rules("fake")
|
self.fw.do_refresh_security_group_rules("fake")
|
||||||
|
|
||||||
|
def test_do_refresh_security_group_rules_instance_disappeared(self):
|
||||||
|
instance1 = {'id': 1, 'uuid': 'fake-uuid1'}
|
||||||
|
instance2 = {'id': 2, 'uuid': 'fake-uuid2'}
|
||||||
|
self.fw.instance_info = {1: (instance1, 'netinfo1'),
|
||||||
|
2: (instance2, 'netinfo2')}
|
||||||
|
mock_filter = mock.MagicMock()
|
||||||
|
with mock.patch.dict(self.fw.iptables.ipv4, {'filter': mock_filter}):
|
||||||
|
mock_filter.has_chain.return_value = False
|
||||||
|
with mock.patch.object(self.fw, 'instance_rules') as mock_ir:
|
||||||
|
mock_ir.return_value = (None, None)
|
||||||
|
self.fw.do_refresh_security_group_rules('secgroup')
|
||||||
|
self.assertEqual(2, mock_ir.call_count)
|
||||||
|
# NOTE(danms): Make sure that it is checking has_chain each time,
|
||||||
|
# continuing to process all the instances, and never adding the
|
||||||
|
# new chains back if has_chain() is False
|
||||||
|
mock_filter.has_chain.assert_has_calls([mock.call('inst-1'),
|
||||||
|
mock.call('inst-2')],
|
||||||
|
any_order=True)
|
||||||
|
self.assertEqual(0, mock_filter.add_chain.call_count)
|
||||||
|
|
||||||
def test_unfilter_instance_undefines_nwfilter(self):
|
def test_unfilter_instance_undefines_nwfilter(self):
|
||||||
admin_ctxt = context.get_admin_context()
|
admin_ctxt = context.get_admin_context()
|
||||||
|
|
||||||
|
|
|
@ -24,6 +24,7 @@ from nova import objects
|
||||||
from nova.objects import security_group as security_group_obj
|
from nova.objects import security_group as security_group_obj
|
||||||
from nova.objects import security_group_rule as security_group_rule_obj
|
from nova.objects import security_group_rule as security_group_rule_obj
|
||||||
from nova.openstack.common.gettextutils import _
|
from nova.openstack.common.gettextutils import _
|
||||||
|
from nova.openstack.common.gettextutils import _LI
|
||||||
from nova.openstack.common import importutils
|
from nova.openstack.common import importutils
|
||||||
from nova.openstack.common import log as logging
|
from nova.openstack.common import log as logging
|
||||||
from nova import utils
|
from nova import utils
|
||||||
|
@ -442,6 +443,13 @@ class IptablesFirewallDriver(FirewallDriver):
|
||||||
@utils.synchronized('iptables', external=True)
|
@utils.synchronized('iptables', external=True)
|
||||||
def _inner_do_refresh_rules(self, instance, network_info, ipv4_rules,
|
def _inner_do_refresh_rules(self, instance, network_info, ipv4_rules,
|
||||||
ipv6_rules):
|
ipv6_rules):
|
||||||
|
chain_name = self._instance_chain_name(instance)
|
||||||
|
if not self.iptables.ipv4['filter'].has_chain(chain_name):
|
||||||
|
LOG.info(
|
||||||
|
_LI('instance chain %s disappeared during refresh, '
|
||||||
|
'skipping') % chain_name,
|
||||||
|
instance=instance)
|
||||||
|
return
|
||||||
self.remove_filters_for_instance(instance)
|
self.remove_filters_for_instance(instance)
|
||||||
self.add_filters_for_instance(instance, network_info, ipv4_rules,
|
self.add_filters_for_instance(instance, network_info, ipv4_rules,
|
||||||
ipv6_rules)
|
ipv6_rules)
|
||||||
|
|
Loading…
Reference in New Issue