From c16ed232da67ed2e2e6a6df05df931f40e60ceb2 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Tue, 18 Aug 2020 12:39:53 +1200 Subject: [PATCH] Convert event endpoint to plain JSON Change-Id: Ie9ce276de04eab0fedf3149a0bbf53f5ee755402 Story: 1651346 Task: 10551 --- ironic/api/controllers/v1/event.py | 74 ++++++++++++--- ironic/api/controllers/v1/types.py | 90 ------------------- .../unit/api/controllers/v1/test_event.py | 80 ++++++++++++----- .../unit/api/controllers/v1/test_types.py | 60 ------------- ironic/tests/unit/api/utils.py | 5 -- 5 files changed, 122 insertions(+), 187 deletions(-) diff --git a/ironic/api/controllers/v1/event.py b/ironic/api/controllers/v1/event.py index 477c57b90a..f11650d4e6 100644 --- a/ironic/api/controllers/v1/event.py +++ b/ironic/api/controllers/v1/event.py @@ -17,10 +17,9 @@ from oslo_log import log import pecan from ironic import api -from ironic.api.controllers.v1 import collection -from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils as api_utils -from ironic.api import expose +from ironic.api import method +from ironic.common import args from ironic.common import exception from ironic.common import policy @@ -29,11 +28,64 @@ METRICS = metrics_utils.get_metrics_logger(__name__) LOG = log.getLogger(__name__) -class EvtCollection(collection.Collection): - """API representation of a collection of events.""" +NETWORK_EVENT_VALIDATOR = args.and_valid( + args.schema({ + 'type': 'object', + 'properties': { + 'event': {'type': 'string'}, + 'port_id': {'type': 'string'}, + 'mac_address': {'type': 'string'}, + 'status': {'type': 'string'}, + 'device_id': {'type': ['string', 'null']}, + 'binding:host_id': {'type': ['string', 'null']}, + 'binding:vnic_type': {'type': ['string', 'null']}, + }, + 'required': ['event', 'port_id', 'mac_address', 'status'], + 'additionalProperties': False, + }), + args.dict_valid(**{ + 'port_id': args.uuid, + 'mac_address': args.mac_address, + 'device_id': args.uuid, + 'binding:host_id': args.uuid + }) +) - events = [types.eventtype] - """A list containing event dict objects""" +EVENT_VALIDATORS = { + 'network.bind_port': NETWORK_EVENT_VALIDATOR, + 'network.unbind_port': NETWORK_EVENT_VALIDATOR, + 'network.delete_port': NETWORK_EVENT_VALIDATOR, +} + +EVENTS_SCHEMA = { + 'type': 'object', + 'properties': { + 'events': { + 'type': 'array', + 'minItems': 1, + 'items': { + 'type': 'object', + 'properties': { + 'event': {'type': 'string', + 'enum': list(EVENT_VALIDATORS.keys())}, + }, + 'required': ['event'], + 'additionalProperties': True, + }, + }, + }, + 'required': ['events'], + 'additionalProperties': False, +} + + +def events_valid(name, value): + '''Validator for events''' + + for event in value['events']: + validator = EVENT_VALIDATORS[event['event']] + validator(name, event) + return value class EventsController(pecan.rest.RestController): @@ -45,12 +97,14 @@ class EventsController(pecan.rest.RestController): pecan.abort(http_client.NOT_FOUND) @METRICS.timer('EventsController.post') - @expose.expose(None, body=EvtCollection, - status_code=http_client.NO_CONTENT) + @method.expose(status_code=http_client.NO_CONTENT) + @method.body('evts') + @args.validate(evts=args.and_valid(args.schema(EVENTS_SCHEMA), + events_valid)) def post(self, evts): if not api_utils.allow_expose_events(): raise exception.NotFound() cdict = api.request.context.to_policy_values() policy.authorize('baremetal:events:post', cdict, cdict) - for e in evts.events: + for e in evts['events']: LOG.debug("Received external event: %s", e) diff --git a/ironic/api/controllers/v1/types.py b/ironic/api/controllers/v1/types.py index dc3b480323..7891fae320 100644 --- a/ironic/api/controllers/v1/types.py +++ b/ironic/api/controllers/v1/types.py @@ -423,93 +423,3 @@ class VifType(JsonType): viftype = VifType() - - -class EventType(atypes.UserType): - """A simple Event type.""" - - basetype = atypes.DictType - name = 'event' - - def _validate_network_port_event(value): - """Validate network port event fields. - - :param value: A event dict - :returns: value - :raises: Invalid if network port event not in proper format - """ - - validators = { - 'port_id': UuidType.validate, - 'mac_address': MacAddressType.validate, - 'status': str, - 'device_id': UuidType.validate, - 'binding:host_id': UuidType.validate, - 'binding:vnic_type': str - } - - keys = set(value) - net_keys = set(validators) - net_mandatory_fields = {'port_id', 'mac_address', 'status'} - - # Check all keys are valid for network port event - invalid = keys.difference(EventType.mandatory_fields.union(net_keys)) - if invalid: - raise exception.Invalid(_('%s are invalid keys') % - ', '.join(invalid)) - - # Check all mandatory fields for network port event is present - missing = net_mandatory_fields.difference(keys) - if missing: - raise exception.Invalid(_('Missing mandatory keys: %s') - % ', '.join(missing)) - - # Check all values are of expected type - for key in net_keys: - if key in value: - try: - validators[key](value[key]) - except Exception as e: - msg = (_('Event validation failure for %(key)s. ' - '%(message)s') % {'key': key, 'message': e}) - raise exception.Invalid(msg) - - return value - - mandatory_fields = {'event'} - event_validators = { - 'network.bind_port': _validate_network_port_event, - 'network.unbind_port': _validate_network_port_event, - 'network.delete_port': _validate_network_port_event, - } - valid_events = set(event_validators) - - @staticmethod - def validate(value): - """Validate the input - - :param value: A event dict - :returns: value - :raises: Invalid if event not in proper format - """ - - atypes.DictType(str, str).validate(value) - keys = set(value) - - # Check all mandatory fields are present - missing = EventType.mandatory_fields.difference(keys) - if missing: - raise exception.Invalid(_('Missing mandatory keys: %s') % - ', '.join(missing)) - - # Check event is a supported event - if value['event'] not in EventType.valid_events: - raise exception.Invalid( - _('%(event)s is not one of valid events: %(valid_events)s.') % - {'event': value['event'], - 'valid_events': ', '.join(EventType.valid_events)}) - - return EventType.event_validators[value['event']](value) - - -eventtype = EventType() diff --git a/ironic/tests/unit/api/controllers/v1/test_event.py b/ironic/tests/unit/api/controllers/v1/test_event.py index b67870c232..b757865a23 100644 --- a/ironic/tests/unit/api/controllers/v1/test_event.py +++ b/ironic/tests/unit/api/controllers/v1/test_event.py @@ -14,13 +14,14 @@ Tests for the API /events methods. """ from http import client as http_client -from unittest import mock from ironic.api.controllers import base as api_base -from ironic.api.controllers.v1 import types +from ironic.api.controllers.v1 import event from ironic.api.controllers.v1 import versions +from ironic.common import args +from ironic.common import exception +from ironic.tests import base as test_base from ironic.tests.unit.api import base as test_api_base -from ironic.tests.unit.api.utils import fake_event_validator def get_fake_port_event(): @@ -33,6 +34,55 @@ def get_fake_port_event(): 'binding:vnic_type': 'baremetal'} +class TestEventValidator(test_base.TestCase): + def setUp(self): + super(TestEventValidator, self).setUp() + self.v_event = event.NETWORK_EVENT_VALIDATOR + self.v_events = args.schema(event.EVENTS_SCHEMA) + + def test_simple_event_type(self): + self.v_events('body', {'events': [get_fake_port_event()]}) + + def test_invalid_event_type(self): + value = {'events': [{'event': 'invalid.event'}]} + self.assertRaisesRegex(exception.Invalid, + "Schema error for body: " + "'invalid.event' is not one of", + self.v_events, 'body', value) + + def test_event_missing_madatory_field(self): + value = {'invalid': 'invalid'} + self.assertRaisesRegex(exception.Invalid, + "Schema error for event: " + "'event' is a required property", + self.v_event, 'event', value) + + def test_invalid_mac_network_port_event(self): + value = {'event': 'network.bind_port', + 'port_id': '11111111-aaaa-bbbb-cccc-555555555555', + 'mac_address': 'INVALID_MAC_ADDRESS', + 'status': 'ACTIVE', + 'device_id': '22222222-aaaa-bbbb-cccc-555555555555', + 'binding:host_id': '22222222-aaaa-bbbb-cccc-555555555555', + 'binding:vnic_type': 'baremetal' + } + self.assertRaisesRegex(exception.Invalid, + 'Expected valid MAC address for mac_address: ' + 'INVALID_MAC_ADDRESS', + self.v_event, 'event', value) + + def test_missing_mandatory_fields_network_port_event(self): + value = {'event': 'network.bind_port', + 'device_id': '22222222-aaaa-bbbb-cccc-555555555555', + 'binding:host_id': '22222222-aaaa-bbbb-cccc-555555555555', + 'binding:vnic_type': 'baremetal' + } + self.assertRaisesRegex(exception.Invalid, + "Schema error for event: " + "'port_id' is a required property", + self.v_event, 'event', value) + + class TestPost(test_api_base.BaseApiTest): def setUp(self): @@ -40,24 +90,15 @@ class TestPost(test_api_base.BaseApiTest): self.headers = {api_base.Version.string: str( versions.max_version_string())} - @mock.patch.object(types.EventType, 'event_validators', - {'valid.event': fake_event_validator}) - @mock.patch.object(types.EventType, 'valid_events', {'valid.event'}) def test_events(self): - events_dict = {'events': [{'event': 'valid.event'}]} + events_dict = {'events': [get_fake_port_event()]} response = self.post_json('/events', events_dict, headers=self.headers) self.assertEqual(http_client.NO_CONTENT, response.status_int) - @mock.patch.object(types.EventType, 'event_validators', - {'valid.event1': fake_event_validator, - 'valid.event2': fake_event_validator, - 'valid.event3': fake_event_validator}) - @mock.patch.object(types.EventType, 'valid_events', - {'valid.event1', 'valid.event2', 'valid.event3'}) def test_multiple_events(self): - events_dict = {'events': [{'event': 'valid.event1'}, - {'event': 'valid.event2'}, - {'event': 'valid.event3'}]} + events_dict = {'events': [get_fake_port_event(), + get_fake_port_event(), + get_fake_port_event()]} response = self.post_json('/events', events_dict, headers=self.headers) self.assertEqual(http_client.NO_CONTENT, response.status_int) @@ -69,8 +110,6 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) - @mock.patch.object(types.EventType, 'event_validators', - {'valid.event': fake_event_validator}) def test_events_invalid_event(self): events_dict = {'events': [{'event': 'invalid.event'}]} response = self.post_json('/events', events_dict, expect_errors=True, @@ -167,12 +206,9 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) - @mock.patch.object(types.EventType, 'event_validators', - {'valid.event': fake_event_validator}) - @mock.patch.object(types.EventType, 'valid_events', {'valid.event'}) def test_events_unsupported_api_version(self): headers = {api_base.Version.string: '1.50'} - events_dict = {'events': [{'event': 'valid.event'}]} + events_dict = {'events': [get_fake_port_event()]} response = self.post_json('/events', events_dict, expect_errors=True, headers=headers) self.assertEqual(http_client.NOT_FOUND, response.status_int) diff --git a/ironic/tests/unit/api/controllers/v1/test_types.py b/ironic/tests/unit/api/controllers/v1/test_types.py index 63d067bc2d..7d16e98342 100644 --- a/ironic/tests/unit/api/controllers/v1/test_types.py +++ b/ironic/tests/unit/api/controllers/v1/test_types.py @@ -27,7 +27,6 @@ from ironic.common import exception from ironic.common import utils from ironic.tests import base from ironic.tests.unit.api import base as api_base -from ironic.tests.unit.api.utils import fake_event_validator class TestMacAddressType(base.TestCase): @@ -412,62 +411,3 @@ class TestVifType(base.TestCase): v = types.viftype self.assertRaises(exception.InvalidUuidOrName, v.frombasetype, {'id': 5678}) - - -class TestEventType(base.TestCase): - - def setUp(self): - super(TestEventType, self).setUp() - self.v = types.eventtype - - @mock.patch.object(types.EventType, 'event_validators', - {'valid.event': fake_event_validator}) - @mock.patch.object(types.EventType, 'valid_events', set(['valid.event'])) - def test_simple_event_type(self): - value = {'event': 'valid.event'} - self.assertCountEqual(value, self.v.validate(value)) - - @mock.patch.object(types.EventType, 'valid_events', set(['valid.event'])) - def test_invalid_event_type(self): - value = {'event': 'invalid.event'} - self.assertRaisesRegex(exception.Invalid, - 'invalid.event is not one of valid events:', - self.v.validate, value) - - def test_event_missing_madatory_field(self): - value = {'invalid': 'invalid'} - self.assertRaisesRegex(exception.Invalid, 'Missing mandatory keys:', - self.v.validate, value) - - def test_network_port_event(self): - value = {'event': 'network.bind_port', - 'port_id': '11111111-aaaa-bbbb-cccc-555555555555', - 'mac_address': 'de:ad:ca:fe:ba:be', - 'status': 'ACTIVE', - 'device_id': '22222222-aaaa-bbbb-cccc-555555555555', - 'binding:host_id': '22222222-aaaa-bbbb-cccc-555555555555', - 'binding:vnic_type': 'baremetal' - } - self.assertCountEqual(value, self.v.validate(value)) - - def test_invalid_mac_network_port_event(self): - value = {'event': 'network.bind_port', - 'port_id': '11111111-aaaa-bbbb-cccc-555555555555', - 'mac_address': 'INVALID_MAC_ADDRESS', - 'status': 'ACTIVE', - 'device_id': '22222222-aaaa-bbbb-cccc-555555555555', - 'binding:host_id': '22222222-aaaa-bbbb-cccc-555555555555', - 'binding:vnic_type': 'baremetal' - } - self.assertRaisesRegex(exception.Invalid, - 'Event validation failure for mac_address.', - self.v.validate, value) - - def test_missing_mandatory_fields_network_port_event(self): - value = {'event': 'network.bind_port', - 'device_id': '22222222-aaaa-bbbb-cccc-555555555555', - 'binding:host_id': '22222222-aaaa-bbbb-cccc-555555555555', - 'binding:vnic_type': 'baremetal' - } - self.assertRaisesRegex(exception.Invalid, 'Missing mandatory keys:', - self.v.validate, value) diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index fd005ed0ad..aa6510d18b 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -200,11 +200,6 @@ def allocation_post_data(node=None, **kw): allocation, al_controller.ALLOCATION_SCHEMA['properties']) -def fake_event_validator(v): - """A fake event validator""" - return v - - def deploy_template_post_data(**kw): """Return a DeployTemplate object without internal attributes.""" template = db_utils.get_test_deploy_template(**kw)