From 1fafcc5ef1cfacac124e1111174df299c39deac9 Mon Sep 17 00:00:00 2001 From: jinquanni Date: Wed, 24 Aug 2016 20:04:10 +0800 Subject: [PATCH] Check unspecified parameters create audit Currently, create audit with unspecified parameters will success. This is not reasonable, we shoud return a FAILED status to notify the admin user. Change-Id: Ifbcb3b8d9e736607b05b1eb408ec0f41bdf58a2f Closes-Bug: #1599879 --- watcher/api/controllers/v1/audit.py | 2 +- watcher/common/exception.py | 4 ++ watcher/common/utils.py | 29 ++++++++-- .../strategy/context/default.py | 2 +- watcher/tests/api/v1/test_audits.py | 56 +++++++++++++++++++ 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/watcher/api/controllers/v1/audit.py b/watcher/api/controllers/v1/audit.py index 77754738b..74622e81f 100644 --- a/watcher/api/controllers/v1/audit.py +++ b/watcher/api/controllers/v1/audit.py @@ -519,7 +519,7 @@ class AuditsController(rest.RestController): if schema: # validate input parameter with default value feedback no_schema = False - utils.DefaultValidatingDraft4Validator(schema).validate( + utils.StrictDefaultValidatingDraft4Validator(schema).validate( audit.parameters) if no_schema and audit.parameters: diff --git a/watcher/common/exception.py b/watcher/common/exception.py index 142941618..54387b272 100644 --- a/watcher/common/exception.py +++ b/watcher/common/exception.py @@ -214,6 +214,10 @@ class AuditTypeNotFound(Invalid): msg_fmt = _("Audit type %(audit_type)s could not be found") +class AuditParameterNotAllowed(Invalid): + msg_fmt = _("Audit parameter %(parameter)s are not allowed") + + class AuditNotFound(ResourceNotFound): msg_fmt = _("Audit %(audit)s could not be found") diff --git a/watcher/common/utils.py b/watcher/common/utils.py index 04b487155..28b4bfdc3 100644 --- a/watcher/common/utils.py +++ b/watcher/common/utils.py @@ -19,6 +19,7 @@ from jsonschema import validators from oslo_config import cfg from oslo_log import log as logging +from watcher.common import exception import re import six @@ -142,13 +143,29 @@ def extend_with_default(validator_class): instance.setdefault(prop, subschema["default"]) for error in validate_properties( - validator, properties, instance, schema, + validator, properties, instance, schema ): yield error - return validators.extend( - validator_class, {"properties": set_defaults}, - ) + return validators.extend(validator_class, + {"properties": set_defaults}) -DefaultValidatingDraft4Validator = extend_with_default( - validators.Draft4Validator) + +# Parameter strict check extension as jsonschema doesn't support it +def extend_with_strict_schema(validator_class): + validate_properties = validator_class.VALIDATORS["properties"] + + def strict_schema(validator, properties, instance, schema): + for para in instance.keys(): + if para not in properties.keys(): + raise exception.AuditParameterNotAllowed(parameter=para) + + for error in validate_properties( + validator, properties, instance, schema + ): + yield error + + return validators.extend(validator_class, {"properties": strict_schema}) + +StrictDefaultValidatingDraft4Validator = extend_with_default( + extend_with_strict_schema(validators.Draft4Validator)) diff --git a/watcher/decision_engine/strategy/context/default.py b/watcher/decision_engine/strategy/context/default.py index 4ac263a81..ce7e4113e 100644 --- a/watcher/decision_engine/strategy/context/default.py +++ b/watcher/decision_engine/strategy/context/default.py @@ -58,7 +58,7 @@ class DefaultStrategyContext(base.BaseStrategyContext): schema = selected_strategy.get_schema() if not audit.parameters and schema: # Default value feedback if no predefined strategy - utils.DefaultValidatingDraft4Validator(schema).validate( + utils.StrictDefaultValidatingDraft4Validator(schema).validate( audit.parameters) selected_strategy.input_parameters.update({ diff --git a/watcher/tests/api/v1/test_audits.py b/watcher/tests/api/v1/test_audits.py index 825b3a918..23b315e40 100644 --- a/watcher/tests/api/v1/test_audits.py +++ b/watcher/tests/api/v1/test_audits.py @@ -567,6 +567,62 @@ class TestPost(api_base.FunctionalTest): self.assertIn(expected_error_msg, response.json['error_message']) assert not mock_trigger_audit.called + @mock.patch.object(deapi.DecisionEngineAPI, 'trigger_audit') + def test_create_audit_with_parameter_not_allowed( + self, mock_trigger_audit): + mock_trigger_audit.return_value = mock.ANY + audit_template = self.prepare_audit_template_strategy_with_parameter() + + audit_dict = api_utils.audit_post_data( + parameters={'fake1': 1, 'fake2': "hello"}) + + audit_dict['audit_template_uuid'] = audit_template['uuid'] + del_keys = ['uuid', 'goal_id', 'strategy_id', 'state', 'interval'] + for k in del_keys: + del audit_dict[k] + + response = self.post_json('/audits', audit_dict, expect_errors=True) + self.assertEqual(400, response.status_int) + self.assertEqual("application/json", response.content_type) + expected_error_msg = 'Audit parameter fake2 are not allowed' + self.assertTrue(response.json['error_message']) + self.assertIn(expected_error_msg, response.json['error_message']) + assert not mock_trigger_audit.called + + def prepare_audit_template_strategy_with_parameter(self): + fake_spec = { + "properties": { + "fake1": { + "description": "number parameter example", + "type": "number", + "default": 3.2, + "minimum": 1.0, + "maximum": 10.2, + } + } + } + template_uuid = 'e74c40e0-d825-11e2-a28f-0800200c9a67' + strategy_uuid = 'e74c40e0-d825-11e2-a28f-0800200c9a68' + template_name = 'my template' + strategy_name = 'my strategy' + strategy_id = 3 + strategy = db_utils.get_test_strategy(parameters_spec=fake_spec, + id=strategy_id, + uuid=strategy_uuid, + name=strategy_name) + obj_utils.create_test_strategy(self.context, + parameters_spec=fake_spec, + id=strategy_id, + uuid=strategy_uuid, + name=strategy_name) + obj_utils.create_test_audit_template(self.context, + strategy_id=strategy_id, + uuid=template_uuid, + name='name') + audit_template = db_utils.get_test_audit_template( + strategy_id=strategy['id'], uuid=template_uuid, name=template_name) + return audit_template + # class TestDelete(api_base.FunctionalTest):