openstack
/
heat
Code Issues Proposed changes

Improve validation in OS::Monasca::Notification 

Current validation is weak, need to add more validation to
resource for webhook and email types.

Besides validation improvements incorrect period property behaviour
has been fixed: since [1] it has default value equals 60 to conform
to autoscaling behaviour, but it means, that even if no period
defined in template, it's value will be set to 60, including case
when type equals to "email", which doesn't pass validation. Revert
changes to Sirushti original solution and add him as author of
comment.

[1] https://review.openstack.org/#/c/325147/2

Change-Id: I0b6d22cf7c352379ba4a15aa9688963f6bc26660
Closes-bug: #1558469
This commit is contained in:
Peter Razumovsky 2016-08-19 17:16:25 +03:00
parent 9f2f859788
commit e3cc29c4e2
2 changed files with 134 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
heat/engine/resources/openstack/monasca/notification.py
View File

@ -11,6 +11,9 @@
# License for the specific language governing permissions and limitations
# under the License.
import re
from six.moves import urllib
from heat.common import exception
from heat.common.i18n import _
from heat.engine import constraints
@ -39,6 +42,10 @@ class MonascaNotification(resource.Resource):
entity = 'notifications'
# NOTE(sirushti): To conform to the autoscaling behaviour in heat, we set
# the default period interval during create/update to 60 for webhooks only.
_default_period_interval = 60
NOTIFICATION_TYPES = (
EMAIL, WEBHOOK, PAGERDUTY
) = (
@ -73,22 +80,27 @@ class MonascaNotification(resource.Resource):
'address, url or service key based on notification type.'),
update_allowed=True,
required=True,
constraints=[constraints.Length(max=512)]
),
PERIOD: properties.Schema(
properties.Schema.INTEGER,
_('Interval in seconds to invoke webhooks if the alarm state '
'does not transition away from the defined trigger state. The '
'default value is a period interval of 60 seconds. A '
'does not transition away from the defined trigger state. A '
'value of 0 will disable continuous notifications. This '
'property is only applicable for the webhook notification '
'type.'),
'type and has default period interval of 60 seconds.'),
support_status=support.SupportStatus(version='7.0.0'),
update_allowed=True,
constraints=[constraints.AllowedValues([0, 60])],
default=60,
constraints=[constraints.AllowedValues([0, 60])]
)
}
def _period_interval(self):
period = self.properties[self.PERIOD]
if period is None:
period = self._default_period_interval
return period
def validate(self):
super(MonascaNotification, self).validate()
if self.properties[self.PERIOD] is not None and (
@ -96,6 +108,41 @@ class MonascaNotification(resource.Resource):
msg = _('The period property can only be specified against a '
'Webhook Notification type.')
raise exception.StackValidationFailed(message=msg)
if self.properties[self.TYPE] == self.WEBHOOK:
address = self.properties[self.ADDRESS]
try:
parsed_address = urllib.parse.urlparse(address)
except Exception:
msg = _('Address "%(addr)s" should have correct format '
'required by "%(wh)s" type of "%(type)s" '
'property') % {
'addr': address,
'wh': self.WEBHOOK,
'type': self.TYPE}
raise exception.StackValidationFailed(message=msg)
if not parsed_address.scheme:
msg = _('Address "%s" doesn\'t have required URL '
'scheme') % address
raise exception.StackValidationFailed(message=msg)
if not parsed_address.netloc:
msg = _('Address "%s" doesn\'t have required network '
'location') % address
raise exception.StackValidationFailed(message=msg)
if parsed_address.scheme not in ['http', 'https']:
msg = _('Address "%(addr)s" doesn\'t satisfies '
'allowed schemes: %(schemes)s') % {
'addr': address,
'schemes': ', '.join(['http', 'https'])
}
raise exception.StackValidationFailed(message=msg)
elif (self.properties[self.TYPE] == self.EMAIL and
not re.match('^.+@.+$', self.properties[self.ADDRESS])):
msg = _('Address "%(addr)s" doesn\'t satisfies allowed format for '
'"%(email)s" type of "%(type)s" property') % {
'addr': self.properties[self.ADDRESS],
'email': self.EMAIL,
'type': self.TYPE}
raise exception.StackValidationFailed(message=msg)
def handle_create(self):
args = dict(
@ -105,7 +152,7 @@ class MonascaNotification(resource.Resource):
address=self.properties[self.ADDRESS],
)
if args['type'] == self.WEBHOOK:
args['period'] = self.properties[self.PERIOD]
args['period'] = self._period_interval()
notification = self.client().notifications.create(**args)
self.resource_id_set(notification['id'])
@ -125,7 +172,7 @@ class MonascaNotification(resource.Resource):
if args['type'] == self.WEBHOOK:
updated_period = prop_diff.get(self.PERIOD)
args['period'] = (updated_period if updated_period is not None
else self.properties[self.PERIOD])
else self._period_interval())
self.client().notifications.update(**args)

80
heat/tests/openstack/monasca/test_notification.py
View File

@ -12,6 +12,7 @@
# under the License.
import mock
import six
from heat.common import exception
from heat.engine.clients.os import monasca as client_plugin
@ -80,6 +81,49 @@ class MonascaNotificationTest(common.HeatTestCase):
self.assertRaises(exception.StackValidationFailed,
self.test_resource.validate)
def test_validate_no_scheme_address_for_webhook(self):
self.test_resource.properties.data['type'] = self.test_resource.WEBHOOK
self.test_resource.properties.data['address'] = 'abc@def.com'
ex = self.assertRaises(exception.StackValidationFailed,
self.test_resource.validate)
self.assertEqual('Address "abc@def.com" doesn\'t have '
'required URL scheme', six.text_type(ex))
def test_validate_no_netloc_address_for_webhook(self):
self.test_resource.properties.data['type'] = self.test_resource.WEBHOOK
self.test_resource.properties.data['address'] = 'https://'
ex = self.assertRaises(exception.StackValidationFailed,
self.test_resource.validate)
self.assertEqual('Address "https://" doesn\'t have '
'required network location', six.text_type(ex))
def test_validate_prohibited_address_for_webhook(self):
self.test_resource.properties.data['type'] = self.test_resource.WEBHOOK
self.test_resource.properties.data['address'] = 'ftp://127.0.0.1'
ex = self.assertRaises(exception.StackValidationFailed,
self.test_resource.validate)
self.assertEqual('Address "ftp://127.0.0.1" doesn\'t satisfies '
'allowed schemes: http, https', six.text_type(ex))
def test_validate_incorrect_address_for_email(self):
self.test_resource.properties.data['type'] = self.test_resource.EMAIL
self.test_resource.properties.data['address'] = 'abc#def.com'
self.test_resource.properties.data.pop('period')
ex = self.assertRaises(exception.StackValidationFailed,
self.test_resource.validate)
self.assertEqual('Address "abc#def.com" doesn\'t satisfies allowed '
'format for "email" type of "type" property',
six.text_type(ex))
def test_validate_invalid_address_parsing(self):
self.test_resource.properties.data['type'] = self.test_resource.WEBHOOK
self.test_resource.properties.data['address'] = "https://example.com]"
ex = self.assertRaises(exception.StackValidationFailed,
self.test_resource.validate)
self.assertEqual('Address "https://example.com]" should have correct '
'format required by "webhook" type of "type" '
'property', six.text_type(ex))
def test_resource_handle_create(self):
mock_notification_create = self.test_client.notifications.create
mock_resource = self._get_mock_resource()
@ -130,6 +174,20 @@ class MonascaNotificationTest(common.HeatTestCase):
)
mock_notification_create.assert_called_once_with(**args)
def test_resource_handle_create_no_period(self):
self.test_resource.properties.data.pop('period')
self.test_resource.properties.data['type'] = 'email'
self.test_resource.properties.data['address'] = 'abc@def.com'
mock_notification_create = self.test_client.notifications.create
self.test_resource.handle_create()
args = dict(
name='test-notification',
type='email',
address='abc@def.com'
)
mock_notification_create.assert_called_once_with(**args)
def test_resource_handle_update(self):
mock_notification_update = self.test_client.notifications.update
self.test_resource.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
@ -176,6 +234,28 @@ class MonascaNotificationTest(common.HeatTestCase):
)
mock_notification_update.assert_called_once_with(**args)
def test_resource_handle_update_no_period(self):
mock_notification_update = self.test_client.notifications.update
self.test_resource.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'
self.test_resource.properties.data.pop('period')
prop_diff = {notification.MonascaNotification.ADDRESS:
'abc@def.com',
notification.MonascaNotification.NAME: 'name-updated',
notification.MonascaNotification.TYPE: 'email'}
self.test_resource.handle_update(json_snippet=None,
tmpl_diff=None,
prop_diff=prop_diff)
args = dict(
notification_id=self.test_resource.resource_id,
name='name-updated',
type='email',
address='abc@def.com'
)
mock_notification_update.assert_called_once_with(**args)
def test_resource_handle_delete(self):
mock_notification_delete = self.test_client.notifications.delete
self.test_resource.resource_id = '477e8273-60a7-4c41-b683-fdb0bc7cd151'