From c4abd6783bd40f39736b62f9fffd95f6af4b8a04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Tue, 8 May 2018 01:13:57 +0200 Subject: [PATCH] PXE Filter dnsmasq: manage macs not in ironic 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 --- ironic_inspector/pxe_filter/dnsmasq.py | 61 ++++++++++--- .../test/unit/test_dnsmasq_pxe_filter.py | 90 ++++++++++++++----- ...-deleted-ironic-macs-4bb766efad8c6d02.yaml | 9 ++ 3 files changed, 124 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/pxe-filter-dnsmasq-manage-deleted-ironic-macs-4bb766efad8c6d02.yaml diff --git a/ironic_inspector/pxe_filter/dnsmasq.py b/ironic_inspector/pxe_filter/dnsmasq.py index cd2527666..5c7c56abd 100644 --- a/ironic_inspector/pxe_filter/dnsmasq.py +++ b/ironic_inspector/pxe_filter/dnsmasq.py @@ -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. diff --git a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py index 17819e666..fdfbda8a0 100644 --- a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py +++ b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py @@ -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)', diff --git a/releasenotes/notes/pxe-filter-dnsmasq-manage-deleted-ironic-macs-4bb766efad8c6d02.yaml b/releasenotes/notes/pxe-filter-dnsmasq-manage-deleted-ironic-macs-4bb766efad8c6d02.yaml new file mode 100644 index 000000000..637a479a9 --- /dev/null +++ b/releasenotes/notes/pxe-filter-dnsmasq-manage-deleted-ironic-macs-4bb766efad8c6d02.yaml @@ -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 `_.