Return 400 if notification payload is incorrect
This patch validates the notification payload at schema level as per API ref document[1]. Raises BadRequest(400) if payload is incorrect based on notification type. APIImpact BadRequest(400) is returned if payload is incorrect instead of 202 during notification create. Closes-Bug: #1808513 [1]: https://developer.openstack.org/api-ref/instance-ha/ Change-Id: Iccad15a955be1b11f31d829624d43b6ea915305c
This commit is contained in:
parent
367ac2f7c0
commit
70ecfe946e
|
@ -112,6 +112,8 @@ Response Codes
|
|||
A conflict(409) is returned if notification with same payload is exists or
|
||||
host for which notification is generated is under maintenance.
|
||||
|
||||
BadRequest (400) is returned if notification payload is incorrect.
|
||||
|
||||
Request
|
||||
-------
|
||||
|
||||
|
|
|
@ -20,11 +20,13 @@ from webob import exc
|
|||
from masakari.api.openstack import common
|
||||
from masakari.api.openstack import extensions
|
||||
from masakari.api.openstack.ha.schemas import notifications as schema
|
||||
from masakari.api.openstack.ha.schemas import payload as payload_schema
|
||||
from masakari.api.openstack import wsgi
|
||||
from masakari.api import validation
|
||||
from masakari import exception
|
||||
from masakari.ha import api as notification_api
|
||||
from masakari.i18n import _
|
||||
from masakari.objects import fields
|
||||
from masakari.policies import notifications as notifications_policies
|
||||
|
||||
ALIAS = 'notifications'
|
||||
|
@ -36,6 +38,18 @@ class NotificationsController(wsgi.Controller):
|
|||
def __init__(self):
|
||||
self.api = notification_api.NotificationAPI()
|
||||
|
||||
@validation.schema(payload_schema.create_process_payload)
|
||||
def _validate_process_payload(self, req, body):
|
||||
pass
|
||||
|
||||
@validation.schema(payload_schema.create_vm_payload)
|
||||
def _validate_vm_payload(self, req, body):
|
||||
pass
|
||||
|
||||
@validation.schema(payload_schema.create_compute_host_payload)
|
||||
def _validate_comp_host_payload(self, req, body):
|
||||
pass
|
||||
|
||||
@wsgi.response(http.ACCEPTED)
|
||||
@extensions.expected_errors((http.BAD_REQUEST, http.FORBIDDEN,
|
||||
http.CONFLICT))
|
||||
|
@ -46,6 +60,17 @@ class NotificationsController(wsgi.Controller):
|
|||
context.can(notifications_policies.NOTIFICATIONS % 'create')
|
||||
|
||||
notification_data = body['notification']
|
||||
if notification_data['type'] == fields.NotificationType.PROCESS:
|
||||
self._validate_process_payload(req,
|
||||
body=notification_data['payload'])
|
||||
|
||||
if notification_data['type'] == fields.NotificationType.VM:
|
||||
self._validate_vm_payload(req, body=notification_data['payload'])
|
||||
|
||||
if notification_data['type'] == fields.NotificationType.COMPUTE_HOST:
|
||||
self._validate_comp_host_payload(req,
|
||||
body=notification_data['payload'])
|
||||
|
||||
try:
|
||||
notification = self.api.create_notification(
|
||||
context, notification_data)
|
||||
|
|
|
@ -0,0 +1,67 @@
|
|||
# Copyright 2018 NTT DATA. 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.
|
||||
|
||||
from masakari.objects import fields
|
||||
|
||||
|
||||
create_compute_host_payload = {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'host_status': {
|
||||
'enum': fields.HostStatusType.ALL,
|
||||
'type': 'string'},
|
||||
'cluster_status': {
|
||||
'enum': fields.ClusterStatusType.ALL,
|
||||
'type': 'string'},
|
||||
'event': {
|
||||
'enum': fields.EventType.ALL,
|
||||
'type': 'string'},
|
||||
},
|
||||
'required': ['event'],
|
||||
'additionalProperties': False
|
||||
}
|
||||
|
||||
create_process_payload = {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'process_name': {
|
||||
'type': 'string',
|
||||
'minLength': 1,
|
||||
'maxLength': 4096},
|
||||
'event': {
|
||||
'enum': fields.EventType.ALL,
|
||||
'type': 'string'},
|
||||
},
|
||||
'required': ['process_name', 'event'],
|
||||
'additionalProperties': False
|
||||
}
|
||||
|
||||
create_vm_payload = {
|
||||
'type': 'object',
|
||||
'properties': {
|
||||
'instance_uuid': {
|
||||
'type': 'string',
|
||||
'format': 'uuid'},
|
||||
'vir_domain_event': {
|
||||
'type': 'string',
|
||||
'minLength': 1,
|
||||
'maxLength': 255},
|
||||
'event': {
|
||||
'type': 'string',
|
||||
'minLength': 1,
|
||||
'maxLength': 255},
|
||||
},
|
||||
'required': ['instance_uuid', 'vir_domain_event', 'event'],
|
||||
'additionalProperties': False
|
||||
}
|
|
@ -23,6 +23,7 @@ import re
|
|||
import jsonschema
|
||||
from jsonschema import exceptions as jsonschema_exc
|
||||
from oslo_utils import timeutils
|
||||
from oslo_utils import uuidutils
|
||||
import six
|
||||
|
||||
from masakari.api.validation import parameter_types
|
||||
|
@ -95,6 +96,11 @@ def _validate_datetime_format(instance):
|
|||
return True
|
||||
|
||||
|
||||
@jsonschema.FormatChecker.cls_checks('uuid')
|
||||
def _validate_uuid_format(instance):
|
||||
return uuidutils.is_uuid_like(instance)
|
||||
|
||||
|
||||
@jsonschema.FormatChecker.cls_checks('name', exception.InvalidName)
|
||||
def _validate_name(instance):
|
||||
regex = parameter_types.valid_name_regex
|
||||
|
|
|
@ -84,6 +84,75 @@ class NotificationType(Enum):
|
|||
return cls.ALL[index]
|
||||
|
||||
|
||||
class EventType(Enum):
|
||||
"""Represents possible event types."""
|
||||
|
||||
STARTED = "STARTED"
|
||||
STOPPED = "STOPPED"
|
||||
|
||||
ALL = (STARTED, STOPPED)
|
||||
|
||||
def __init__(self):
|
||||
super(EventType,
|
||||
self).__init__(valid_values=EventType.ALL)
|
||||
|
||||
@classmethod
|
||||
def index(cls, value):
|
||||
"""Return an index into the Enum given a value."""
|
||||
return cls.ALL.index(value)
|
||||
|
||||
@classmethod
|
||||
def from_index(cls, index):
|
||||
"""Return the Enum value at a given index."""
|
||||
return cls.ALL[index]
|
||||
|
||||
|
||||
class HostStatusType(Enum):
|
||||
"""Represents possible event types for Host status."""
|
||||
|
||||
NORMAL = "NORMAL"
|
||||
UNKNOWN = "UNKNOWN"
|
||||
|
||||
ALL = (NORMAL, UNKNOWN)
|
||||
|
||||
def __init__(self):
|
||||
super(HostStatusType,
|
||||
self).__init__(valid_values=HostStatusType.ALL)
|
||||
|
||||
@classmethod
|
||||
def index(cls, value):
|
||||
"""Return an index into the Enum given a value."""
|
||||
return cls.ALL.index(value)
|
||||
|
||||
@classmethod
|
||||
def from_index(cls, index):
|
||||
"""Return the Enum value at a given index."""
|
||||
return cls.ALL[index]
|
||||
|
||||
|
||||
class ClusterStatusType(Enum):
|
||||
"""Represents possible event types for Cluster status."""
|
||||
|
||||
ONLINE = "ONLINE"
|
||||
OFFLINE = "OFFLINE"
|
||||
|
||||
ALL = (ONLINE, OFFLINE)
|
||||
|
||||
def __init__(self):
|
||||
super(ClusterStatusType,
|
||||
self).__init__(valid_values=ClusterStatusType.ALL)
|
||||
|
||||
@classmethod
|
||||
def index(cls, value):
|
||||
"""Return an index into the Enum given a value."""
|
||||
return cls.ALL.index(value)
|
||||
|
||||
@classmethod
|
||||
def from_index(cls, index):
|
||||
"""Return the Enum value at a given index."""
|
||||
return cls.ALL[index]
|
||||
|
||||
|
||||
class NotificationStatus(Enum):
|
||||
"""Represents possible statuses for notifications."""
|
||||
|
||||
|
|
|
@ -27,6 +27,7 @@ from masakari.engine import rpcapi as engine_rpcapi
|
|||
from masakari import exception
|
||||
from masakari.ha import api as ha_api
|
||||
from masakari.objects import base as obj_base
|
||||
from masakari.objects import fields
|
||||
from masakari.objects import notification as notification_obj
|
||||
from masakari import test
|
||||
from masakari.tests.unit.api.openstack import fakes
|
||||
|
@ -154,12 +155,30 @@ class NotificationTestCase(test.TestCase):
|
|||
|
||||
mock_create.return_value = NOTIFICATION
|
||||
result = self.controller.create(self.req, body={
|
||||
"notification": {"hostname": "fake_host",
|
||||
"payload": {"event": "STOPPED",
|
||||
"host_status": "NORMAL",
|
||||
"cluster_status": "ONLINE"},
|
||||
"type": "VM",
|
||||
"generated_time": "2016-09-13T09:11:21.656788"}})
|
||||
"notification": {
|
||||
"hostname": "fake_host",
|
||||
"payload": {
|
||||
"instance_uuid": uuidsentinel.instance_uuid,
|
||||
"vir_domain_event": "STOPPED_FAILED",
|
||||
"event": "LIFECYCLE"
|
||||
},
|
||||
"type": "VM",
|
||||
"generated_time": "2016-09-13T09:11:21.656788"}})
|
||||
result = result['notification']
|
||||
test_objects.compare_obj(self, result, NOTIFICATION_DATA)
|
||||
|
||||
@mock.patch.object(ha_api.NotificationAPI, 'create_notification')
|
||||
def test_create_process_notification(self, mock_create):
|
||||
mock_create.return_value = NOTIFICATION
|
||||
result = self.controller.create(self.req, body={
|
||||
"notification": {
|
||||
"hostname": "fake_host",
|
||||
"payload": {
|
||||
"process_name": "nova-compute",
|
||||
"event": "STOPPED"
|
||||
},
|
||||
"type": "PROCESS",
|
||||
"generated_time": "2016-09-13T09:11:21.656788"}})
|
||||
result = result['notification']
|
||||
test_objects.compare_obj(self, result, NOTIFICATION_DATA)
|
||||
|
||||
|
@ -171,9 +190,9 @@ class NotificationTestCase(test.TestCase):
|
|||
"notification": {
|
||||
"hostname": "fake_host",
|
||||
"payload": {
|
||||
"event": "STOPPED",
|
||||
"host_status": "NORMAL",
|
||||
"cluster_status": "ONLINE"
|
||||
"instance_uuid": uuidsentinel.instance_uuid,
|
||||
"vir_domain_event": "STOPPED_FAILED",
|
||||
"event": "LIFECYCLE"
|
||||
},
|
||||
"type": "VM",
|
||||
"generated_time": NOW
|
||||
|
@ -189,12 +208,17 @@ class NotificationTestCase(test.TestCase):
|
|||
@mock.patch.object(ha_api.NotificationAPI, 'create_notification')
|
||||
def test_create_host_not_found(self, mock_create):
|
||||
body = {
|
||||
"notification": {"hostname": "fake_host",
|
||||
"payload": {"event": "STOPPED",
|
||||
"host_status": "NORMAL",
|
||||
"cluster_status": "ONLINE"},
|
||||
"type": "VM",
|
||||
"generated_time": "2016-09-13T09:11:21.656788"}}
|
||||
"notification": {
|
||||
"hostname": "fake_host",
|
||||
"payload": {
|
||||
"instance_uuid": uuidsentinel.instance_uuid,
|
||||
"vir_domain_event": "STOPPED_FAILED",
|
||||
"event": "LIFECYCLE"
|
||||
},
|
||||
"type": "VM",
|
||||
"generated_time": "2016-09-13T09:11:21.656788"
|
||||
}
|
||||
}
|
||||
mock_create.side_effect = exception.HostNotFoundByName(
|
||||
host_name="fake_host")
|
||||
self.assertRaises(exc.HTTPBadRequest, self.controller.create,
|
||||
|
@ -272,6 +296,54 @@ class NotificationTestCase(test.TestCase):
|
|||
self.assertRaises(self.bad_request, self.controller.create,
|
||||
self.req, body=body)
|
||||
|
||||
@ddt.data(
|
||||
# invalid event for PROCESS type
|
||||
{"params": {"payload": {"event": "invalid",
|
||||
"process_name": "nova-compute"},
|
||||
"type": fields.NotificationType.PROCESS}},
|
||||
|
||||
# invalid event for VM type
|
||||
{"params": {"payload": {"event": "invalid",
|
||||
"host_status": fields.HostStatusType.NORMAL,
|
||||
"cluster_status": fields.ClusterStatusType.ONLINE},
|
||||
"type": fields.NotificationType.VM}},
|
||||
|
||||
# invalid event for HOST_COMPUTE type
|
||||
{"params": {"payload": {"event": "invalid"},
|
||||
"type": fields.NotificationType.COMPUTE_HOST}},
|
||||
|
||||
# empty payload
|
||||
{"params": {"payload": {},
|
||||
"type": fields.NotificationType.COMPUTE_HOST}},
|
||||
|
||||
# empty process_name
|
||||
{"params": {"payload": {"event": fields.EventType.STOPPED,
|
||||
"process_name": ""},
|
||||
"type": fields.NotificationType.PROCESS}},
|
||||
|
||||
# process_name too long value
|
||||
{"params": {"payload": {"event": fields.EventType.STOPPED,
|
||||
"process_name": "a" * 4097},
|
||||
"type": fields.NotificationType.PROCESS}},
|
||||
|
||||
# process_name invalid data_type
|
||||
{"params": {"payload": {"event": fields.EventType.STOPPED,
|
||||
"process_name": 123},
|
||||
"type": fields.NotificationType.PROCESS}}
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_create_with_invalid_payload(self, params):
|
||||
body = {
|
||||
"notification": {"hostname": "fake_host",
|
||||
"generated_time": "2016-09-13T09:11:21.656788"
|
||||
}
|
||||
}
|
||||
|
||||
body['notification']['payload'] = params['payload']
|
||||
body['notification']['type'] = params['type']
|
||||
self.assertRaises(self.bad_request, self.controller.create,
|
||||
self.req, body=body)
|
||||
|
||||
@mock.patch.object(ha_api.NotificationAPI, 'create_notification')
|
||||
def test_create_duplicate_notification(self, mock_create_notification):
|
||||
mock_create_notification.side_effect = exception.DuplicateNotification(
|
||||
|
|
Loading…
Reference in New Issue