openstack/ironic-inspector
Code Issues Proposed changes

PXE Filter dnsmasq: manage macs not in ironic

Browse Source 
This changes the dnsmasq PXE filter so that it keeps
macs that are no longer in ironic blacklisted unless
introspection is active or node_not_found_hook is set.

Replacing the previous behaviour that would
exclusively whitelist macs that are no longer in
ironic.

Story: 2001979
Task: 19589
Change-Id: Ib417089116dcbfb25f759708ee3cddcb88ae2111
This commit is contained in:
Harald Jensås 2018-05-08 01:13:57 +02:00
parent 7c5d147fc1
commit c4abd6783b
3 changed files with 124 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
ironic_inspector/pxe_filter/dnsmasq.py
View File

@ -41,6 +41,7 @@ _EXCLUSIVE_WRITE_ATTEMPTS_DELAY = 0.01
_ROOTWRAP_COMMAND = 'sudo ironic-inspector-rootwrap {rootwrap_config!s}'
_MACBL_LEN = len('ff:ff:ff:ff:ff:ff,ignore\n')
_MACWL_LEN = len('ff:ff:ff:ff:ff:ff\n')
_UNKNOWN_HOSTS_FILE = 'unknown_hosts_filter'
_BLACKLIST_UNKNOWN_HOSTS = '*:*:*:*:*:*,ignore\n'
_WHITELIST_UNKNOWN_HOSTS = '*:*:*:*:*:*\n'
@ -77,22 +78,33 @@ class DnsmasqFilter(pxe_filter.BaseFilter):
"""
LOG.debug('Syncing the driver')
timestamp_start = timeutils.utcnow()
# active_macs are the MACs for which introspection is active
active_macs = node_cache.active_macs()
# ironic_macs are all the MACs know to ironic (all ironic ports)
ironic_macs = set(port.address for port in
ir_utils.call_with_retries(ironic.port.list, limit=0,
fields=['address']))
blacklist_macs = _get_blacklist()
# NOTE(milan) whitelist MACs of ports not kept in ironic anymore
# also whitelist active MACs that are still blacklisted in the
# dnsmasq configuration but have just been asked to be introspected
for mac in ((blacklist_macs - ironic_macs) |
(blacklist_macs & active_macs)):
blacklist, whitelist = _get_black_white_lists()
# removedlist are the MACs that are in either blacklist or whitelist,
# but not kept in ironic (ironic_macs) any more
removedlist = blacklist.union(whitelist).difference(ironic_macs)
# Whitelist active MACs that are not already whitelisted
for mac in active_macs.difference(whitelist):
_whitelist_mac(mac)
# blacklist new ports that aren't being inspected
for mac in ironic_macs - (blacklist_macs | active_macs):
# Blacklist any ironic MACs that is not active for introspection unless
# it is already blacklisted
for mac in ironic_macs.difference(blacklist.union(active_macs)):
_blacklist_mac(mac)
# Whitelist or Blacklist unknown hosts and MACs not kept in ironic
# NOTE(hjensas): Treat unknown hosts and MACs not kept in ironic the
# same. Neither should boot the inspection image unless introspection
# is active. Deleted MACs must be whitelisted when introspection is
# active in case the host is re-enrolled.
_configure_unknown_hosts()
_configure_removedlist(removedlist)
timestamp_end = timeutils.utcnow()
LOG.debug('The dnsmasq PXE filter was synchronized (took %s)',
@ -150,7 +162,7 @@ def _purge_dhcp_hostsdir():
LOG.debug('Removed %s', path)
def _get_blacklist():
def _get_black_white_lists():
"""Get addresses currently blacklisted in dnsmasq.
:raises: FileNotFoundError in case the dhcp_hostsdir is invalid.
@ -158,9 +170,15 @@ def _get_blacklist():
"""
hostsdir = CONF.dnsmasq_pxe_filter.dhcp_hostsdir
# whitelisted MACs lack the ,ignore directive
return set(address for address in os.listdir(hostsdir)
if os.stat(os.path.join(hostsdir, address)).st_size ==
_MACBL_LEN)
blacklist = set()
whitelist = set()
for mac in os.listdir(hostsdir):
if os.stat(os.path.join(hostsdir, mac)).st_size == _MACBL_LEN:
blacklist.add(mac)
if os.stat(os.path.join(hostsdir, mac)).st_size == _MACWL_LEN:
whitelist.add(mac)
return blacklist, whitelist
def _exclusive_write_or_pass(path, buf):
@ -202,6 +220,25 @@ def _exclusive_write_or_pass(path, buf):
return False
def _configure_removedlist(macs):
"""Manages a dhcp_hostsdir ignore/not-ignore record for removed macs
:raises: FileNotFoundError in case the dhcp_hostsdir is invalid,
:returns: None.
"""
hostsdir = CONF.dnsmasq_pxe_filter.dhcp_hostsdir
if _should_enable_unknown_hosts():
for mac in macs:
if os.stat(os.path.join(hostsdir, mac)).st_size != _MACWL_LEN:
_whitelist_mac(mac)
else:
for mac in macs:
if os.stat(os.path.join(hostsdir, mac)).st_size != _MACBL_LEN:
_blacklist_mac(mac)
def _configure_unknown_hosts():
"""Manages a dhcp_hostsdir ignore/not-ignore record for unknown macs.

90
ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py
View File

@ -247,6 +247,26 @@ class TestMACHandlers(test_base.BaseTest):
'A %s record for all unknown hosts using wildcard mac '
'created', 'blacklist')
def test__configure_removedlist_whitelist(self):
self.mock_introspection_active.return_value = True
self.mock_stat.return_value.st_size = dnsmasq._MACBL_LEN
dnsmasq._configure_removedlist({self.mac})
self.mock_join.assert_called_with(self.dhcp_hostsdir, self.mac)
self.mock__exclusive_write_or_pass.assert_called_once_with(
self.mock_join.return_value, '%s\n' % self.mac)
def test__configure_removedlist_blacklist(self):
self.mock_introspection_active.return_value = False
self.mock_stat.return_value.st_size = dnsmasq._MACWL_LEN
dnsmasq._configure_removedlist({self.mac})
self.mock_join.assert_called_with(self.dhcp_hostsdir, self.mac)
self.mock__exclusive_write_or_pass.assert_called_once_with(
self.mock_join.return_value, '%s,ignore\n' % self.mac)
def test__whitelist_mac(self):
dnsmasq._whitelist_mac(self.mac)
@ -264,22 +284,42 @@ class TestMACHandlers(test_base.BaseTest):
def test__get_blacklist(self):
self.mock_listdir.return_value = [self.mac]
self.mock_stat.return_value.st_size = len('%s,ignore\n' % self.mac)
ret = dnsmasq._get_blacklist()
blacklist, whitelist = dnsmasq._get_black_white_lists()
self.assertEqual({self.mac}, ret)
self.assertEqual({self.mac}, blacklist)
self.mock_listdir.assert_called_once_with(self.dhcp_hostsdir)
self.mock_join.assert_called_once_with(self.dhcp_hostsdir, self.mac)
self.mock_stat.assert_called_once_with(self.mock_join.return_value)
self.mock_join.assert_called_with(self.dhcp_hostsdir, self.mac)
self.mock_stat.assert_called_with(self.mock_join.return_value)
def test__get_whitelist(self):
self.mock_listdir.return_value = [self.mac]
self.mock_stat.return_value.st_size = len('%s\n' % self.mac)
blacklist, whitelist = dnsmasq._get_black_white_lists()
self.assertEqual({self.mac}, whitelist)
self.mock_listdir.assert_called_once_with(self.dhcp_hostsdir)
self.mock_join.assert_called_with(self.dhcp_hostsdir, self.mac)
self.mock_stat.assert_called_with(self.mock_join.return_value)
def test__get_no_blacklist(self):
self.mock_listdir.return_value = [self.mac]
self.mock_stat.return_value.st_size = len('%s\n' % self.mac)
ret = dnsmasq._get_blacklist()
blacklist, whitelist = dnsmasq._get_black_white_lists()
self.assertEqual(set(), ret)
self.assertEqual(set(), blacklist)
self.mock_listdir.assert_called_once_with(self.dhcp_hostsdir)
self.mock_join.assert_called_once_with(self.dhcp_hostsdir, self.mac)
self.mock_stat.assert_called_once_with(self.mock_join.return_value)
self.mock_join.assert_called_with(self.dhcp_hostsdir, self.mac)
self.mock_stat.assert_called_with(self.mock_join.return_value)
def test__get_no_whitelist(self):
self.mock_listdir.return_value = [self.mac]
self.mock_stat.return_value.st_size = len('%s,ignore\n' % self.mac)
blacklist, whitelist = dnsmasq._get_black_white_lists()
self.assertEqual(set(), whitelist)
self.mock_listdir.assert_called_once_with(self.dhcp_hostsdir)
self.mock_join.assert_called_with(self.dhcp_hostsdir, self.mac)
self.mock_stat.assert_called_with(self.mock_join.return_value)
def test__purge_dhcp_hostsdir(self):
self.mock_listdir.return_value = [self.mac]
@ -304,14 +344,16 @@ class TestMACHandlers(test_base.BaseTest):
class TestSync(DnsmasqTestBase):
def setUp(self):
super(TestSync, self).setUp()
self.mock__get_blacklist = self.useFixture(
fixtures.MockPatchObject(dnsmasq, '_get_blacklist')).mock
self.mock__get_black_white_lists = self.useFixture(
fixtures.MockPatchObject(dnsmasq, '_get_black_white_lists')).mock
self.mock__whitelist_mac = self.useFixture(
fixtures.MockPatchObject(dnsmasq, '_whitelist_mac')).mock
self.mock__blacklist_mac = self.useFixture(
fixtures.MockPatchObject(dnsmasq, '_blacklist_mac')).mock
self.mock__configure_unknown_hosts = self.useFixture(
fixtures.MockPatchObject(dnsmasq, '_configure_unknown_hosts')).mock
self.mock__configure_removedlist = self.useFixture(
fixtures.MockPatchObject(dnsmasq, '_configure_removedlist')).mock
self.mock_ironic = mock.Mock()
self.mock_utcnow = self.useFixture(
@ -330,8 +372,10 @@ class TestSync(DnsmasqTestBase):
fixtures.MockPatchObject(node_cache, 'active_macs')).mock
self.ironic_macs = {'new_mac', 'active_mac'}
self.active_macs = {'active_mac'}
self.blacklist_macs = {'gone_mac', 'active_mac'}
self.mock__get_blacklist.return_value = self.blacklist_macs
self.blacklist = {'gone_mac', 'active_mac'}
self.whitelist = {}
self.mock__get_black_white_lists.return_value = (self.blacklist,
self.whitelist)
self.mock_ironic.port.list.return_value = [
mock.Mock(address=address) for address in self.ironic_macs]
self.mock_active_macs.return_value = self.active_macs
@ -355,16 +399,15 @@ class TestSync(DnsmasqTestBase):
def test__sync(self):
self.driver._sync(self.mock_ironic)
self.mock__whitelist_mac.assert_has_calls([mock.call('active_mac'),
mock.call('gone_mac')],
any_order=True)
self.mock__blacklist_mac.assert_has_calls([mock.call('new_mac')],
any_order=True)
self.mock__whitelist_mac.assert_called_once_with('active_mac')
self.mock__blacklist_mac.assert_called_once_with('new_mac')
self.mock_ironic.port.list.assert_called_once_with(limit=0,
fields=['address'])
self.mock_active_macs.assert_called_once_with()
self.mock__get_blacklist.assert_called_once_with()
self.mock__get_black_white_lists.assert_called_once_with()
self.mock__configure_unknown_hosts.assert_called_once_with()
self.mock__configure_removedlist.assert_called_once_with({'gone_mac'})
self.mock_log.debug.assert_has_calls([
mock.call('Syncing the driver'),
mock.call('The dnsmasq PXE filter was synchronized (took %s)',
@ -379,15 +422,14 @@ class TestSync(DnsmasqTestBase):
]
self.driver._sync(self.mock_ironic)
self.mock__whitelist_mac.assert_has_calls([mock.call('active_mac'),
mock.call('gone_mac')],
any_order=True)
self.mock__blacklist_mac.assert_has_calls([mock.call('new_mac')],
any_order=True)
self.mock__whitelist_mac.assert_called_once_with('active_mac')
self.mock__blacklist_mac.assert_called_once_with('new_mac')
self.mock_ironic.port.list.assert_called_with(limit=0,
fields=['address'])
self.mock_active_macs.assert_called_once_with()
self.mock__get_blacklist.assert_called_once_with()
self.mock__get_black_white_lists.assert_called_once_with()
self.mock__configure_removedlist.assert_called_once_with({'gone_mac'})
self.mock_log.debug.assert_has_calls([
mock.call('Syncing the driver'),
mock.call('The dnsmasq PXE filter was synchronized (took %s)',

9
releasenotes/notes/pxe-filter-dnsmasq-manage-deleted-ironic-macs-4bb766efad8c6d02.yaml Normal file
View File

@ -0,0 +1,9 @@
---
fixes:
- |
The ``dnsmasq`` PXE filter no longer whitelists the MAC addresses of ports
deleted from the Bare Metal service. Instead they are blacklisted unless
introspection is active or the ``node_not_found_hook`` is set in the
configuration. This ensures that no previously enrolled node accidentally
boot the inspection image when no node introspection is active.
`Bug #2001979 <https://storyboard.openstack.org/#!/story/2001979>`_.