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

api-ref/source/notifications.inc

@@ -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
 -------
 

masakari/api/openstack/ha/notifications.py

@@ -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)

masakari/api/openstack/ha/schemas/payload.py

@@ -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
+}

masakari/api/validation/validators.py

@@ -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

masakari/objects/fields.py

@@ -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."""
 

masakari/tests/unit/api/openstack/ha/test_notifications.py

@@ -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",
+            "notification": {
-                             "payload": {"event": "STOPPED",
+                "hostname": "fake_host",
-                                         "host_status": "NORMAL",
+                "payload": {
-                                         "cluster_status": "ONLINE"},
+                    "instance_uuid": uuidsentinel.instance_uuid,
-                             "type": "VM",
+                    "vir_domain_event": "STOPPED_FAILED",
-                             "generated_time": "2016-09-13T09:11:21.656788"}})
+                    "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",
+                    "instance_uuid": uuidsentinel.instance_uuid,
-                    "host_status": "NORMAL",
+                    "vir_domain_event": "STOPPED_FAILED",
-                    "cluster_status": "ONLINE"
+                    "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",
+            "notification": {
-                             "payload": {"event": "STOPPED",
+                "hostname": "fake_host",
-                                         "host_status": "NORMAL",
+                "payload": {
-                                         "cluster_status": "ONLINE"},
+                    "instance_uuid": uuidsentinel.instance_uuid,
-                             "type": "VM",
+                    "vir_domain_event": "STOPPED_FAILED",
-                             "generated_time": "2016-09-13T09:11:21.656788"}}
+                    "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(