diff --git a/ironic_inspector/conf/iptables.py b/ironic_inspector/conf/iptables.py index 7ecf61388..bb6e80b8d 100644 --- a/ironic_inspector/conf/iptables.py +++ b/ironic_inspector/conf/iptables.py @@ -26,7 +26,7 @@ _OPTS = [ help=_('iptables chain name to use.')), cfg.ListOpt('ethoib_interfaces', default=[], - help=_('List of Etherent Over InfiniBand interfaces ' + help=_('List of Ethernet Over InfiniBand interfaces ' 'on the Inspector host which are used for physical ' 'access to the DHCP network. Multiple interfaces would ' 'be attached to a bond or bridge specified in ' diff --git a/ironic_inspector/conf/pxe_filter.py b/ironic_inspector/conf/pxe_filter.py index 2e1d915ad..07af59c56 100644 --- a/ironic_inspector/conf/pxe_filter.py +++ b/ironic_inspector/conf/pxe_filter.py @@ -24,6 +24,15 @@ _OPTS = [ cfg.IntOpt('sync_period', default=15, min=0, help=_('Amount of time in seconds, after which repeat periodic ' 'update of the filter.')), + cfg.BoolOpt('deny_unknown_macs', default=False, + help=_('By default inspector will open the DHCP server for ' + 'any node when introspection is active. Opening DHCP ' + 'for unknown MAC addresses when introspection is ' + 'active allow for users to add nodes with no ports to ' + 'ironic and have ironic-inspector enroll ports based ' + 'on node introspection results. NOTE: If this option ' + 'is True, nodes must have at least one enrolled port ' + 'prior to introspection.')) ] diff --git a/ironic_inspector/pxe_filter/base.py b/ironic_inspector/pxe_filter/base.py index 437eb9342..b0a11fbb3 100644 --- a/ironic_inspector/pxe_filter/base.py +++ b/ironic_inspector/pxe_filter/base.py @@ -15,6 +15,8 @@ import contextlib import functools +import os +import re from automaton import exceptions as automaton_errors from automaton import machines @@ -27,13 +29,17 @@ import stevedore from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils +from ironic_inspector import node_cache from ironic_inspector.pxe_filter import interface +from ironic_inspector import utils CONF = cfg.CONF LOG = log.getLogger(__name__) _STEVEDORE_DRIVER_NAMESPACE = 'ironic_inspector.pxe_filter' +_EMAC_REGEX = 'EMAC=([0-9a-f]{2}(:[0-9a-f]{2}){5}) IMAC=.*' + class InvalidFilterDriverState(RuntimeError): """The fsm of the filter driver raised an error.""" @@ -96,6 +102,14 @@ class BaseFilter(interface.FilterDriver): def __init__(self): super(BaseFilter, self).__init__() + # Configuration check + if (CONF.pxe_filter.deny_unknown_macs + and CONF.processing.node_not_found_hook): + msg = _('Configuration error: [pxe_filter]deny_unknown_macs is' + 'enabled and [processing]node_not_found_hook is enabled.' + 'These options cannot both be enabled simultaneously.') + raise utils.Error(msg) + self.lock = semaphore.BoundedSemaphore() self.fsm.initialize(start_state=States.uninitialized) @@ -235,3 +249,66 @@ def driver(): :returns: the singleton PXE filter driver object. """ return _driver_manager().driver + + +def _ib_mac_to_rmac_mapping(ports): + """Update port InfiniBand MAC address to EthernetOverInfiniBand MAC + + On InfiniBand deployment we need to map between the baremetal host + InfiniBand MAC to the EoIB MAC. The EoIB MAC addresses are learned + automatically by the EoIB interfaces and those MACs are recorded to the + /sys/class/net//eth/neighs file. The InfiniBand GUID is + taken from the ironic port client-id extra attribute. The InfiniBand GUID + is the last 8 bytes of the client-id. The file format allows to map the + GUID to EoIB MAC. The filter rules based on those MACs get applied during a + driver.update() call + + :param ports: list of ironic ports + :returns: Nothing. + """ + ethoib_interfaces = CONF.iptables.ethoib_interfaces + for iface in ethoib_interfaces: + neighs_file = (os.path.join('/sys/class/net', iface, 'eth/neighs')) + try: + with open(neighs_file, 'r') as fd: + data = fd.read() + except IOError: + LOG.error('Interface %s is not Ethernet Over InfiniBand; ' + 'Skipping ...', iface) + continue + for port in ports: + client_id = port.extra.get('client-id') + if client_id: + # Note(moshele): The last 8 bytes in the client-id is + # the baremetal node InfiniBand GUID + guid = client_id[-23:] + p = re.compile(_EMAC_REGEX + guid) + match = p.search(data) + if match: + port.address = match.group(1) + + +def get_ironic_macs(ironic): + ports = [port for port in + ir_utils.call_with_retries(ironic.ports, limit=None, + fields=['address', 'extra'])] + _ib_mac_to_rmac_mapping(ports) + return {port.address for port in ports} + + +def get_inactive_macs(ironic): + ports = [port for port in + ir_utils.call_with_retries(ironic.ports, limit=None, + fields=['address', 'extra']) + if port.address not in node_cache.active_macs()] + _ib_mac_to_rmac_mapping(ports) + return {port.address for port in ports} + + +def get_active_macs(ironic): + ports = [port for port in + ir_utils.call_with_retries(ironic.ports, limit=None, + fields=['address', 'extra']) + if port.address in node_cache.active_macs()] + _ib_mac_to_rmac_mapping(ports) + return {port.address for port in ports} diff --git a/ironic_inspector/pxe_filter/dnsmasq.py b/ironic_inspector/pxe_filter/dnsmasq.py index 101734c22..0935728ed 100644 --- a/ironic_inspector/pxe_filter/dnsmasq.py +++ b/ironic_inspector/pxe_filter/dnsmasq.py @@ -84,11 +84,11 @@ class DnsmasqFilter(pxe_filter.BaseFilter): timestamp_start = timeutils.utcnow() # active_macs are the MACs for which introspection is active - active_macs = node_cache.active_macs() + active_macs = pxe_filter.get_active_macs(ironic) + # 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.ports, limit=None, - fields=['address'])) + ironic_macs = pxe_filter.get_ironic_macs(ironic) + denylist, allowlist = _get_deny_allow_lists() # removedlist are the MACs that are in either in allow or denylist, # but not kept in ironic (ironic_macs) any more @@ -233,7 +233,8 @@ def _configure_removedlist(macs): hostsdir = CONF.dnsmasq_pxe_filter.dhcp_hostsdir - if _should_enable_unknown_hosts(): + if (_should_enable_unknown_hosts() + and not CONF.pxe_filter.deny_unknown_macs): for mac in macs: if os.stat(os.path.join(hostsdir, mac)).st_size != _MAC_ALLOW_LEN: _add_mac_to_allowlist(mac) @@ -253,7 +254,8 @@ def _configure_unknown_hosts(): path = os.path.join(CONF.dnsmasq_pxe_filter.dhcp_hostsdir, _UNKNOWN_HOSTS_FILE) - if _should_enable_unknown_hosts(): + if (_should_enable_unknown_hosts() + and not CONF.pxe_filter.deny_unknown_macs): wildcard_filter = _ALLOW_UNKNOWN_HOSTS log_wildcard_filter = 'allow' else: diff --git a/ironic_inspector/pxe_filter/iptables.py b/ironic_inspector/pxe_filter/iptables.py index 1d9c5248d..5205882e7 100644 --- a/ironic_inspector/pxe_filter/iptables.py +++ b/ironic_inspector/pxe_filter/iptables.py @@ -12,23 +12,17 @@ # limitations under the License. import contextlib -import os -import re from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log -from ironic_inspector.common import ironic as ir_utils from ironic_inspector import node_cache from ironic_inspector.pxe_filter import base as pxe_filter - CONF = cfg.CONF LOG = log.getLogger(__name__) -_EMAC_REGEX = 'EMAC=([0-9a-f]{2}(:[0-9a-f]{2}){5}) IMAC=.*' - def _should_enable_dhcp(): """Check whether we should enable DHCP at all. @@ -46,6 +40,7 @@ class IptablesFilter(pxe_filter.BaseFilter): def __init__(self): super(IptablesFilter, self).__init__() self.denylist_cache = None + self.allowlist_cache = None self.enabled = True self.interface = CONF.iptables.dnsmasq_interface self.chain = CONF.iptables.firewall_chain @@ -66,6 +61,7 @@ class IptablesFilter(pxe_filter.BaseFilter): def reset(self): self.enabled = True self.denylist_cache = None + self.allowlist_cache = None for chain in (self.chain, self.new_chain): try: self._clean_up(chain) @@ -114,26 +110,44 @@ class IptablesFilter(pxe_filter.BaseFilter): self._disable_dhcp() return - to_deny = _get_denylist(ironic) - if to_deny == self.denylist_cache: - LOG.debug('Not updating iptables - no changes in MAC list %s', - to_deny) + if CONF.pxe_filter.deny_unknown_macs: + to_allow = pxe_filter.get_active_macs(ironic) + to_deny = None + else: + to_deny = pxe_filter.get_inactive_macs(ironic) + to_allow = None + + if to_deny == self.denylist_cache and to_allow == self.allowlist_cache: + LOG.debug('Not updating iptables - no changes in MAC lists %s %s', + to_deny, to_allow) return - LOG.debug('Adding active MAC\'s %s to the deny list', to_deny) + if CONF.pxe_filter.deny_unknown_macs: + LOG.debug('Adding MAC\'s %s to the allow list', to_allow) + else: + LOG.debug('Adding MAC\'s %s to the deny list', to_deny) with self._temporary_chain(self.new_chain, self.chain): # Force update on the next iteration if this attempt fails self.denylist_cache = None - # - Add active macs to the deny list, so that nova can boot them - for mac in to_deny: - self._iptables('-A', self.new_chain, '-m', 'mac', - '--mac-source', mac, '-j', 'DROP') - # - Add everything else to the allow list - self._iptables('-A', self.new_chain, '-j', 'ACCEPT') + self.allowlist_cache = None + # - Add active macs to allow list, so that they are introspected + if CONF.pxe_filter.deny_unknown_macs: + for mac in to_allow: + self._iptables('-A', self.new_chain, '-m', 'mac', + '--mac-source', mac, '-j', 'ACCEPT') + # - Add inactive macs to the deny list, so that nova can boot them + if not CONF.pxe_filter.deny_unknown_macs: + for mac in to_deny: + self._iptables('-A', self.new_chain, '-m', 'mac', + '--mac-source', mac, '-j', 'DROP') + # - Add everything else to the unknown list + policy = 'DROP' if CONF.pxe_filter.deny_unknown_macs else 'ACCEPT' + self._iptables('-A', self.new_chain, '-j', policy) # Cache result of successful iptables update self.enabled = True self.denylist_cache = to_deny + self.allowlist_cache = to_allow LOG.debug('The iptables filter was synchronized') @contextlib.contextmanager @@ -190,50 +204,3 @@ class IptablesFilter(pxe_filter.BaseFilter): # deny everything self._iptables('-A', self.new_chain, '-j', 'REJECT') self.enabled = False - - -def _ib_mac_to_rmac_mapping(ports): - """Update port InfiniBand MAC address to EthernetOverInfiniBand MAC - - On InfiniBand deployment we need to map between the baremetal host - InfiniBand MAC to the EoIB MAC. The EoIB MAC addresses are learned - automatically by the EoIB interfaces and those MACs are recorded to the - /sys/class/net//eth/neighs file. The InfiniBand GUID is - taken from the ironic port client-id extra attribute. The InfiniBand GUID - is the last 8 bytes of the client-id. The file format allows to map the - GUID to EoIB MAC. The filter rules based on those MACs get applied during a - driver.update() call - - :param ports: list of ironic ports - :returns: Nothing. - """ - ethoib_interfaces = CONF.iptables.ethoib_interfaces - for interface in ethoib_interfaces: - neighs_file = ( - os.path.join('/sys/class/net', interface, 'eth/neighs')) - try: - with open(neighs_file, 'r') as fd: - data = fd.read() - except IOError: - LOG.error('Interface %s is not Ethernet Over InfiniBand; ' - 'Skipping ...', interface) - continue - for port in ports: - client_id = port.extra.get('client-id') - if client_id: - # Note(moshele): The last 8 bytes in the client-id is - # the baremetal node InfiniBand GUID - guid = client_id[-23:] - p = re.compile(_EMAC_REGEX + guid) - match = p.search(data) - if match: - port.address = match.group(1) - - -def _get_denylist(ironic): - ports = [port for port in - ir_utils.call_with_retries(ironic.ports, limit=None, - fields=['address', 'extra']) - if port.address not in node_cache.active_macs()] - _ib_mac_to_rmac_mapping(ports) - return [port.address for port in ports] diff --git a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py index c66af8ec7..8bd52d748 100644 --- a/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py +++ b/ironic_inspector/test/unit/test_dnsmasq_pxe_filter.py @@ -26,8 +26,10 @@ from oslo_config import cfg from ironic_inspector.common import ironic as ir_utils from ironic_inspector import node_cache +from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.pxe_filter import dnsmasq from ironic_inspector.test import base as test_base +from ironic_inspector import utils CONF = cfg.CONF @@ -38,6 +40,15 @@ class DnsmasqTestBase(test_base.BaseTest): self.driver = dnsmasq.DnsmasqFilter() +class TestConfiguration(DnsmasqTestBase): + + def test_deny_unknown_macs_and_node_not_found_hook_bad(self): + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + CONF.set_override('node_not_found_hook', True, 'processing') + self.assertRaisesRegex(utils.Error, 'Configuration error:', + self.driver.__init__) + + class TestShouldEnableUnknownHosts(DnsmasqTestBase): def setUp(self): super(TestShouldEnableUnknownHosts, self).setUp() @@ -251,6 +262,23 @@ class TestMACHandlers(test_base.BaseTest): 'A %s record for all unknown hosts using wildcard mac ' 'created', 'deny') + def test__macs_unknown_hosts_deny_unknown(self): + self.mock_join.return_value = "%s/%s" % (self.dhcp_hostsdir, + dnsmasq._UNKNOWN_HOSTS_FILE) + self.mock_introspection_active.return_value = True + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + + dnsmasq._configure_unknown_hosts() + + self.mock_join.assert_called_once_with(self.dhcp_hostsdir, + dnsmasq._UNKNOWN_HOSTS_FILE) + self.mock__exclusive_write_or_pass.assert_called_once_with( + self.mock_join.return_value, + '%s' % dnsmasq._DENY_UNKNOWN_HOSTS) + self.mock_log.debug.assert_called_once_with( + 'A %s record for all unknown hosts using wildcard mac ' + 'created', 'deny') + def test__configure_removedlist_allowlist(self): self.mock_introspection_active.return_value = True self.mock_stat.return_value.st_size = dnsmasq._MAC_DENY_LEN @@ -271,6 +299,17 @@ class TestMACHandlers(test_base.BaseTest): self.mock__exclusive_write_or_pass.assert_called_once_with( self.mock_join.return_value, '%s,ignore\n' % self.mac) + def test__configure_removedlist_denylist_deny_unknown(self): + self.mock_introspection_active.return_value = True + self.mock_stat.return_value.st_size = dnsmasq._MAC_ALLOW_LEN + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + + 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__allowlist_mac(self): dnsmasq._add_mac_to_allowlist(self.mac) @@ -372,6 +411,9 @@ class TestSync(DnsmasqTestBase): get_client_mock = self.useFixture( fixtures.MockPatchObject(ir_utils, 'get_client')).mock get_client_mock.return_value = self.mock_ironic + self.mock_get_active_macs = self.useFixture( + fixtures.MockPatchObject(pxe_filter, 'get_active_macs')).mock + self.mock_get_active_macs.return_value = set() self.mock_active_macs = self.useFixture( fixtures.MockPatchObject(node_cache, 'active_macs')).mock self.ironic_macs = {'new_mac', 'active_mac'} @@ -401,14 +443,15 @@ class TestSync(DnsmasqTestBase): self.mock__configure_unknown_hosts.assert_called_once_with() def test__sync(self): + self.mock_get_active_macs.return_value = {'active_mac'} self.driver._sync(self.mock_ironic) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) self.mock__add_mac_to_allowlist.assert_called_once_with('active_mac') self.mock__add_mac_to_denylist.assert_called_once_with('new_mac') self.mock_ironic.ports.assert_called_once_with( - limit=None, fields=['address']) - self.mock_active_macs.assert_called_once_with() + fields=['address', 'extra'], limit=None) self.mock__get_deny_allow_lists.assert_called_once_with() self.mock__configure_unknown_hosts.assert_called_once_with() self.mock__configure_removedlist.assert_called_once_with({'gone_mac'}) @@ -424,14 +467,15 @@ class TestSync(DnsmasqTestBase): os_exc.SDKException('boom'), [mock.Mock(address=address) for address in self.ironic_macs] ] + self.mock_get_active_macs.return_value = {'active_mac'} self.driver._sync(self.mock_ironic) self.mock__add_mac_to_allowlist.assert_called_once_with('active_mac') self.mock__add_mac_to_denylist.assert_called_once_with('new_mac') - self.mock_ironic.ports.assert_called_with( - limit=None, fields=['address']) - self.mock_active_macs.assert_called_once_with() + self.mock_ironic.ports.assert_called_with(fields=['address', 'extra'], + limit=None) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) self.mock__get_deny_allow_lists.assert_called_once_with() self.mock__configure_removedlist.assert_called_once_with({'gone_mac'}) self.mock_log.debug.assert_has_calls([ diff --git a/ironic_inspector/test/unit/test_iptables.py b/ironic_inspector/test/unit/test_iptables.py index 939811dfb..1fb7f9693 100644 --- a/ironic_inspector/test/unit/test_iptables.py +++ b/ironic_inspector/test/unit/test_iptables.py @@ -16,14 +16,13 @@ from unittest import mock import fixtures -from openstack import exceptions as os_exc from oslo_config import cfg from ironic_inspector import node_cache from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.pxe_filter import iptables from ironic_inspector.test import base as test_base - +from ironic_inspector import utils CONF = cfg.CONF @@ -44,10 +43,14 @@ class TestIptablesDriver(test_base.NodeTest): fixtures.MockPatchObject(self.driver, '_iptables')).mock self.mock_should_enable_dhcp = self.useFixture( fixtures.MockPatchObject(iptables, '_should_enable_dhcp')).mock - self.mock__get_denylist = self.useFixture( - fixtures.MockPatchObject(iptables, '_get_denylist')).mock - self.mock__get_denylist.return_value = [] + self.mock_get_inactive_macs = self.useFixture( + fixtures.MockPatchObject(pxe_filter, 'get_inactive_macs')).mock + self.mock_get_inactive_macs.return_value = set() + self.mock_get_active_macs = self.useFixture( + fixtures.MockPatchObject(pxe_filter, 'get_active_macs')).mock + self.mock_get_active_macs.return_value = set() self.mock_ironic = mock.Mock() + self.mock_ironic.ports.return_value = [] def check_fsm(self, events): # assert the iptables.fsm.process_event() was called with the events @@ -143,7 +146,7 @@ class TestIptablesDriver(test_base.NodeTest): for (args, call) in zip(_iptables_expected_args, call_args_list): self.assertEqual(args, call[0]) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) self.check_fsm([pxe_filter.Events.sync]) def test__iptables_args_ipv4(self): @@ -179,7 +182,8 @@ class TestIptablesDriver(test_base.NodeTest): self.driver = iptables.IptablesFilter() self.mock_iptables = self.useFixture( fixtures.MockPatchObject(self.driver, '_iptables')).mock - self.mock__get_denylist.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_active_macs.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_inactive_macs.return_value = ['FF:EE:DD:CC:BB:AA'] self.mock_should_enable_dhcp.return_value = True _iptables_expected_args = [ @@ -190,7 +194,7 @@ class TestIptablesDriver(test_base.NodeTest): ('-N', self.driver.new_chain), # deny ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', - self.mock__get_denylist.return_value[0], '-j', 'DROP'), + self.mock_get_inactive_macs.return_value[0], '-j', 'DROP'), ('-A', self.driver.new_chain, '-j', 'ACCEPT'), ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', expected_port, '-j', self.driver.new_chain), @@ -208,14 +212,14 @@ class TestIptablesDriver(test_base.NodeTest): for (args, call) in zip(_iptables_expected_args, call_args_list): self.assertEqual(args, call[0]) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) # check caching self.mock_iptables.reset_mock() - self.mock__get_denylist.reset_mock() + self.mock_get_inactive_macs.reset_mock() self.driver.sync(self.mock_ironic) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) self.assertFalse(self.mock_iptables.called) def test_sync_with_denylist_ipv4(self): @@ -226,18 +230,71 @@ class TestIptablesDriver(test_base.NodeTest): CONF.set_override('ip_version', '6', 'iptables') self._test_sync_with_denylist('547') + def _test_sync_with_allowlist(self, expected_port): + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + self.driver = iptables.IptablesFilter() + self.mock_iptables = self.useFixture( + fixtures.MockPatchObject(self.driver, '_iptables')).mock + self.mock_get_active_macs.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_inactive_macs.return_value = ['FF:EE:DD:CC:BB:AA'] + self.mock_should_enable_dhcp.return_value = True + + _iptables_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + expected_port, '-j', self.driver.new_chain), + ('-F', self.driver.new_chain), + ('-X', self.driver.new_chain), + ('-N', self.driver.new_chain), + # deny + ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', + self.mock_get_active_macs.return_value[0], '-j', 'ACCEPT'), + ('-A', self.driver.new_chain, '-j', 'DROP'), + ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + expected_port, '-j', self.driver.new_chain), + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + expected_port, '-j', self.driver.chain), + ('-F', self.driver.chain), + ('-X', self.driver.chain), + ('-E', self.driver.new_chain, self.driver.chain) + ] + + self.driver.sync(self.mock_ironic) + self.check_fsm([pxe_filter.Events.sync]) + call_args_list = self.mock_iptables.call_args_list + + for (args, call) in zip(_iptables_expected_args, + call_args_list): + self.assertEqual(args, call[0]) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) + + # check caching + + self.mock_iptables.reset_mock() + self.mock_get_active_macs.reset_mock() + self.driver.sync(self.mock_ironic) + self.mock_get_active_macs.assert_called_once_with(self.mock_ironic) + self.assertFalse(self.mock_iptables.called) + + def test_sync_with_allowlist_ipv4(self): + CONF.set_override('ip_version', '4', 'iptables') + self._test_sync_with_allowlist('67') + + def test_sync_with_allowlist_ipv6(self): + CONF.set_override('ip_version', '6', 'iptables') + self._test_sync_with_allowlist('547') + def _test__iptables_clean_cache_on_error(self, expected_port): self.driver = iptables.IptablesFilter() self.mock_iptables = self.useFixture( fixtures.MockPatchObject(self.driver, '_iptables')).mock - self.mock__get_denylist.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_get_inactive_macs.return_value = ['AA:BB:CC:DD:EE:FF'] self.mock_should_enable_dhcp.return_value = True self.mock_iptables.side_effect = [None, None, RuntimeError('Oops!'), None, None, None, None, None, None] self.assertRaises(RuntimeError, self.driver.sync, self.mock_ironic) self.check_fsm([pxe_filter.Events.sync, pxe_filter.Events.reset]) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) # check caching syncs_expected_args = [ @@ -249,7 +306,7 @@ class TestIptablesDriver(test_base.NodeTest): ('-N', self.driver.new_chain), # deny ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', - self.mock__get_denylist.return_value[0], '-j', 'DROP'), + self.mock_get_inactive_macs.return_value[0], '-j', 'DROP'), ('-A', self.driver.new_chain, '-j', 'ACCEPT'), ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', expected_port, '-j', self.driver.new_chain), @@ -262,7 +319,7 @@ class TestIptablesDriver(test_base.NodeTest): self.mock_iptables.reset_mock() self.mock_iptables.side_effect = None - self.mock__get_denylist.reset_mock() + self.mock_get_inactive_macs.reset_mock() self.mock_fsm.reset_mock() self.driver.sync(self.mock_ironic) self.check_fsm([pxe_filter.Events.sync]) @@ -271,7 +328,7 @@ class TestIptablesDriver(test_base.NodeTest): for (idx, (args, call)) in enumerate(zip(syncs_expected_args, call_args_list)): self.assertEqual(args, call[0], 'idx: %s' % idx) - self.mock__get_denylist.assert_called_once_with(self.mock_ironic) + self.mock_get_inactive_macs.assert_called_once_with(self.mock_ironic) def test__iptables_clean_cache_on_error_ipv4(self): CONF.set_override('ip_version', '4', 'iptables') @@ -291,6 +348,12 @@ class TestIptablesDriver(test_base.NodeTest): driver = iptables.IptablesFilter() self.assertEqual(driver._cmd_iptables, 'ip6tables') + def test_deny_unknown_macs_and_node_not_found_hook_bad(self): + CONF.set_override('deny_unknown_macs', True, 'pxe_filter') + CONF.set_override('node_not_found_hook', True, 'processing') + self.assertRaisesRegex(utils.Error, 'Configuration error:', + self.driver.__init__) + class Test_ShouldEnableDhcp(test_base.BaseTest): def setUp(self): @@ -311,113 +374,3 @@ class Test_ShouldEnableDhcp(test_base.BaseTest): def test__should_enable_dhcp_false(self): self.mock_introspection_active.return_value = False self.assertIs(False, iptables._should_enable_dhcp()) - - -class TestIBMapping(test_base.BaseTest): - def setUp(self): - super(TestIBMapping, self).setUp() - CONF.set_override('ethoib_interfaces', ['eth0'], 'iptables') - self.ib_data = ( - 'EMAC=02:00:02:97:00:01 IMAC=97:fe:80:00:00:00:00:00:00:7c:fe:90:' - '03:00:29:26:52\n' - 'EMAC=02:00:00:61:00:02 IMAC=61:fe:80:00:00:00:00:00:00:7c:fe:90:' - '03:00:29:24:4f\n' - ) - self.client_id = ('ff:00:00:00:00:00:02:00:00:02:c9:00:7c:fe:90:03:00:' - '29:24:4f') - self.ib_address = '7c:fe:90:29:24:4f' - self.ib_port = mock.Mock(address=self.ib_address, - extra={'client-id': self.client_id}, - spec=['address', 'extra']) - self.port = mock.Mock(address='aa:bb:cc:dd:ee:ff', - extra={}, spec=['address', 'extra']) - self.ports = [self.ib_port, self.port] - self.expected_rmac = '02:00:00:61:00:02' - self.fileobj = mock.mock_open(read_data=self.ib_data) - - def test_matching_ib(self): - with mock.patch('builtins.open', self.fileobj, - create=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.expected_rmac, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', - 'r') - - def test_ib_not_match(self): - self.ports[0].extra['client-id'] = 'foo' - with mock.patch('builtins.open', self.fileobj, - create=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.ib_address, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', - 'r') - - def test_open_no_such_file(self): - with mock.patch('builtins.open', - side_effect=IOError(), autospec=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.ib_address, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', - 'r') - - def test_no_interfaces(self): - CONF.set_override('ethoib_interfaces', [], 'iptables') - with mock.patch('builtins.open', self.fileobj, - create=True) as mock_open: - iptables._ib_mac_to_rmac_mapping(self.ports) - - self.assertEqual(self.ib_address, self.ib_port.address) - self.assertEqual(self.ports, [self.ib_port, self.port]) - mock_open.assert_not_called() - - -class TestGetDenylist(test_base.BaseTest): - def setUp(self): - super(TestGetDenylist, self).setUp() - self.mock__ib_mac_to_rmac_mapping = self.useFixture( - fixtures.MockPatchObject(iptables, '_ib_mac_to_rmac_mapping')).mock - self.mock_active_macs = self.useFixture( - fixtures.MockPatchObject(node_cache, 'active_macs')).mock - self.mock_ironic = mock.Mock() - - def test_active_port(self): - mock_ports_list = [ - mock.Mock(address='foo'), - mock.Mock(address='bar'), - ] - self.mock_ironic.ports.return_value = mock_ports_list - self.mock_active_macs.return_value = {'foo'} - - ports = iptables._get_denylist(self.mock_ironic) - # foo is an active address so we expect the denylist contain only bar - self.assertEqual(['bar'], ports) - self.mock_ironic.ports.assert_called_once_with( - limit=None, fields=['address', 'extra']) - self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( - [mock_ports_list[1]]) - - @mock.patch('time.sleep', lambda _x: None) - def test_retry_on_port_list_failure(self): - mock_ports_list = [ - mock.Mock(address='foo'), - mock.Mock(address='bar'), - ] - self.mock_ironic.ports.side_effect = [ - os_exc.SDKException('boom'), - mock_ports_list - ] - self.mock_active_macs.return_value = {'foo'} - - ports = iptables._get_denylist(self.mock_ironic) - # foo is an active address so we expect the denylist contain only bar - self.assertEqual(['bar'], ports) - self.mock_ironic.ports.assert_called_with( - limit=None, fields=['address', 'extra']) - self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( - [mock_ports_list[1]]) diff --git a/ironic_inspector/test/unit/test_pxe_filter.py b/ironic_inspector/test/unit/test_pxe_filter.py index 150d68b33..8846fcc46 100644 --- a/ironic_inspector/test/unit/test_pxe_filter.py +++ b/ironic_inspector/test/unit/test_pxe_filter.py @@ -17,10 +17,12 @@ from automaton import exceptions as automaton_errors from eventlet import semaphore import fixtures from futurist import periodics +from openstack import exceptions as os_exc from oslo_config import cfg import stevedore from ironic_inspector.common import ironic as ir_utils +from ironic_inspector import node_cache from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.pxe_filter import interface from ironic_inspector.test import base as test_base @@ -292,3 +294,198 @@ class TestDriver(test_base.BaseTest): self.assertIs(self.mock_driver, ret) self.mock__driver_manager.assert_called_once_with() + + +class TestIBMapping(test_base.BaseTest): + def setUp(self): + super(TestIBMapping, self).setUp() + CONF.set_override('ethoib_interfaces', ['eth0'], 'iptables') + self.ib_data = ( + 'EMAC=02:00:02:97:00:01 IMAC=97:fe:80:00:00:00:00:00:00:7c:fe:90:' + '03:00:29:26:52\n' + 'EMAC=02:00:00:61:00:02 IMAC=61:fe:80:00:00:00:00:00:00:7c:fe:90:' + '03:00:29:24:4f\n' + ) + self.client_id = ('ff:00:00:00:00:00:02:00:00:02:c9:00:7c:fe:90:03:00:' + '29:24:4f') + self.ib_address = '7c:fe:90:29:24:4f' + self.ib_port = mock.Mock(address=self.ib_address, + extra={'client-id': self.client_id}, + spec=['address', 'extra']) + self.port = mock.Mock(address='aa:bb:cc:dd:ee:ff', + extra={}, spec=['address', 'extra']) + self.ports = [self.ib_port, self.port] + self.expected_rmac = '02:00:00:61:00:02' + self.fileobj = mock.mock_open(read_data=self.ib_data) + + def test_matching_ib(self): + with mock.patch('builtins.open', self.fileobj, + create=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.expected_rmac, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', + 'r') + + def test_ib_not_match(self): + self.ports[0].extra['client-id'] = 'foo' + with mock.patch('builtins.open', self.fileobj, + create=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.ib_address, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', + 'r') + + def test_open_no_such_file(self): + with mock.patch('builtins.open', + side_effect=IOError(), autospec=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.ib_address, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_called_once_with('/sys/class/net/eth0/eth/neighs', + 'r') + + def test_no_interfaces(self): + CONF.set_override('ethoib_interfaces', [], 'iptables') + with mock.patch('builtins.open', self.fileobj, + create=True) as mock_open: + pxe_filter._ib_mac_to_rmac_mapping(self.ports) + + self.assertEqual(self.ib_address, self.ib_port.address) + self.assertEqual(self.ports, [self.ib_port, self.port]) + mock_open.assert_not_called() + + +class TestGetInactiveMacs(test_base.BaseTest): + def setUp(self): + super(TestGetInactiveMacs, self).setUp() + self.mock__ib_mac_to_rmac_mapping = self.useFixture( + fixtures.MockPatchObject(pxe_filter, + '_ib_mac_to_rmac_mapping')).mock + self.mock_active_macs = self.useFixture( + fixtures.MockPatchObject(node_cache, 'active_macs')).mock + self.mock_ironic = mock.Mock() + + def test_inactive_port(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.return_value = mock_ports_list + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_inactive_macs(self.mock_ironic) + self.assertEqual({'bar'}, ports) + self.mock_ironic.ports.assert_called_once_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[1]]) + + @mock.patch('time.sleep', lambda _x: None) + def test_retry_on_port_list_failure(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.side_effect = [ + os_exc.SDKException('boom'), + mock_ports_list + ] + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_inactive_macs(self.mock_ironic) + self.assertEqual({'bar'}, ports) + self.mock_ironic.ports.assert_called_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[1]]) + + +class TestGetActiveMacs(test_base.BaseTest): + def setUp(self): + super(TestGetActiveMacs, self).setUp() + self.mock__ib_mac_to_rmac_mapping = self.useFixture( + fixtures.MockPatchObject(pxe_filter, + '_ib_mac_to_rmac_mapping')).mock + self.mock_active_macs = self.useFixture( + fixtures.MockPatchObject(node_cache, 'active_macs')).mock + self.mock_ironic = mock.Mock() + + def test_active_port(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.return_value = mock_ports_list + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_active_macs(self.mock_ironic) + self.assertEqual({'foo'}, ports) + self.mock_ironic.ports.assert_called_once_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[0]]) + + @mock.patch('time.sleep', lambda _x: None) + def test_retry_on_port_list_failure(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.side_effect = [ + os_exc.SDKException('boom'), + mock_ports_list + ] + self.mock_active_macs.return_value = {'foo'} + + ports = pxe_filter.get_active_macs(self.mock_ironic) + self.assertEqual({'foo'}, ports) + self.mock_ironic.ports.assert_called_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + [mock_ports_list[0]]) + + +class TestGetIronicMacs(test_base.BaseTest): + def setUp(self): + super(TestGetIronicMacs, self).setUp() + self.mock__ib_mac_to_rmac_mapping = self.useFixture( + fixtures.MockPatchObject(pxe_filter, + '_ib_mac_to_rmac_mapping')).mock + self.mock_ironic = mock.Mock() + + def test_active_port(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.return_value = mock_ports_list + + ports = pxe_filter.get_ironic_macs(self.mock_ironic) + self.assertEqual({'foo', 'bar'}, ports) + self.mock_ironic.ports.assert_called_once_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + mock_ports_list) + + @mock.patch('time.sleep', lambda _x: None) + def test_retry_on_port_list_failure(self): + mock_ports_list = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_ironic.ports.side_effect = [ + os_exc.SDKException('boom'), + mock_ports_list + ] + + ports = pxe_filter.get_ironic_macs(self.mock_ironic) + self.assertEqual({'foo', 'bar'}, ports) + self.mock_ironic.ports.assert_called_with( + limit=None, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with( + mock_ports_list) diff --git a/releasenotes/notes/dnsmasq-pxe-filter-eoib-mac-support-7567bbc7c6bf1878.yaml b/releasenotes/notes/dnsmasq-pxe-filter-eoib-mac-support-7567bbc7c6bf1878.yaml new file mode 100644 index 000000000..9768dfa18 --- /dev/null +++ b/releasenotes/notes/dnsmasq-pxe-filter-eoib-mac-support-7567bbc7c6bf1878.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The dnsmasq pxe-filter now supports mapping between host InfiniBand MAC to + EthernetOverInfiniBand MAC. (This was previously only supported by the + iptables pxe-filter.) diff --git a/releasenotes/notes/pxe-filter-add-deny-unknown-host-option-b84b2aa1f7f49a17.yaml b/releasenotes/notes/pxe-filter-add-deny-unknown-host-option-b84b2aa1f7f49a17.yaml new file mode 100644 index 000000000..265e38b26 --- /dev/null +++ b/releasenotes/notes/pxe-filter-add-deny-unknown-host-option-b84b2aa1f7f49a17.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + By default the DHCP filtering will open the DHCP server for any node when + introspection is active. It will only block DHCP for enrolled nodes that + are not being introspected. Doing so is required to support interface + discovery (which by default will enroll the pxe port to ironic if not + present). This behaviour is not always wanted, as nodes not managed by + ironic may boot the inspection image. + + A new option was added ``[pxe_filter]deny_unknown_macs`` which allow + changeing this behaviour so that the DHCP server only allow enrolled nodes + being introspected and deny everything else. + + .. Note:: If this option is ``True``, nodes must have at least one + enrolled port prior to introspection. +