diff options
| -rw-r--r-- | ironic/api/controllers/v1/ramdisk.py | 32 | ||||
| -rw-r--r-- | ironic/api/controllers/v1/types.py | 21 | ||||
| -rw-r--r-- | ironic/tests/unit/api/v1/test_ramdisk.py | 17 | ||||
| -rw-r--r-- | ironic/tests/unit/api/v1/test_types.py | 21 | ||||
| -rw-r--r-- | releasenotes/notes/lookup-ignore-malformed-macs-09e7e909f3a134a3.yaml | 8 |
5 files changed, 54 insertions, 45 deletions
diff --git a/ironic/api/controllers/v1/ramdisk.py b/ironic/api/controllers/v1/ramdisk.py index ed9c77b29..cceb1987b 100644 --- a/ironic/api/controllers/v1/ramdisk.py +++ b/ironic/api/controllers/v1/ramdisk.py @@ -13,6 +13,7 @@ # under the License. from oslo_config import cfg +from oslo_log import log import pecan from pecan import rest from six.moves import http_client @@ -24,12 +25,15 @@ from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils as api_utils from ironic.api import expose from ironic.common import exception +from ironic.common.i18n import _LW from ironic.common import policy from ironic.common import states +from ironic.common import utils from ironic import objects CONF = cfg.CONF +LOG = log.getLogger(__name__) _LOOKUP_RETURN_FIELDS = ('uuid', 'properties', 'instance_info', 'driver_internal_info') @@ -78,7 +82,7 @@ class LookupResult(base.APIBase): class LookupController(rest.RestController): """Controller handling node lookup for a deploy ramdisk.""" - @expose.expose(LookupResult, types.list_of_macaddress, types.uuid) + @expose.expose(LookupResult, types.listtype, types.uuid) def get_all(self, addresses=None, node_uuid=None): """Look up a node by its MAC addresses and optionally UUID. @@ -97,7 +101,29 @@ class LookupController(rest.RestController): cdict = pecan.request.context.to_dict() policy.authorize('baremetal:driver:ipa_lookup', cdict, cdict) - if not addresses and not node_uuid: + # Validate the list of MAC addresses + if addresses is None: + addresses = [] + + valid_addresses = [] + invalid_addresses = [] + for addr in addresses: + try: + mac = utils.validate_and_normalize_mac(addr) + valid_addresses.append(mac) + except exception.InvalidMAC: + invalid_addresses.append(addr) + + if invalid_addresses: + node_log = ('' if not node_uuid + else _LW('(Node UUID: %s)') % node_uuid) + LOG.warning(_LW('The following MAC addresses "%(addrs)s" are ' + 'invalid and will be ignored by the lookup ' + 'request %(node)s'), + {'addrs': ', '.join(invalid_addresses), + 'node': node_log}) + + if not valid_addresses and not node_uuid: raise exception.IncompleteLookup() try: @@ -106,7 +132,7 @@ class LookupController(rest.RestController): pecan.request.context, node_uuid) else: node = objects.Node.get_by_port_addresses( - pecan.request.context, addresses) + pecan.request.context, valid_addresses) except exception.NotFound: # NOTE(dtantsur): we are reraising the same exception to make sure # we don't disclose the difference between nodes that are not found diff --git a/ironic/api/controllers/v1/types.py b/ironic/api/controllers/v1/types.py index 11241ebfa..482e99326 100644 --- a/ironic/api/controllers/v1/types.py +++ b/ironic/api/controllers/v1/types.py @@ -176,26 +176,6 @@ class ListType(wtypes.UserType): return ListType.validate(value) -class ListOfMacAddressesType(ListType): - """List of MAC addresses.""" - - @staticmethod - def validate(value): - """Validate and convert the input to a ListOfMacAddressesType. - - :param value: A comma separated string of MAC addresses. - :returns: A list of unique MACs, whose order is not guaranteed. - """ - items = ListType.validate(value) - return [MacAddressType.validate(item) for item in items] - - @staticmethod - def frombasetype(value): - if value is None: - return None - return ListOfMacAddressesType.validate(value) - - macaddress = MacAddressType() uuid_or_name = UuidOrNameType() name = NameType() @@ -204,7 +184,6 @@ boolean = BooleanType() listtype = ListType() # Can't call it 'json' because that's the name of the stdlib module jsontype = JsonType() -list_of_macaddress = ListOfMacAddressesType() class JsonPatchType(wtypes.Base): diff --git a/ironic/tests/unit/api/v1/test_ramdisk.py b/ironic/tests/unit/api/v1/test_ramdisk.py index 601747ec9..8d4ebc6d7 100644 --- a/ironic/tests/unit/api/v1/test_ramdisk.py +++ b/ironic/tests/unit/api/v1/test_ramdisk.py @@ -100,6 +100,23 @@ class TestLookup(test_api_base.BaseApiTest): set(data['node'])) self._check_config(data) + @mock.patch.object(ramdisk.LOG, 'warning', autospec=True) + def test_ignore_malformed_address(self, mock_log): + obj_utils.create_test_port(self.context, + node_id=self.node.id, + address=self.addresses[1]) + + addresses = ('not-a-valid-address,80:00:02:48:fe:80:00:00:00:00:00:00' + ':f4:52:14:03:00:54:06:c2,' + ','.join(self.addresses)) + data = self.get_json( + '/lookup?addresses=%s' % addresses, + headers={api_base.Version.string: str(api_v1.MAX_VER)}) + self.assertEqual(self.node.uuid, data['node']['uuid']) + self.assertEqual(set(ramdisk._LOOKUP_RETURN_FIELDS) | {'links'}, + set(data['node'])) + self._check_config(data) + self.assertTrue(mock_log.called) + def test_found_by_uuid(self): data = self.get_json( '/lookup?addresses=%s&node_uuid=%s' % diff --git a/ironic/tests/unit/api/v1/test_types.py b/ironic/tests/unit/api/v1/test_types.py index 34f4679fd..2da257527 100644 --- a/ironic/tests/unit/api/v1/test_types.py +++ b/ironic/tests/unit/api/v1/test_types.py @@ -41,27 +41,6 @@ class TestMacAddressType(base.TestCase): types.MacAddressType.validate, 'invalid-mac') -class TestListOfMacAddressesType(base.TestCase): - - def test_valid_mac_addr(self): - test_mac = 'aa:bb:cc:11:22:33' - self.assertEqual([test_mac], - types.ListOfMacAddressesType.validate(test_mac)) - - def test_valid_list(self): - test_mac = 'aa:bb:cc:11:22:33,11:22:33:44:55:66' - self.assertEqual( - sorted(test_mac.split(',')), - sorted(types.ListOfMacAddressesType.validate(test_mac))) - - def test_invalid_mac_addr(self): - self.assertRaises(exception.InvalidMAC, - types.ListOfMacAddressesType.validate, 'invalid-mac') - self.assertRaises(exception.InvalidMAC, - types.ListOfMacAddressesType.validate, - 'aa:bb:cc:11:22:33,invalid-mac') - - class TestUuidType(base.TestCase): def test_valid_uuid(self): diff --git a/releasenotes/notes/lookup-ignore-malformed-macs-09e7e909f3a134a3.yaml b/releasenotes/notes/lookup-ignore-malformed-macs-09e7e909f3a134a3.yaml new file mode 100644 index 000000000..56c1725e3 --- /dev/null +++ b/releasenotes/notes/lookup-ignore-malformed-macs-09e7e909f3a134a3.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - Fixes a problem where the deployment of a node would fail to continue + if a malformed MAC address was passed to the lookup mechanism in the + Ironic API. For example, if a node contains an Infiniband card, the + lookup used to fail because the agent ramdisk passes a MAC address + (or GID) with 20 octets (instead of the expected 6 octets) as part + of the lookup request. Invalid addresses are now ignored. |