diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 707c6127e..6642e3a08 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -320,8 +320,6 @@ the database:: Implementing PXE Filter Drivers ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -.. note:: It is not possible yet to use PXE filter drivers. - Background ---------- diff --git a/doc/source/install/index.rst b/doc/source/install/index.rst index daaaae473..6515cddbe 100644 --- a/doc/source/install/index.rst +++ b/doc/source/install/index.rst @@ -71,7 +71,7 @@ Fill in these minimum configuration values: * ``connection`` in the ``database`` section - SQLAlchemy connection string for the database. -* ``dnsmasq_interface`` in the ``firewall`` section - interface on which +* ``dnsmasq_interface`` in the ``iptables`` section - interface on which ``dnsmasq`` (or another DHCP service) listens for PXE boot requests (defaults to ``br-ctlplane`` which is a sane default for **tripleo**-based installations but is unlikely to work for other cases). @@ -93,7 +93,10 @@ Here is an example *inspector.conf* (adapted from a gate run):: [database] connection = mysql+pymysql://root:@127.0.0.1/ironic_inspector?charset=utf8 - [firewall] + [pxe_filter] + driver=iptables + + [iptables] dnsmasq_interface = br-ctlplane [ironic] diff --git a/doc/source/user/usage.rst b/doc/source/user/usage.rst index 0ebeac6fa..c06d3fbbf 100644 --- a/doc/source/user/usage.rst +++ b/doc/source/user/usage.rst @@ -371,12 +371,12 @@ InfiniBand network interfaces. A recent (Ocata or newer) IPA image is required for that to work. When an InfiniBand network interface is discovered, the **Ironic Inspector** adds a ``client-id`` attribute to the ``extra`` attribute in the ironic port. The **Ironic Inspector** should be configured with -``firewall.ethoib_interfaces`` to indicate the Ethernet Over InfiniBand (EoIB) +``iptables.ethoib_interfaces`` to indicate the Ethernet Over InfiniBand (EoIB) which are used for physical access access to the DHCP network. For example if **Ironic Inspector** DHCP server is using ``br-inspector`` and the ``br-inspector`` has EoIB port e.g. ``eth0``, -the ``firewall.ethoib_interfaces`` should be set to ``eth0``. -The ``firewall.ethoib_interfaces`` allows to map the baremetal GUID to it's +the ``iptables.ethoib_interfaces`` should be set to ``eth0``. +The ``iptables.ethoib_interfaces`` allows to map the baremetal GUID to it's EoIB MAC based on the neighs files. This is needed for blocking DHCP traffic of the nodes (MACs) which are not part of the introspection. diff --git a/doc/source/user/workflow.rst b/doc/source/user/workflow.rst index bbcaade8f..c92ea7b87 100644 --- a/doc/source/user/workflow.rst +++ b/doc/source/user/workflow.rst @@ -18,7 +18,7 @@ Usual hardware introspection flow is as follows: * On receiving node UUID **ironic-inspector**: * validates node power credentials, current power and provisioning states, - * allows firewall access to PXE boot service for the nodes, + * allows access to PXE boot service for the nodes, * issues reboot command for the nodes, so that they boot the ramdisk. * The ramdisk collects the required information and posts it back to diff --git a/example.conf b/example.conf index 90cabf2c3..ea711d9c1 100644 --- a/example.conf +++ b/example.conf @@ -340,23 +340,25 @@ #enroll_node_driver = fake -[firewall] +[iptables] # # From ironic_inspector # -# Whether to manage firewall rules for PXE port. (boolean value) +# DEPRECATED: Whether to manage firewall rules for PXE port. This +# configuration option was deprecated in favor of the ``driver`` +# option in the ``pxe_filter`` section. Please, use the ``noop`` +# filter driver to disable the firewall filtering or the ``iptables`` +# filter driver to enable it. (boolean value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. #manage_firewall = true # Interface on which dnsmasq listens, the default is for VM's. (string # value) #dnsmasq_interface = br-ctlplane -# Amount of time in seconds, after which repeat periodic update of -# firewall. (integer value) -#firewall_update_period = 15 - # iptables chain name to use. (string value) #firewall_chain = ironic-inspector @@ -782,13 +784,13 @@ # From ironic_inspector # -# PXE boot filter driver to use, such as iptables. This option has no -# effect yet. (string value) -#driver = noop +# PXE boot filter driver to use, such as iptables (string value) +#driver = iptables # Amount of time in seconds, after which repeat periodic update of the -# filter. This option has no effect yet. (integer value) +# filter. (integer value) # Minimum value: 0 +# Deprecated group/name - [firewall]/firewall_update_period #sync_period = 15 diff --git a/ironic_inspector/conf.py b/ironic_inspector/conf.py index 854344748..f0e8414a7 100644 --- a/ironic_inspector/conf.py +++ b/ironic_inspector/conf.py @@ -28,22 +28,30 @@ VALID_KEEP_PORTS_VALUES = ('all', 'present', 'added') VALID_STORE_DATA_VALUES = ('none', 'swift') -FIREWALL_OPTS = [ +IPTABLES_OPTS = [ cfg.BoolOpt('manage_firewall', default=True, - help=_('Whether to manage firewall rules for PXE port.')), + # NOTE(milan) this filter driver will be replaced by + # a dnsmasq filter driver + deprecated_for_removal=True, + deprecated_group='firewall', + help=_('Whether to manage firewall rules for PXE port. ' + 'This configuration option was deprecated in favor of ' + 'the ``driver`` option in the ``pxe_filter`` section. ' + 'Please, use the ``noop`` filter driver to disable the ' + 'firewall filtering or the ``iptables`` filter driver ' + 'to enable it.')), cfg.StrOpt('dnsmasq_interface', default='br-ctlplane', + deprecated_group='firewall', help=_('Interface on which dnsmasq listens, the default is for ' 'VM\'s.')), - cfg.IntOpt('firewall_update_period', - default=15, - help=_('Amount of time in seconds, after which repeat periodic ' - 'update of firewall.')), cfg.StrOpt('firewall_chain', default='ironic-inspector', + deprecated_group='firewall', help=_('iptables chain name to use.')), cfg.ListOpt('ethoib_interfaces', + deprecated_group='firewall', default=[], help=_('List of Etherent Over InfiniBand interfaces ' 'on the Inspector host which are used for physical ' @@ -190,17 +198,20 @@ SERVICE_OPTS = [ help=_('Limit the number of elements an API list-call returns')) ] + PXE_FILTER_OPTS = [ - cfg.StrOpt('driver', default='noop', - help=_('PXE boot filter driver to use, such as iptables. ' - 'This option has no effect yet.')), + cfg.StrOpt('driver', default='iptables', + help=_('PXE boot filter driver to use, such as iptables')), cfg.IntOpt('sync_period', default=15, min=0, + deprecated_name='firewall_update_period', + deprecated_group='firewall', help=_('Amount of time in seconds, after which repeat periodic ' - 'update of the filter. This option has no effect yet.')), + 'update of the filter.')), ] + cfg.CONF.register_opts(SERVICE_OPTS) -cfg.CONF.register_opts(FIREWALL_OPTS, group='firewall') +cfg.CONF.register_opts(IPTABLES_OPTS, group='iptables') cfg.CONF.register_opts(PROCESSING_OPTS, group='processing') cfg.CONF.register_opts(PXE_FILTER_OPTS, 'pxe_filter') @@ -208,7 +219,7 @@ cfg.CONF.register_opts(PXE_FILTER_OPTS, 'pxe_filter') def list_opts(): return [ ('', SERVICE_OPTS), - ('firewall', FIREWALL_OPTS), + ('iptables', IPTABLES_OPTS), ('processing', PROCESSING_OPTS), ('pxe_filter', PXE_FILTER_OPTS), ] diff --git a/ironic_inspector/firewall.py b/ironic_inspector/firewall.py deleted file mode 100644 index a94165b28..000000000 --- a/ironic_inspector/firewall.py +++ /dev/null @@ -1,257 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or -# implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import contextlib -import os -import re - -from eventlet.green import subprocess -from eventlet import semaphore -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 - - -CONF = cfg.CONF -LOG = log.getLogger("ironic_inspector.firewall") -NEW_CHAIN = None -CHAIN = None -INTERFACE = None -LOCK = semaphore.BoundedSemaphore() -BASE_COMMAND = None -BLACKLIST_CACHE = None -ENABLED = True -EMAC_REGEX = 'EMAC=([0-9a-f]{2}(:[0-9a-f]{2}){5}) IMAC=.*' - - -def _iptables(*args, **kwargs): - # NOTE(dtantsur): -w flag makes it wait for xtables lock - cmd = BASE_COMMAND + args - ignore = kwargs.pop('ignore', False) - LOG.debug('Running iptables %s', args) - kwargs['stderr'] = subprocess.STDOUT - try: - subprocess.check_output(cmd, **kwargs) - except subprocess.CalledProcessError as exc: - output = exc.output.replace('\n', '. ') - if ignore: - LOG.debug('Ignoring failed iptables %(args)s: %(output)s', - {'args': args, 'output': output}) - else: - LOG.error('iptables %(iptables)s failed: %(exc)s', - {'iptables': args, 'exc': output}) - raise - - -def init(): - """Initialize firewall management. - - Must be called one on start-up. - """ - if not CONF.firewall.manage_firewall: - return - - global INTERFACE, CHAIN, NEW_CHAIN, BASE_COMMAND, BLACKLIST_CACHE - BLACKLIST_CACHE = None - INTERFACE = CONF.firewall.dnsmasq_interface - CHAIN = CONF.firewall.firewall_chain - NEW_CHAIN = CHAIN + '_temp' - BASE_COMMAND = ('sudo', 'ironic-inspector-rootwrap', - CONF.rootwrap_config, 'iptables',) - - # -w flag makes iptables wait for xtables lock, but it's not supported - # everywhere yet - try: - with open(os.devnull, 'wb') as null: - subprocess.check_call(BASE_COMMAND + ('-w', '-h'), - stderr=null, stdout=null) - except subprocess.CalledProcessError: - LOG.warning('iptables does not support -w flag, please update ' - 'it to at least version 1.4.21') - else: - BASE_COMMAND += ('-w',) - - _clean_up(CHAIN) - # Not really needed, but helps to validate that we have access to iptables - _iptables('-N', CHAIN) - - -def _clean_up(chain): - _iptables('-D', 'INPUT', '-i', INTERFACE, '-p', 'udp', - '--dport', '67', '-j', chain, - ignore=True) - _iptables('-F', chain, ignore=True) - _iptables('-X', chain, ignore=True) - - -def clean_up(): - """Clean up everything before exiting.""" - if not CONF.firewall.manage_firewall: - return - - _clean_up(CHAIN) - _clean_up(NEW_CHAIN) - - -def _should_enable_dhcp(): - """Check whether we should enable DHCP at all. - - We won't even open our DHCP if no nodes are on introspection and - node_not_found_hook is not set. - """ - return (node_cache.introspection_active() or - CONF.processing.node_not_found_hook) - - -@contextlib.contextmanager -def _temporary_chain(chain, main_chain): - """Context manager to operate on a temporary chain.""" - # Clean up a bit to account for possible troubles on previous run - _clean_up(chain) - _iptables('-N', chain) - - yield - - # Swap chains - _iptables('-I', 'INPUT', '-i', INTERFACE, '-p', 'udp', - '--dport', '67', '-j', chain) - _iptables('-D', 'INPUT', '-i', INTERFACE, '-p', 'udp', - '--dport', '67', '-j', main_chain, - ignore=True) - _iptables('-F', main_chain, ignore=True) - _iptables('-X', main_chain, ignore=True) - _iptables('-E', chain, main_chain) - - -def _disable_dhcp(): - """Disable DHCP completely.""" - global ENABLED, BLACKLIST_CACHE - - if not ENABLED: - LOG.debug('DHCP is already disabled, not updating') - return - - LOG.debug('No nodes on introspection and node_not_found_hook is ' - 'not set - disabling DHCP') - BLACKLIST_CACHE = None - with _temporary_chain(NEW_CHAIN, CHAIN): - # Blacklist everything - _iptables('-A', NEW_CHAIN, '-j', 'REJECT') - - ENABLED = False - - -def update_filters(ironic=None): - """Update firewall filter rules for introspection. - - Gives access to PXE boot port for any machine, except for those, - whose MAC is registered in Ironic and is not on introspection right now. - - This function is called from both introspection initialization code and - from periodic task. This function is supposed to be resistant to unexpected - iptables state. - - ``init()`` function must be called once before any call to this function. - This function is using ``eventlet`` semaphore to serialize access from - different green threads. - - Does nothing, if firewall management is disabled in configuration. - - :param ironic: Ironic client instance, optional. - """ - global BLACKLIST_CACHE, ENABLED - - if not CONF.firewall.manage_firewall: - return - - assert INTERFACE is not None - ironic = ir_utils.get_client() if ironic is None else ironic - with LOCK: - if not _should_enable_dhcp(): - _disable_dhcp() - return - - ports_active = ironic.port.list(limit=0, fields=['address', 'extra']) - macs_active = set(p.address for p in ports_active) - to_blacklist = macs_active - node_cache.active_macs() - ib_mac_mapping = ( - _ib_mac_to_rmac_mapping(to_blacklist, ports_active)) - - if (BLACKLIST_CACHE is not None and - to_blacklist == BLACKLIST_CACHE and not ib_mac_mapping): - LOG.debug('Not updating iptables - no changes in MAC list %s', - to_blacklist) - return - - LOG.debug('Blacklisting active MAC\'s %s', to_blacklist) - # Force update on the next iteration if this attempt fails - BLACKLIST_CACHE = None - - with _temporary_chain(NEW_CHAIN, CHAIN): - # - Blacklist active macs, so that nova can boot them - for mac in to_blacklist: - mac = ib_mac_mapping.get(mac) or mac - _iptables('-A', NEW_CHAIN, '-m', 'mac', - '--mac-source', mac, '-j', 'DROP') - # - Whitelist everything else - _iptables('-A', NEW_CHAIN, '-j', 'ACCEPT') - - # Cache result of successful iptables update - ENABLED = True - BLACKLIST_CACHE = to_blacklist - - -def _ib_mac_to_rmac_mapping(blacklist_macs, ports_active): - """Mapping between host InfiniBand MAC 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 firewall - rules based on those MACs get applied to the dnsmasq_interface by the - update_filters function. - - :param blacklist_macs: List of InfiniBand baremetal hosts macs to - blacklist. - :param ports_active: list of active ironic ports - :return: baremetal InfiniBand to remote mac on ironic node mapping - """ - ethoib_interfaces = CONF.firewall.ethoib_interfaces - ib_mac_to_remote_mac = {} - 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_active: - if port.address in blacklist_macs: - 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: - ib_mac_to_remote_mac[port.address] = match.group(1) - return ib_mac_to_remote_mac diff --git a/ironic_inspector/introspect.py b/ironic_inspector/introspect.py index d0dac9ce3..418b02c40 100644 --- a/ironic_inspector/introspect.py +++ b/ironic_inspector/introspect.py @@ -20,9 +20,9 @@ from oslo_config import cfg from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils -from ironic_inspector import firewall from ironic_inspector import introspection_state as istate from ironic_inspector import node_cache +from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector import utils CONF = cfg.CONF @@ -97,9 +97,9 @@ def _background_introspect_locked(node_info, ironic): macs = list(node_info.ports()) if macs: node_info.add_attribute(node_cache.MACS_ATTRIBUTE, macs) - LOG.info('Whitelisting MAC\'s %s on the firewall', macs, + LOG.info('Whitelisting MAC\'s %s for a PXE boot', macs, node_info=node_info) - firewall.update_filters(ironic) + pxe_filter.driver().sync(ironic) attrs = node_info.attributes if CONF.processing.node_not_found_hook is None and not attrs: @@ -173,10 +173,10 @@ def _abort(node_info, ironic): # block this node from PXE Booting the introspection image try: - firewall.update_filters(ironic) + pxe_filter.driver().sync(ironic) except Exception as exc: - # Note(mkovacik): this will be retried in firewall update + # Note(mkovacik): this will be retried in the PXE filter sync # periodic task; we continue aborting - LOG.warning('Failed to update firewall filters: %s', exc, + LOG.warning('Failed to sync the PXE filter: %s', exc, node_info=node_info) LOG.info('Introspection aborted', node_info=node_info) diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index f68b72115..ff11a9ae5 100644 --- a/ironic_inspector/process.py +++ b/ironic_inspector/process.py @@ -26,10 +26,10 @@ from oslo_utils import timeutils from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils from ironic_inspector.common import swift -from ironic_inspector import firewall from ironic_inspector import introspection_state as istate from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base +from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector import rules from ironic_inspector import utils @@ -269,7 +269,7 @@ def _process_node(node_info, node, introspection_data): _store_data(node_info, introspection_data) ironic = ir_utils.get_client() - firewall.update_filters(ironic) + pxe_filter.driver().sync(ironic) node_info.invalidate_cache() rules.apply(node_info, introspection_data) diff --git a/ironic_inspector/pxe_filter/iptables.py b/ironic_inspector/pxe_filter/iptables.py new file mode 100644 index 000000000..77fd0ff9c --- /dev/null +++ b/ironic_inspector/pxe_filter/iptables.py @@ -0,0 +1,232 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import contextlib +import os +import re + +from eventlet.green import subprocess +from oslo_config import cfg +from oslo_log import log + +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. + + We won't even open our DHCP if no nodes are on introspection and + node_not_found_hook is not set. + """ + return (node_cache.introspection_active() or + CONF.processing.node_not_found_hook is not None) + + +class IptablesFilter(pxe_filter.BaseFilter): + """A PXE boot filtering interface implementation.""" + + def __init__(self): + super(IptablesFilter, self).__init__() + self.blacklist_cache = None + self.enabled = True + self.interface = CONF.iptables.dnsmasq_interface + self.chain = CONF.iptables.firewall_chain + self.new_chain = self.chain + '_temp' + self.base_command = ('sudo', 'ironic-inspector-rootwrap', + CONF.rootwrap_config, 'iptables') + + def reset(self): + self.enabled = True + self.blacklist_cache = None + for chain in (self.chain, self.new_chain): + try: + self._clean_up(chain) + except Exception as e: + LOG.exception('Encountered exception resetting filter: %s', e) + super(IptablesFilter, self).reset() + + @pxe_filter.locked_driver_event(pxe_filter.Events.initialize) + def init_filter(self): + # -w flag makes iptables wait for xtables lock, but it's not supported + # everywhere yet + try: + with open(os.devnull, 'wb') as null: + subprocess.check_call(self.base_command + ('-w', '-h'), + stderr=null, stdout=null) + except subprocess.CalledProcessError: + LOG.warning('iptables does not support -w flag, please update ' + 'it to at least version 1.4.21') + else: + self.base_command += ('-w',) + + self._clean_up(self.chain) + # Not really needed, but helps to validate that we have access to + # iptables + self._iptables('-N', self.chain) + LOG.debug('The iptables filter was initialized') + + @pxe_filter.locked_driver_event(pxe_filter.Events.sync) + def sync(self, ironic): + """Sync firewall filter rules for introspection. + + Gives access to PXE boot port for any machine, except for those, whose + MAC is registered in Ironic and is not on introspection right now. + + This function is called from both introspection initialization code and + from periodic task. This function is supposed to be resistant to + unexpected iptables state. + + ``init()`` function must be called once before any call to this + function. This function is using ``eventlet`` semaphore to serialize + access from different green threads. + + :param ironic: an ironic client instance. + :returns: nothing. + """ + if not _should_enable_dhcp(): + self._disable_dhcp() + return + + to_blacklist = _get_blacklist(ironic) + if to_blacklist == self.blacklist_cache: + LOG.debug('Not updating iptables - no changes in MAC list %s', + to_blacklist) + return + + LOG.debug('Blacklisting active MAC\'s %s', to_blacklist) + with self._temporary_chain(self.new_chain, self.chain): + # Force update on the next iteration if this attempt fails + self.blacklist_cache = None + # - Blacklist active macs, so that nova can boot them + for mac in to_blacklist: + self._iptables('-A', self.new_chain, '-m', 'mac', + '--mac-source', mac, '-j', 'DROP') + # - Whitelist everything else + self._iptables('-A', self.new_chain, '-j', 'ACCEPT') + + # Cache result of successful iptables update + self.enabled = True + self.blacklist_cache = to_blacklist + LOG.debug('The iptables filter was synchronized') + + @contextlib.contextmanager + def _temporary_chain(self, chain, main_chain): + """Context manager to operate on a temporary chain.""" + # Clean up a bit to account for possible troubles on previous run + self._clean_up(chain) + self._iptables('-N', chain) + + yield + + # Swap chains + self._iptables('-I', 'INPUT', '-i', self.interface, '-p', 'udp', + '--dport', '67', '-j', chain) + self._iptables('-D', 'INPUT', '-i', self.interface, '-p', 'udp', + '--dport', '67', '-j', main_chain, + ignore=True) + self._iptables('-F', main_chain, ignore=True) + self._iptables('-X', main_chain, ignore=True) + self._iptables('-E', chain, main_chain) + + def _iptables(self, *args, **kwargs): + # NOTE(dtantsur): -w flag makes it wait for xtables lock + cmd = self.base_command + args + ignore = kwargs.pop('ignore', False) + LOG.debug('Running iptables %s', args) + kwargs['stderr'] = subprocess.STDOUT + try: + subprocess.check_output(cmd, **kwargs) + except subprocess.CalledProcessError as exc: + output = exc.output.replace('\n', '. ') + if ignore: + LOG.debug('Ignoring failed iptables %(args)s: %(output)s', + {'args': args, 'output': output}) + else: + LOG.error('iptables %(iptables)s failed: %(exc)s', + {'iptables': args, 'exc': output}) + raise + + def _clean_up(self, chain): + self._iptables('-D', 'INPUT', '-i', self.interface, '-p', 'udp', + '--dport', '67', '-j', chain, + ignore=True) + self._iptables('-F', chain, ignore=True) + self._iptables('-X', chain, ignore=True) + + def _disable_dhcp(self): + """Disable DHCP completely.""" + if not self.enabled: + LOG.debug('DHCP is already disabled, not updating') + return + + LOG.debug('No nodes on introspection and node_not_found_hook is ' + 'not set - disabling DHCP') + self.blacklist_cache = None + with self._temporary_chain(self.new_chain, self.chain): + # Blacklist 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_blacklist(ironic): + ports = [ + port.address for port in ironic.port.list( + limit=0, fields=['address', 'extra']) + if port.address not in node_cache.active_macs()] + _ib_mac_to_rmac_mapping(ports) + return ports diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py index 3ce0baab4..821ab11ed 100644 --- a/ironic_inspector/test/functional.py +++ b/ironic_inspector/test/functional.py @@ -53,8 +53,8 @@ os_auth_url = http://url os_username = user os_password = password os_tenant_name = tenant -[firewall] -manage_firewall = False +[pxe_filter] +driver = noop [DEFAULT] debug = True auth_strategy = noauth diff --git a/ironic_inspector/test/unit/test_firewall.py b/ironic_inspector/test/unit/test_firewall.py deleted file mode 100644 index 387068ce3..000000000 --- a/ironic_inspector/test/unit/test_firewall.py +++ /dev/null @@ -1,444 +0,0 @@ -# Copyright 2015 NEC Corporation -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - - -import mock -from oslo_config import cfg - -from ironic_inspector.common import ironic as ir_utils -from ironic_inspector import firewall -from ironic_inspector import introspection_state as istate -from ironic_inspector import node_cache -from ironic_inspector.test import base as test_base - - -CONF = cfg.CONF -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 -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 -""" - - -@mock.patch.object(firewall, '_iptables') -@mock.patch.object(ir_utils, 'get_client') -@mock.patch.object(firewall.subprocess, 'check_call') -class TestFirewall(test_base.NodeTest): - CLIENT_ID = 'ff:00:00:00:00:00:02:00:00:02:c9:00:7c:fe:90:03:00:29:24:4f' - - def test_update_filters_without_manage_firewall(self, mock_call, - mock_get_client, - mock_iptables): - CONF.set_override('manage_firewall', False, 'firewall') - firewall.update_filters() - self.assertEqual(0, mock_iptables.call_count) - - def test_init_args(self, mock_call, mock_get_client, mock_iptables): - rootwrap_path = '/some/fake/path' - CONF.set_override('rootwrap_config', rootwrap_path) - firewall.init() - init_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', '67', - '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain)] - - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(init_expected_args, call_args_list): - self.assertEqual(args, call[0]) - - expected = ('sudo', 'ironic-inspector-rootwrap', rootwrap_path, - 'iptables', '-w') - self.assertEqual(expected, firewall.BASE_COMMAND) - - def test_init_args_old_iptables(self, mock_call, mock_get_client, - mock_iptables): - rootwrap_path = '/some/fake/path' - CONF.set_override('rootwrap_config', rootwrap_path) - mock_call.side_effect = firewall.subprocess.CalledProcessError(2, '') - firewall.init() - init_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', '67', - '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain)] - - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(init_expected_args, call_args_list): - self.assertEqual(args, call[0]) - - expected = ('sudo', 'ironic-inspector-rootwrap', rootwrap_path, - 'iptables',) - self.assertEqual(expected, firewall.BASE_COMMAND) - - def test_init_kwargs(self, mock_call, mock_get_client, mock_iptables): - firewall.init() - init_expected_kwargs = [ - {'ignore': True}, - {'ignore': True}, - {'ignore': True}] - - call_args_list = mock_iptables.call_args_list - - for (kwargs, call) in zip(init_expected_kwargs, call_args_list): - self.assertEqual(kwargs, call[1]) - - def test_update_filters_args(self, mock_call, mock_get_client, - mock_iptables): - # Pretend that we have nodes on introspection - node_cache.add_node(self.node.uuid, state=istate.States.waiting, - bmc_address='1.2.3.4') - - firewall.init() - - update_filters_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-F', firewall.NEW_CHAIN), - ('-X', firewall.NEW_CHAIN), - ('-N', firewall.NEW_CHAIN), - ('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT'), - ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) - ] - - firewall.update_filters() - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(update_filters_expected_args, - call_args_list): - self.assertEqual(args, call[0]) - - def test_update_filters_kwargs(self, mock_call, mock_get_client, - mock_iptables): - firewall.init() - - update_filters_expected_kwargs = [ - {'ignore': True}, - {'ignore': True}, - {'ignore': True}, - {}, - {'ignore': True}, - {'ignore': True}, - {'ignore': True}, - {}, - {}, - {}, - {'ignore': True}, - {'ignore': True}, - {'ignore': True} - ] - - firewall.update_filters() - call_args_list = mock_iptables.call_args_list - - for (kwargs, call) in zip(update_filters_expected_kwargs, - call_args_list): - self.assertEqual(kwargs, call[1]) - - def test_update_filters_with_blacklist(self, mock_call, mock_get_client, - mock_iptables): - active_macs = ['11:22:33:44:55:66', '66:55:44:33:22:11'] - inactive_mac = ['AA:BB:CC:DD:EE:FF'] - self.macs = active_macs + inactive_mac - self.ports = [mock.Mock(address=m) for m in self.macs] - mock_get_client.port.list.return_value = self.ports - node_cache.add_node(self.node.uuid, mac=active_macs, - state=istate.States.finished, - bmc_address='1.2.3.4', foo=None) - firewall.init() - - update_filters_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-F', firewall.NEW_CHAIN), - ('-X', firewall.NEW_CHAIN), - ('-N', firewall.NEW_CHAIN), - # Blacklist - ('-A', firewall.NEW_CHAIN, '-m', 'mac', '--mac-source', - inactive_mac[0], '-j', 'DROP'), - ('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT'), - ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) - ] - - firewall.update_filters(mock_get_client) - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(update_filters_expected_args, - call_args_list): - self.assertEqual(args, call[0]) - - # check caching - - mock_iptables.reset_mock() - firewall.update_filters(mock_get_client) - self.assertFalse(mock_iptables.called) - - def test_update_filters_clean_cache_on_error(self, mock_call, - mock_get_client, - mock_iptables): - active_macs = ['11:22:33:44:55:66', '66:55:44:33:22:11'] - inactive_mac = ['AA:BB:CC:DD:EE:FF'] - self.macs = active_macs + inactive_mac - self.ports = [mock.Mock(address=m) for m in self.macs] - mock_get_client.port.list.return_value = self.ports - node_cache.add_node(self.node.uuid, mac=active_macs, - state=istate.States.finished, - bmc_address='1.2.3.4', foo=None) - firewall.init() - - update_filters_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-F', firewall.NEW_CHAIN), - ('-X', firewall.NEW_CHAIN), - ('-N', firewall.NEW_CHAIN), - # Blacklist - ('-A', firewall.NEW_CHAIN, '-m', 'mac', '--mac-source', - inactive_mac[0], '-j', 'DROP'), - ('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT'), - ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) - ] - - mock_iptables.side_effect = [None, None, RuntimeError()] - self.assertRaises(RuntimeError, firewall.update_filters, - mock_get_client) - - # check caching - - mock_iptables.reset_mock() - mock_iptables.side_effect = None - firewall.update_filters(mock_get_client) - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(update_filters_expected_args, - call_args_list): - self.assertEqual(args, call[0]) - - def test_update_filters_args_node_not_found_hook(self, mock_call, - mock_get_client, - mock_iptables): - # DHCP should be always opened if node_not_found hook is set - CONF.set_override('node_not_found_hook', 'enroll', 'processing') - - firewall.init() - - update_filters_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-F', firewall.NEW_CHAIN), - ('-X', firewall.NEW_CHAIN), - ('-N', firewall.NEW_CHAIN), - ('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT'), - ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) - ] - - firewall.update_filters() - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(update_filters_expected_args, - call_args_list): - self.assertEqual(args, call[0]) - - def test_update_filters_args_no_introspection(self, mock_call, - mock_get_client, - mock_iptables): - firewall.init() - firewall.BLACKLIST_CACHE = ['foo'] - mock_get_client.return_value.port.list.return_value = [ - mock.Mock(address='foobar')] - - update_filters_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-F', firewall.NEW_CHAIN), - ('-X', firewall.NEW_CHAIN), - ('-N', firewall.NEW_CHAIN), - ('-A', firewall.NEW_CHAIN, '-j', 'REJECT'), - ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) - ] - - firewall.update_filters() - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(update_filters_expected_args, - call_args_list): - self.assertEqual(args, call[0]) - - self.assertIsNone(firewall.BLACKLIST_CACHE) - - # Check caching enabled flag - - mock_iptables.reset_mock() - firewall.update_filters() - self.assertFalse(mock_iptables.called) - - # Adding a node changes it back - - node_cache.add_node(self.node.uuid, state=istate.States.starting, - bmc_address='1.2.3.4') - mock_iptables.reset_mock() - firewall.update_filters() - - mock_iptables.assert_any_call('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT') - self.assertEqual({'foobar'}, firewall.BLACKLIST_CACHE) - - def test_update_filters_infiniband( - self, mock_call, mock_get_client, mock_iptables): - - CONF.set_override('ethoib_interfaces', ['eth0'], 'firewall') - active_macs = ['11:22:33:44:55:66', '66:55:44:33:22:11'] - expected_rmac = '02:00:00:61:00:02' - ports = [mock.Mock(address=m) for m in active_macs] - ports.append(mock.Mock(address='7c:fe:90:29:24:4f', - extra={'client-id': self.CLIENT_ID}, - spec=['address', 'extra'])) - mock_get_client.port.list.return_value = ports - node_cache.add_node(self.node.uuid, mac=active_macs, - state=istate.States.finished, - bmc_address='1.2.3.4', foo=None) - firewall.init() - - update_filters_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-F', firewall.NEW_CHAIN), - ('-X', firewall.NEW_CHAIN), - ('-N', firewall.NEW_CHAIN), - # Blacklist - ('-A', firewall.NEW_CHAIN, '-m', 'mac', '--mac-source', - expected_rmac, '-j', 'DROP'), - ('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT'), - ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) - ] - - fileobj = mock.mock_open(read_data=IB_DATA) - with mock.patch('six.moves.builtins.open', fileobj, create=True): - firewall.update_filters(mock_get_client) - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(update_filters_expected_args, - call_args_list): - self.assertEqual(args, call[0]) - - def test_update_filters_infiniband_no_such_file( - self, mock_call, mock_get_client, mock_iptables): - - CONF.set_override('ethoib_interfaces', ['eth0'], 'firewall') - active_macs = ['11:22:33:44:55:66', '66:55:44:33:22:11'] - ports = [mock.Mock(address=m) for m in active_macs] - ports.append(mock.Mock(address='7c:fe:90:29:24:4f', - extra={'client-id': self.CLIENT_ID}, - spec=['address', 'extra'])) - mock_get_client.port.list.return_value = ports - node_cache.add_node(self.node.uuid, mac=active_macs, - state=istate.States.finished, - bmc_address='1.2.3.4', foo=None) - firewall.init() - - update_filters_expected_args = [ - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-N', CONF.firewall.firewall_chain), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-F', firewall.NEW_CHAIN), - ('-X', firewall.NEW_CHAIN), - ('-N', firewall.NEW_CHAIN), - # Blacklist - ('-A', firewall.NEW_CHAIN, '-m', 'mac', '--mac-source', - '7c:fe:90:29:24:4f', '-j', 'DROP'), - ('-A', firewall.NEW_CHAIN, '-j', 'ACCEPT'), - ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', firewall.NEW_CHAIN), - ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', - '67', '-j', CONF.firewall.firewall_chain), - ('-F', CONF.firewall.firewall_chain), - ('-X', CONF.firewall.firewall_chain), - ('-E', firewall.NEW_CHAIN, CONF.firewall.firewall_chain) - ] - - with mock.patch('six.moves.builtins.open', side_effect=IOError()): - firewall.update_filters(mock_get_client) - call_args_list = mock_iptables.call_args_list - - for (args, call) in zip(update_filters_expected_args, - call_args_list): - self.assertEqual(args, call[0]) diff --git a/ironic_inspector/test/unit/test_introspect.py b/ironic_inspector/test/unit/test_introspect.py index 1f21199b9..0dfde8cb1 100644 --- a/ironic_inspector/test/unit/test_introspect.py +++ b/ironic_inspector/test/unit/test_introspect.py @@ -14,14 +14,15 @@ import collections import time +import fixtures from ironicclient import exceptions import mock from oslo_config import cfg from ironic_inspector.common import ironic as ir_utils -from ironic_inspector import firewall from ironic_inspector import introspect from ironic_inspector import node_cache +from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.test import base as test_base from ironic_inspector import utils @@ -39,6 +40,10 @@ class BaseTest(test_base.NodeTest): self.node_info = mock.Mock(uuid=self.uuid, options={}) self.node_info.ports.return_value = self.ports_dict self.node_info.node.return_value = self.node + driver_fixture = self.useFixture(fixtures.MockPatchObject( + pxe_filter, 'driver', autospec=True)) + driver_mock = driver_fixture.mock.return_value + self.sync_filter_mock = driver_mock.sync def _prepare(self, client_mock): cli = client_mock.return_value @@ -47,11 +52,10 @@ class BaseTest(test_base.NodeTest): return cli -@mock.patch.object(firewall, 'update_filters', autospec=True) @mock.patch.object(node_cache, 'start_introspection', autospec=True) @mock.patch.object(ir_utils, 'get_client', autospec=True) class TestIntrospect(BaseTest): - def test_ok(self, client_mock, start_mock, filters_mock): + def test_ok(self, client_mock, start_mock): cli = self._prepare(client_mock) start_mock.return_value = self.node_info @@ -66,7 +70,7 @@ class TestIntrospect(BaseTest): self.node_info.ports.assert_called_once_with() self.node_info.add_attribute.assert_called_once_with('mac', self.macs) - filters_mock.assert_called_with(cli) + self.sync_filter_mock.assert_called_with(cli) cli.node.set_boot_device.assert_called_once_with(self.uuid, 'pxe', persistent=False) @@ -75,7 +79,7 @@ class TestIntrospect(BaseTest): self.node_info.acquire_lock.assert_called_once_with() self.node_info.release_lock.assert_called_once_with() - def test_loopback_bmc_address(self, client_mock, start_mock, filters_mock): + def test_loopback_bmc_address(self, client_mock, start_mock): self.node.driver_info['ipmi_address'] = '127.0.0.1' cli = self._prepare(client_mock) start_mock.return_value = self.node_info @@ -91,7 +95,7 @@ class TestIntrospect(BaseTest): self.node_info.ports.assert_called_once_with() self.node_info.add_attribute.assert_called_once_with('mac', self.macs) - filters_mock.assert_called_with(cli) + self.sync_filter_mock.assert_called_with(cli) cli.node.set_boot_device.assert_called_once_with(self.uuid, 'pxe', persistent=False) @@ -100,7 +104,7 @@ class TestIntrospect(BaseTest): self.node_info.acquire_lock.assert_called_once_with() self.node_info.release_lock.assert_called_once_with() - def test_ok_ilo_and_drac(self, client_mock, start_mock, filters_mock): + def test_ok_ilo_and_drac(self, client_mock, start_mock): cli = self._prepare(client_mock) start_mock.return_value = self.node_info @@ -112,7 +116,7 @@ class TestIntrospect(BaseTest): bmc_address=self.bmc_address, ironic=cli) - def test_power_failure(self, client_mock, start_mock, filters_mock): + def test_power_failure(self, client_mock, start_mock): cli = self._prepare(client_mock) cli.node.set_boot_device.side_effect = exceptions.BadRequest() cli.node.set_power_state.side_effect = exceptions.BadRequest() @@ -135,10 +139,10 @@ class TestIntrospect(BaseTest): self.node_info.acquire_lock.assert_called_once_with() self.node_info.release_lock.assert_called_once_with() - def test_unexpected_error(self, client_mock, start_mock, filters_mock): + def test_unexpected_error(self, client_mock, start_mock): cli = self._prepare(client_mock) start_mock.return_value = self.node_info - filters_mock.side_effect = RuntimeError() + self.sync_filter_mock.side_effect = RuntimeError() introspect.introspect(self.node.uuid) @@ -153,7 +157,7 @@ class TestIntrospect(BaseTest): self.node_info.acquire_lock.assert_called_once_with() self.node_info.release_lock.assert_called_once_with() - def test_no_macs(self, client_mock, start_mock, filters_mock): + def test_no_macs(self, client_mock, start_mock): cli = self._prepare(client_mock) self.node_info.ports.return_value = [] start_mock.return_value = self.node_info @@ -166,14 +170,14 @@ class TestIntrospect(BaseTest): bmc_address=self.bmc_address, ironic=cli) self.assertFalse(self.node_info.add_attribute.called) - self.assertFalse(filters_mock.called) + self.assertFalse(self.sync_filter_mock.called) cli.node.set_boot_device.assert_called_once_with(self.uuid, 'pxe', persistent=False) cli.node.set_power_state.assert_called_once_with(self.uuid, 'reboot') - def test_no_lookup_attrs(self, client_mock, start_mock, filters_mock): + def test_no_lookup_attrs(self, client_mock, start_mock): cli = self._prepare(client_mock) self.node_info.ports.return_value = [] start_mock.return_value = self.node_info @@ -183,14 +187,13 @@ class TestIntrospect(BaseTest): self.node_info.ports.assert_called_once_with() self.node_info.finished.assert_called_once_with(error=mock.ANY) - self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, self.sync_filter_mock.call_count) self.assertEqual(0, cli.node.set_power_state.call_count) self.node_info.acquire_lock.assert_called_once_with() self.node_info.release_lock.assert_called_once_with() def test_no_lookup_attrs_with_node_not_found_hook(self, client_mock, - start_mock, - filters_mock): + start_mock): CONF.set_override('node_not_found_hook', 'example', 'processing') cli = self._prepare(client_mock) self.node_info.ports.return_value = [] @@ -207,7 +210,7 @@ class TestIntrospect(BaseTest): cli.node.set_power_state.assert_called_once_with(self.uuid, 'reboot') - def test_failed_to_get_node(self, client_mock, start_mock, filters_mock): + def test_failed_to_get_node(self, client_mock, start_mock): cli = client_mock.return_value cli.node.get.side_effect = exceptions.NotFound() self.assertRaisesRegex(utils.Error, @@ -220,13 +223,12 @@ class TestIntrospect(BaseTest): introspect.introspect, self.uuid) self.assertEqual(0, self.node_info.ports.call_count) - self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, self.sync_filter_mock.call_count) self.assertEqual(0, cli.node.set_power_state.call_count) self.assertFalse(start_mock.called) self.assertFalse(self.node_info.acquire_lock.called) - def test_failed_to_validate_node(self, client_mock, start_mock, - filters_mock): + def test_failed_to_validate_node(self, client_mock, start_mock): cli = client_mock.return_value cli.node.get.return_value = self.node cli.node.validate.return_value = mock.Mock(power={'result': False, @@ -239,13 +241,12 @@ class TestIntrospect(BaseTest): cli.node.validate.assert_called_once_with(self.uuid) self.assertEqual(0, self.node_info.ports.call_count) - self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, self.sync_filter_mock.call_count) self.assertEqual(0, cli.node.set_power_state.call_count) self.assertFalse(start_mock.called) self.assertFalse(self.node_info.acquire_lock.called) - def test_wrong_provision_state(self, client_mock, start_mock, - filters_mock): + def test_wrong_provision_state(self, client_mock, start_mock): self.node.provision_state = 'active' cli = client_mock.return_value cli.node.get.return_value = self.node @@ -255,14 +256,13 @@ class TestIntrospect(BaseTest): introspect.introspect, self.uuid) self.assertEqual(0, self.node_info.ports.call_count) - self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, self.sync_filter_mock.call_count) self.assertEqual(0, cli.node.set_power_state.call_count) self.assertFalse(start_mock.called) self.assertFalse(self.node_info.acquire_lock.called) @mock.patch.object(time, 'time') - def test_introspection_delay(self, time_mock, client_mock, - start_mock, filters_mock): + def test_introspection_delay(self, time_mock, client_mock, start_mock): time_mock.return_value = 42 introspect._LAST_INTROSPECTION_TIME = 40 CONF.set_override('introspection_delay', 10) @@ -282,9 +282,8 @@ class TestIntrospect(BaseTest): self.assertEqual(42, introspect._LAST_INTROSPECTION_TIME) @mock.patch.object(time, 'time') - def test_introspection_delay_not_needed( - self, time_mock, client_mock, - start_mock, filters_mock): + def test_introspection_delay_not_needed(self, time_mock, client_mock, + start_mock): time_mock.return_value = 100 introspect._LAST_INTROSPECTION_TIME = 40 @@ -305,7 +304,6 @@ class TestIntrospect(BaseTest): self.assertEqual(100, introspect._LAST_INTROSPECTION_TIME) -@mock.patch.object(firewall, 'update_filters', autospec=True) @mock.patch.object(node_cache, 'get_node', autospec=True) @mock.patch.object(ir_utils, 'get_client', autospec=True) class TestAbort(BaseTest): @@ -314,7 +312,7 @@ class TestAbort(BaseTest): self.node_info.started_at = None self.node_info.finished_at = None - def test_ok(self, client_mock, get_mock, filters_mock): + def test_ok(self, client_mock, get_mock): cli = self._prepare(client_mock) get_mock.return_value = self.node_info self.node_info.acquire_lock.return_value = True @@ -326,12 +324,12 @@ class TestAbort(BaseTest): get_mock.assert_called_once_with(self.uuid, ironic=cli, locked=False) self.node_info.acquire_lock.assert_called_once_with(blocking=False) - filters_mock.assert_called_once_with(cli) + self.sync_filter_mock.assert_called_once_with(cli) cli.node.set_power_state.assert_called_once_with(self.uuid, 'off') self.node_info.finished.assert_called_once_with(error='Canceled ' 'by operator') - def test_node_not_found(self, client_mock, get_mock, filters_mock): + def test_node_not_found(self, client_mock, get_mock): cli = self._prepare(client_mock) exc = utils.Error('Not found.', code=404) get_mock.side_effect = exc @@ -339,11 +337,11 @@ class TestAbort(BaseTest): self.assertRaisesRegex(utils.Error, str(exc), introspect.abort, self.uuid) - self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, self.sync_filter_mock.call_count) self.assertEqual(0, cli.node.set_power_state.call_count) self.assertEqual(0, self.node_info.finished.call_count) - def test_node_locked(self, client_mock, get_mock, filters_mock): + def test_node_locked(self, client_mock, get_mock): cli = self._prepare(client_mock) get_mock.return_value = self.node_info self.node_info.acquire_lock.return_value = False @@ -352,12 +350,11 @@ class TestAbort(BaseTest): self.assertRaisesRegex(utils.Error, 'Node is locked, please, ' 'retry later', introspect.abort, self.uuid) - self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, self.sync_filter_mock.call_count) self.assertEqual(0, cli.node.set_power_state.call_count) self.assertEqual(0, self.node_info.finshed.call_count) - def test_introspection_already_finished(self, client_mock, - get_mock, filters_mock): + def test_introspection_already_finished(self, client_mock, get_mock): cli = self._prepare(client_mock) get_mock.return_value = self.node_info self.node_info.acquire_lock.return_value = True @@ -366,31 +363,29 @@ class TestAbort(BaseTest): introspect.abort(self.uuid) - self.assertEqual(0, filters_mock.call_count) + self.assertEqual(0, self.sync_filter_mock.call_count) self.assertEqual(0, cli.node.set_power_state.call_count) self.assertEqual(0, self.node_info.finshed.call_count) - def test_firewall_update_exception(self, client_mock, get_mock, - filters_mock): + def test_firewall_update_exception(self, client_mock, get_mock): cli = self._prepare(client_mock) get_mock.return_value = self.node_info self.node_info.acquire_lock.return_value = True self.node_info.started_at = time.time() self.node_info.finished_at = None - filters_mock.side_effect = Exception('Boom') + self.sync_filter_mock.side_effect = Exception('Boom') introspect.abort(self.uuid) get_mock.assert_called_once_with(self.uuid, ironic=cli, locked=False) self.node_info.acquire_lock.assert_called_once_with(blocking=False) - filters_mock.assert_called_once_with(cli) + self.sync_filter_mock.assert_called_once_with(cli) cli.node.set_power_state.assert_called_once_with(self.uuid, 'off') self.node_info.finished.assert_called_once_with(error='Canceled ' 'by operator') - def test_node_power_off_exception(self, client_mock, get_mock, - filters_mock): + def test_node_power_off_exception(self, client_mock, get_mock): cli = self._prepare(client_mock) get_mock.return_value = self.node_info self.node_info.acquire_lock.return_value = True @@ -403,7 +398,7 @@ class TestAbort(BaseTest): get_mock.assert_called_once_with(self.uuid, ironic=cli, locked=False) self.node_info.acquire_lock.assert_called_once_with(blocking=False) - filters_mock.assert_called_once_with(cli) + self.sync_filter_mock.assert_called_once_with(cli) cli.node.set_power_state.assert_called_once_with(self.uuid, 'off') self.node_info.finished.assert_called_once_with(error='Canceled ' 'by operator') diff --git a/ironic_inspector/test/unit/test_iptables.py b/ironic_inspector/test/unit/test_iptables.py new file mode 100644 index 000000000..8b334e875 --- /dev/null +++ b/ironic_inspector/test/unit/test_iptables.py @@ -0,0 +1,356 @@ +# Copyright 2015 NEC Corporation +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import fixtures +import mock +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 + + +CONF = cfg.CONF + + +class TestIptablesDriver(test_base.NodeTest): + + def setUp(self): + super(TestIptablesDriver, self).setUp() + CONF.set_override('rootwrap_config', '/some/fake/path') + # NOTE(milan) we ignore the state checking in order to avoid having to + # always call e.g self.driver.init_filter() to set proper driver state + self.mock_fsm = self.useFixture( + fixtures.MockPatchObject(iptables.IptablesFilter, 'fsm')).mock + self.mock_call = self.useFixture( + fixtures.MockPatchObject(iptables.subprocess, 'check_call')).mock + self.driver = iptables.IptablesFilter() + self.mock_iptables = self.useFixture( + fixtures.MockPatchObject(self.driver, '_iptables')).mock + self.mock_should_enable_dhcp = self.useFixture( + fixtures.MockPatchObject(iptables, '_should_enable_dhcp')).mock + self.mock__get_blacklist = self.useFixture( + fixtures.MockPatchObject(iptables, '_get_blacklist')).mock + self.mock__get_blacklist.return_value = [] + self.mock_ironic = mock.Mock() + + def check_fsm(self, events): + # assert the iptables.fsm.process_event() was called with the events + calls = [mock.call(event) for event in events] + self.assertEqual(calls, self.driver.fsm.process_event.call_args_list) + + def test_init_args(self): + self.driver.init_filter() + init_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', '67', + '-j', self.driver.chain), + ('-F', self.driver.chain), + ('-X', self.driver.chain), + ('-N', self.driver.chain)] + + call_args_list = self.mock_iptables.call_args_list + + for (args, call) in zip(init_expected_args, call_args_list): + self.assertEqual(args, call[0]) + + expected = ('sudo', 'ironic-inspector-rootwrap', CONF.rootwrap_config, + 'iptables', '-w') + self.assertEqual(expected, self.driver.base_command) + self.check_fsm([pxe_filter.Events.initialize]) + + def test_init_args_old_iptables(self): + self.mock_call.side_effect = iptables.subprocess.CalledProcessError( + 2, '') + self.driver.init_filter() + init_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', '67', + '-j', self.driver.chain), + ('-F', self.driver.chain), + ('-X', self.driver.chain), + ('-N', self.driver.chain)] + + call_args_list = self.mock_iptables.call_args_list + + for (args, call) in zip(init_expected_args, call_args_list): + self.assertEqual(args, call[0]) + + expected = ('sudo', 'ironic-inspector-rootwrap', CONF.rootwrap_config, + 'iptables',) + self.assertEqual(expected, self.driver.base_command) + self.check_fsm([pxe_filter.Events.initialize]) + + def test_init_kwargs(self): + self.driver.init_filter() + init_expected_kwargs = [ + {'ignore': True}, + {'ignore': True}, + {'ignore': True}] + + call_args_list = self.mock_iptables.call_args_list + + for (kwargs, call) in zip(init_expected_kwargs, call_args_list): + self.assertEqual(kwargs, call[1]) + self.check_fsm([pxe_filter.Events.initialize]) + + def test_init_fails(self): + class MyError(Exception): + pass + + self.mock_call.side_effect = MyError('Oops!') + self.assertRaisesRegex(MyError, 'Oops!', self.driver.init_filter) + self.check_fsm([pxe_filter.Events.initialize, pxe_filter.Events.reset]) + + def test__iptables_args(self): + self.mock_should_enable_dhcp.return_value = True + + _iptables_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', self.driver.new_chain), + ('-F', self.driver.new_chain), + ('-X', self.driver.new_chain), + ('-N', self.driver.new_chain), + ('-A', self.driver.new_chain, '-j', 'ACCEPT'), + ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', self.driver.new_chain), + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-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) + 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_blacklist.assert_called_once_with(self.mock_ironic) + self.check_fsm([pxe_filter.Events.sync]) + + def test__iptables_kwargs(self): + _iptables_expected_kwargs = [ + {'ignore': True}, + {'ignore': True}, + {'ignore': True}, + {}, + {}, + {}, + {'ignore': True}, + {'ignore': True}, + {'ignore': True} + ] + + self.driver.sync(self.mock_ironic) + call_args_list = self.mock_iptables.call_args_list + + for (kwargs, call) in zip(_iptables_expected_kwargs, + call_args_list): + self.assertEqual(kwargs, call[1]) + self.check_fsm([pxe_filter.Events.sync]) + + def test_sync_with_blacklist(self): + self.mock__get_blacklist.return_value = ['AA:BB:CC:DD:EE:FF'] + self.mock_should_enable_dhcp.return_value = True + + _iptables_expected_args = [ + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', self.driver.new_chain), + ('-F', self.driver.new_chain), + ('-X', self.driver.new_chain), + ('-N', self.driver.new_chain), + # Blacklist + ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', + self.mock__get_blacklist.return_value[0], '-j', 'DROP'), + ('-A', self.driver.new_chain, '-j', 'ACCEPT'), + ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', self.driver.new_chain), + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-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_blacklist.assert_called_once_with(self.mock_ironic) + + # check caching + + self.mock_iptables.reset_mock() + self.mock__get_blacklist.reset_mock() + self.driver.sync(self.mock_ironic) + self.mock__get_blacklist.assert_called_once_with(self.mock_ironic) + self.assertFalse(self.mock_iptables.called) + + def test__iptables_clean_cache_on_error(self): + self.mock__get_blacklist.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_blacklist.assert_called_once_with(self.mock_ironic) + + # check caching + syncs_expected_args = [ + # driver reset + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', self.driver.new_chain), + ('-F', self.driver.new_chain), + ('-X', self.driver.new_chain), + ('-N', self.driver.new_chain), + # Blacklist + ('-A', self.driver.new_chain, '-m', 'mac', '--mac-source', + self.mock__get_blacklist.return_value[0], '-j', 'DROP'), + ('-A', self.driver.new_chain, '-j', 'ACCEPT'), + ('-I', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', self.driver.new_chain), + ('-D', 'INPUT', '-i', 'br-ctlplane', '-p', 'udp', '--dport', + '67', '-j', self.driver.chain), + ('-F', self.driver.chain), + ('-X', self.driver.chain), + ('-E', self.driver.new_chain, self.driver.chain) + ] + + self.mock_iptables.reset_mock() + self.mock_iptables.side_effect = None + self.mock__get_blacklist.reset_mock() + self.mock_fsm.reset_mock() + self.driver.sync(self.mock_ironic) + self.check_fsm([pxe_filter.Events.sync]) + call_args_list = self.mock_iptables.call_args_list + + for (idx, (args, call)) in enumerate(zip(syncs_expected_args, + call_args_list)): + self.assertEqual(args, call[0], 'idx: %s' % idx) + self.mock__get_blacklist.assert_called_once_with(self.mock_ironic) + + +class Test_ShouldEnableDhcp(test_base.BaseTest): + def setUp(self): + super(Test_ShouldEnableDhcp, self).setUp() + self.mock_introspection_active = self.useFixture( + fixtures.MockPatchObject(node_cache, 'introspection_active')).mock + + def test_introspection_active(self): + self.mock_introspection_active.return_value = True + self.assertIs(True, iptables._should_enable_dhcp()) + + def test_node_not_found_hook_set(self): + # DHCP should be always opened if node_not_found hook is set + CONF.set_override('node_not_found_hook', 'enroll', 'processing') + self.mock_introspection_active.return_value = False + self.assertIs(True, iptables._should_enable_dhcp()) + + 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('six.moves.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('six.moves.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('six.moves.builtins.open', + side_effect=IOError()) 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('six.moves.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 TestGetBlacklist(test_base.BaseTest): + def setUp(self): + super(TestGetBlacklist, 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): + self.mock_ironic.port.list.return_value = [ + mock.Mock(address='foo'), + mock.Mock(address='bar'), + ] + self.mock_active_macs.return_value = {'foo'} + + ports = iptables._get_blacklist(self.mock_ironic) + # foo is an active address so we expect the blacklist contains only bar + self.assertEqual(['bar'], ports) + self.mock_ironic.port.list.assert_called_once_with( + limit=0, fields=['address', 'extra']) + self.mock__ib_mac_to_rmac_mapping.assert_called_once_with(ports) diff --git a/ironic_inspector/test/unit/test_process.py b/ironic_inspector/test/unit/test_process.py index b983a7640..f7ec1e558 100644 --- a/ironic_inspector/test/unit/test_process.py +++ b/ironic_inspector/test/unit/test_process.py @@ -29,12 +29,12 @@ from oslo_utils import uuidutils from ironic_inspector.common import ironic as ir_utils from ironic_inspector import db -from ironic_inspector import firewall from ironic_inspector import introspection_state as istate from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base from ironic_inspector.plugins import example as example_plugin from ironic_inspector import process +from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.test import base as test_base from ironic_inspector import utils @@ -383,7 +383,7 @@ class TestProcessNode(BaseTest): self.cli.node.list_ports.return_value = [] self.useFixture(fixtures.MockPatchObject( - firewall, 'update_filters', autospec=True)) + pxe_filter, 'driver', autospec=True)) self.useFixture(fixtures.MockPatchObject( eventlet.greenthread, 'sleep', autospec=True)) diff --git a/ironic_inspector/test/unit/test_pxe_filter.py b/ironic_inspector/test/unit/test_pxe_filter.py index bf9866b9a..7ed1a8566 100644 --- a/ironic_inspector/test/unit/test_pxe_filter.py +++ b/ironic_inspector/test/unit/test_pxe_filter.py @@ -41,7 +41,7 @@ class TestDriverManager(test_base.BaseTest): driver_manager = pxe_filter._driver_manager() self.stevedore_driver_mock.assert_called_once_with( pxe_filter._STEVEDORE_DRIVER_NAMESPACE, - name='noop', + name='iptables', invoke_on_load=True ) self.assertIsNotNone(driver_manager) diff --git a/ironic_inspector/test/unit/test_wsgi_service.py b/ironic_inspector/test/unit/test_wsgi_service.py index e99b5bbb1..6c95a6c84 100644 --- a/ironic_inspector/test/unit/test_wsgi_service.py +++ b/ironic_inspector/test/unit/test_wsgi_service.py @@ -89,8 +89,8 @@ class TestWSGIServiceInitHost(BaseWSGITest): self.mock_validate_processing_hooks = self.useFixture( fixtures.MockPatchObject(wsgi_service.plugins_base, 'validate_processing_hooks')).mock - self.mock_firewall_init = self.useFixture(fixtures.MockPatchObject( - wsgi_service.firewall, 'init')).mock + self.mock_filter = self.useFixture(fixtures.MockPatchObject( + wsgi_service.pxe_filter, 'driver')).mock.return_value self.mock_periodic = self.useFixture(fixtures.MockPatchObject( wsgi_service.periodics, 'periodic')).mock self.mock_PeriodicWorker = self.useFixture(fixtures.MockPatchObject( @@ -102,35 +102,25 @@ class TestWSGIServiceInitHost(BaseWSGITest): self.mock_exit = self.useFixture(fixtures.MockPatchObject( wsgi_service.sys, 'exit')).mock - # 'positive' settings - CONF.set_override('manage_firewall', True, 'firewall') - def assert_periodics(self): - outer_update_decorator_call = mock.call( - spacing=CONF.firewall.firewall_update_period, - enabled=CONF.firewall.manage_firewall) outer_cleanup_decorator_call = mock.call( spacing=CONF.clean_up_period) self.mock_periodic.assert_has_calls([ - outer_update_decorator_call, - mock.call()(wsgi_service.firewall.update_filters), outer_cleanup_decorator_call, mock.call()(wsgi_service.periodic_clean_up)]) inner_decorator = self.mock_periodic.return_value - inner_update_decorator_call = mock.call( - wsgi_service.firewall.update_filters) inner_cleanup_decorator_call = mock.call( wsgi_service.periodic_clean_up) - inner_decorator.assert_has_calls([inner_update_decorator_call, - inner_cleanup_decorator_call]) + inner_decorator.assert_has_calls([inner_cleanup_decorator_call]) self.mock_ExistingExecutor.assert_called_once_with( self.mock_executor.return_value) periodic_worker = self.mock_PeriodicWorker.return_value - callables = [(inner_decorator.return_value, None, None), + periodic_sync = self.mock_filter.get_periodic_sync_task.return_value + callables = [(periodic_sync, None, None), (inner_decorator.return_value, None, None)] self.mock_PeriodicWorker.assert_called_once_with( callables=callables, @@ -146,7 +136,7 @@ class TestWSGIServiceInitHost(BaseWSGITest): self.mock_db_init.asset_called_once_with() self.mock_validate_processing_hooks.assert_called_once_with() - self.mock_firewall_init.assert_called_once_with() + self.mock_filter.init_filter.assert_called_once_with() self.assert_periodics() def test_init_host_validate_processing_hooks_exception(self): @@ -164,14 +154,7 @@ class TestWSGIServiceInitHost(BaseWSGITest): self.mock_db_init.assert_called_once_with() self.mock_log.critical.assert_called_once_with(str(error)) self.mock_exit.assert_called_once_with(1) - - def test_init_host_not_manage_firewall(self): - CONF.set_override('manage_firewall', False, 'firewall') - self.service._init_host() - - self.mock_db_init.assert_called_once_with() - self.mock_firewall_init.assert_not_called() - self.assert_periodics() + self.mock_filter.init_filter.assert_not_called() class TestWSGIServicePeriodicWatchDog(BaseWSGITest): @@ -252,8 +235,8 @@ class TestWSGIServiceRun(BaseWSGITest): class TestWSGIServiceShutdown(BaseWSGITest): def setUp(self): super(TestWSGIServiceShutdown, self).setUp() - self.mock_firewall_clean_up = self.useFixture(fixtures.MockPatchObject( - wsgi_service.firewall, 'clean_up')).mock + self.mock_filter = self.useFixture(fixtures.MockPatchObject( + wsgi_service.pxe_filter, 'driver')).mock.return_value self.mock_executor = mock.Mock() self.mock_executor.alive = True self.mock_get_executor = self.useFixture(fixtures.MockPatchObject( @@ -279,7 +262,7 @@ class TestWSGIServiceShutdown(BaseWSGITest): self.mock__periodic_worker.wait.assert_called_once_with() self.assertIsNone(self.service._periodics_worker) self.mock_executor.shutdown.assert_called_once_with(wait=True) - self.mock_firewall_clean_up.assert_called_once_with() + self.mock_filter.tear_down_filter.assert_called_once_with() self.mock__shutting_down.release.assert_called_once_with() self.mock_exit.assert_called_once_with(error) @@ -297,7 +280,7 @@ class TestWSGIServiceShutdown(BaseWSGITest): self.assertIs(self.mock__periodic_worker, self.service._periodics_worker) self.mock_executor.shutdown.assert_not_called() - self.mock_firewall_clean_up.assert_not_called() + self.mock_filter.tear_down_filter.assert_not_called() self.mock__shutting_down.release.assert_not_called() self.mock_exit.assert_not_called() @@ -319,7 +302,7 @@ class TestWSGIServiceShutdown(BaseWSGITest): error) self.assertIsNone(self.service._periodics_worker) self.mock_executor.shutdown.assert_called_once_with(wait=True) - self.mock_firewall_clean_up.assert_called_once_with() + self.mock_filter.tear_down_filter.assert_called_once_with() self.mock__shutting_down.release.assert_called_once_with() self.mock_exit.assert_called_once_with(None) @@ -334,7 +317,7 @@ class TestWSGIServiceShutdown(BaseWSGITest): self.mock__periodic_worker.wait.assert_not_called() self.assertIsNone(self.service._periodics_worker) self.mock_executor.shutdown.assert_called_once_with(wait=True) - self.mock_firewall_clean_up.assert_called_once_with() + self.mock_filter.tear_down_filter.assert_called_once_with() self.mock__shutting_down.release.assert_called_once_with() self.mock_exit.assert_called_once_with(None) @@ -349,7 +332,7 @@ class TestWSGIServiceShutdown(BaseWSGITest): self.mock__periodic_worker.wait.assert_called_once_with() self.assertIsNone(self.service._periodics_worker) self.mock_executor.shutdown.assert_not_called() - self.mock_firewall_clean_up.assert_called_once_with() + self.mock_filter.tear_down_filter.assert_called_once_with() self.mock__shutting_down.release.assert_called_once_with() self.mock_exit.assert_called_once_with(None) diff --git a/ironic_inspector/wsgi_service.py b/ironic_inspector/wsgi_service.py index 996966b9e..3ff12b3b6 100644 --- a/ironic_inspector/wsgi_service.py +++ b/ironic_inspector/wsgi_service.py @@ -23,10 +23,10 @@ from oslo_utils import reflection from ironic_inspector.common import ironic as ir_utils from ironic_inspector import db -from ironic_inspector import firewall from ironic_inspector import main as app from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base +from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector import utils @@ -114,19 +114,15 @@ class WSGIService(object): LOG.info('Enabled processing hooks: %s', [h.name for h in hooks]) - if CONF.firewall.manage_firewall: - firewall.init() + driver = pxe_filter.driver() + driver.init_filter() - periodic_update_ = periodics.periodic( - spacing=CONF.firewall.firewall_update_period, - enabled=CONF.firewall.manage_firewall - )(firewall.update_filters) periodic_clean_up_ = periodics.periodic( spacing=CONF.clean_up_period )(periodic_clean_up) self._periodics_worker = periodics.PeriodicWorker( - callables=[(periodic_update_, None, None), + callables=[(driver.get_periodic_sync_task(), None, None), (periodic_clean_up_, None, None)], executor_factory=periodics.ExistingExecutor(utils.executor()), on_failure=self._periodics_watchdog) @@ -164,7 +160,7 @@ class WSGIService(object): if utils.executor().alive: utils.executor().shutdown(wait=True) - firewall.clean_up() + pxe_filter.driver().tear_down_filter() self._shutting_down.release() LOG.info('Shut down successfully') @@ -197,7 +193,7 @@ class WSGIService(object): def periodic_clean_up(): # pragma: no cover try: if node_cache.clean_up(): - firewall.update_filters() + pxe_filter.driver().sync(ir_utils.get_client()) sync_with_ironic() except Exception: LOG.exception('Periodic clean up of node cache failed') diff --git a/releasenotes/notes/firewall-refactoring-17e8ad764f2cde8d.yaml b/releasenotes/notes/firewall-refactoring-17e8ad764f2cde8d.yaml new file mode 100644 index 000000000..369d9e8e4 --- /dev/null +++ b/releasenotes/notes/firewall-refactoring-17e8ad764f2cde8d.yaml @@ -0,0 +1,23 @@ +--- +features: + - | + The PXE filter drivers mechanism was enabled and the firewall-based + filtering was re-implemented in the ``iptables`` driver. +deprecations: + - | + The firewall-specific configuration options were moved from the + ``firewall`` to the ``iptables``. group. + - | + The generic firewall options ``firewall_update_period`` and + ``manage_firewall`` were moved under the ``pxe_filter`` group as + ``sync_period`` and ``driver=iptables/noop`` respectively. +fixes: + - | + Should the ``iptables`` PXE filter encounter an unexpected exception in the + periodic ``sync`` call, the exception will be logged and the filter driver + will be reset in order to make subsequent ``sync`` calls fail (and propagate + the failure exiting **inspector** eventually) +other: + - | + The periodic sync of ``iptables`` and **ironic** is now handled by the + ``iptables`` PXE filter driver. diff --git a/setup.cfg b/setup.cfg index 2c2193132..2c6ce63d9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -58,6 +58,7 @@ ironic_inspector.rules.actions = set-capability = ironic_inspector.plugins.rules:SetCapabilityAction extend-attribute = ironic_inspector.plugins.rules:ExtendAttributeAction ironic_inspector.pxe_filter = + iptables = ironic_inspector.pxe_filter.iptables:IptablesFilter noop = ironic_inspector.pxe_filter.base:NoopFilter oslo.config.opts = ironic_inspector = ironic_inspector.conf:list_opts