From e52dc4f8aab349ae595eda0abf5e030f4fe59d5e Mon Sep 17 00:00:00 2001 From: Alexander Chadin Date: Wed, 6 Apr 2016 17:42:06 +0300 Subject: [PATCH] Add parameters verification when Audit is being created We have to check Audit Type and Audit State to make sure these parameters are in valid status. Also, we provide default states for the next attributes: - 'audit_template' is required and should be either UUID or text field - 'state' is readonly so it raises an error if submitted in POST and is set by default to PENDING - 'deadline' is optional and should be a datetime - 'type' is a required text field Change-Id: I2a7e0deec0ee2040e86400b500bb0efd8eade564 Closes-Bug: #1532843 Closes-Bug: #1533210 --- watcher/api/controllers/v1/audit.py | 32 +++++++++++++++++-- watcher/api/controllers/v1/types.py | 25 --------------- watcher/common/exception.py | 4 +++ watcher/objects/audit.py | 4 ++- watcher/tests/api/v1/test_audits.py | 29 ++++++++++++++--- .../services/infra_optim/v1/json/client.py | 2 ++ .../tests/api/admin/base.py | 4 +-- .../tests/api/admin/test_audit.py | 3 -- watcher_tempest_plugin/tests/scenario/base.py | 4 +-- 9 files changed, 67 insertions(+), 40 deletions(-) diff --git a/watcher/api/controllers/v1/audit.py b/watcher/api/controllers/v1/audit.py index 8cee072cf..a57e67f37 100644 --- a/watcher/api/controllers/v1/audit.py +++ b/watcher/api/controllers/v1/audit.py @@ -49,6 +49,31 @@ from watcher.decision_engine import rpcapi from watcher import objects +class AuditPostType(wtypes.Base): + + uuid = wtypes.wsattr(types.uuid, mandatory=False) + + audit_template_uuid = wtypes.wsattr(types.uuid, mandatory=True) + + type = wtypes.wsattr(wtypes.text, mandatory=True) + + deadline = wtypes.wsattr(datetime.datetime, mandatory=False) + + state = wsme.wsattr(wtypes.text, readonly=True, + default=objects.audit.State.PENDING) + + def as_audit(self): + audit_type_values = [val.value for val in objects.audit.AuditType] + if self.type not in audit_type_values: + raise exception.AuditTypeNotFound(audit_type=self.type) + + return Audit( + uuid=self.uuid or utils.generate_uuid(), + audit_template_id=self.audit_template_uuid, + type=self.type, + deadline=self.deadline) + + class AuditPatchType(types.JsonPatchType): @staticmethod @@ -325,12 +350,13 @@ class AuditsController(rest.RestController): audit_uuid) return Audit.convert_with_links(rpc_audit) - @wsme_pecan.wsexpose(Audit, body=Audit, status_code=201) - def post(self, audit): + @wsme_pecan.wsexpose(Audit, body=AuditPostType, status_code=201) + def post(self, audit_p): """Create a new audit. - :param audit: a audit within the request body. + :param audit_p: a audit within the request body. """ + audit = audit_p.as_audit() if self.from_audits: raise exception.OperationNotPermitted diff --git a/watcher/api/controllers/v1/types.py b/watcher/api/controllers/v1/types.py index f204f1f48..61aaf3844 100644 --- a/watcher/api/controllers/v1/types.py +++ b/watcher/api/controllers/v1/types.py @@ -31,11 +31,6 @@ class UuidOrNameType(wtypes.UserType): basetype = wtypes.text name = 'uuid_or_name' - # FIXME(lucasagomes): When used with wsexpose decorator WSME will try - # to get the name of the type by accessing it's __name__ attribute. - # Remove this __name__ attribute once it's fixed in WSME. - # https://bugs.launchpad.net/wsme/+bug/1265590 - __name__ = name @staticmethod def validate(value): @@ -55,11 +50,6 @@ class NameType(wtypes.UserType): basetype = wtypes.text name = 'name' - # FIXME(lucasagomes): When used with wsexpose decorator WSME will try - # to get the name of the type by accessing it's __name__ attribute. - # Remove this __name__ attribute once it's fixed in WSME. - # https://bugs.launchpad.net/wsme/+bug/1265590 - __name__ = name @staticmethod def validate(value): @@ -79,11 +69,6 @@ class UuidType(wtypes.UserType): basetype = wtypes.text name = 'uuid' - # FIXME(lucasagomes): When used with wsexpose decorator WSME will try - # to get the name of the type by accessing it's __name__ attribute. - # Remove this __name__ attribute once it's fixed in WSME. - # https://bugs.launchpad.net/wsme/+bug/1265590 - __name__ = name @staticmethod def validate(value): @@ -103,11 +88,6 @@ class BooleanType(wtypes.UserType): basetype = wtypes.text name = 'boolean' - # FIXME(lucasagomes): When used with wsexpose decorator WSME will try - # to get the name of the type by accessing it's __name__ attribute. - # Remove this __name__ attribute once it's fixed in WSME. - # https://bugs.launchpad.net/wsme/+bug/1265590 - __name__ = name @staticmethod def validate(value): @@ -129,11 +109,6 @@ class JsonType(wtypes.UserType): basetype = wtypes.text name = 'json' - # FIXME(lucasagomes): When used with wsexpose decorator WSME will try - # to get the name of the type by accessing it's __name__ attribute. - # Remove this __name__ attribute once it's fixed in WSME. - # https://bugs.launchpad.net/wsme/+bug/1265590 - __name__ = name def __str__(self): # These are the json serializable native types diff --git a/watcher/common/exception.py b/watcher/common/exception.py index a0d325171..53e99d9e7 100644 --- a/watcher/common/exception.py +++ b/watcher/common/exception.py @@ -180,6 +180,10 @@ class AuditTemplateReferenced(Invalid): "multiple audit") +class AuditTypeNotFound(Invalid): + msg_fmt = _("Audit type %(audit_type)s could not be found") + + class AuditNotFound(ResourceNotFound): msg_fmt = _("Audit %(audit)s could not be found") diff --git a/watcher/objects/audit.py b/watcher/objects/audit.py index 8f5340ca0..df65ebffb 100644 --- a/watcher/objects/audit.py +++ b/watcher/objects/audit.py @@ -48,6 +48,8 @@ be one of the following: :ref:`Administrator ` """ +import enum + from watcher.common import exception from watcher.common import utils from watcher.db import api as dbapi @@ -65,7 +67,7 @@ class State(object): PENDING = 'PENDING' -class AuditType(object): +class AuditType(enum.Enum): ONESHOT = 'ONESHOT' CONTINUOUS = 'CONTINUOUS' diff --git a/watcher/tests/api/v1/test_audits.py b/watcher/tests/api/v1/test_audits.py index 782dbac7b..0ef027084 100644 --- a/watcher/tests/api/v1/test_audits.py +++ b/watcher/tests/api/v1/test_audits.py @@ -432,7 +432,8 @@ class TestPost(api_base.FunctionalTest): test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time - audit_dict = post_get_test_audit() + audit_dict = post_get_test_audit(state=objects.audit.State.PENDING) + del audit_dict['state'] response = self.post_json('/audits', audit_dict) self.assertEqual('application/json', response.content_type) @@ -451,12 +452,28 @@ class TestPost(api_base.FunctionalTest): response.json['created_at']).replace(tzinfo=None) self.assertEqual(test_time, return_created_at) + @mock.patch.object(deapi.DecisionEngineAPI, 'trigger_audit') + @mock.patch('oslo_utils.timeutils.utcnow') + def test_create_audit_with_state_not_allowed(self, mock_utcnow, + mock_trigger_audit): + mock_trigger_audit.return_value = mock.ANY + test_time = datetime.datetime(2000, 1, 1, 0, 0) + mock_utcnow.return_value = test_time + + audit_dict = post_get_test_audit(state=objects.audit.State.SUCCEEDED) + + response = self.post_json('/audits', audit_dict, expect_errors=True) + self.assertEqual(400, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertTrue(response.json['error_message']) + @mock.patch('oslo_utils.timeutils.utcnow') def test_create_audit_invalid_audit_template_uuid(self, mock_utcnow): test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time audit_dict = post_get_test_audit() + del audit_dict['state'] # Make the audit template UUID some garbage value audit_dict['audit_template_uuid'] = ( '01234567-8910-1112-1314-151617181920') @@ -473,11 +490,13 @@ class TestPost(api_base.FunctionalTest): def test_create_audit_doesnt_contain_id(self, mock_trigger_audit): mock_trigger_audit.return_value = mock.ANY - audit_dict = post_get_test_audit(state='ONGOING') + audit_dict = post_get_test_audit(state=objects.audit.State.PENDING) + state = audit_dict['state'] + del audit_dict['state'] with mock.patch.object(self.dbapi, 'create_audit', wraps=self.dbapi.create_audit) as cn_mock: response = self.post_json('/audits', audit_dict) - self.assertEqual(audit_dict['state'], response.json['state']) + self.assertEqual(state, response.json['state']) cn_mock.assert_called_once_with(mock.ANY) # Check that 'id' is not in first arg of positional args self.assertNotIn('id', cn_mock.call_args[0][0]) @@ -488,6 +507,7 @@ class TestPost(api_base.FunctionalTest): audit_dict = post_get_test_audit() del audit_dict['uuid'] + del audit_dict['state'] response = self.post_json('/audits', audit_dict) self.assertEqual('application/json', response.content_type) @@ -499,7 +519,8 @@ class TestPost(api_base.FunctionalTest): def test_create_audit_trigger_decision_engine(self): with mock.patch.object(deapi.DecisionEngineAPI, 'trigger_audit') as de_mock: - audit_dict = post_get_test_audit(state='ONGOING') + audit_dict = post_get_test_audit(state=objects.audit.State.PENDING) + del audit_dict['state'] self.post_json('/audits', audit_dict) de_mock.assert_called_once_with(mock.ANY, audit_dict['uuid']) diff --git a/watcher_tempest_plugin/services/infra_optim/v1/json/client.py b/watcher_tempest_plugin/services/infra_optim/v1/json/client.py index b3ad8b7df..47bd62710 100644 --- a/watcher_tempest_plugin/services/infra_optim/v1/json/client.py +++ b/watcher_tempest_plugin/services/infra_optim/v1/json/client.py @@ -138,6 +138,8 @@ class InfraOptimClientJSON(base.BaseInfraOptimClient): """ audit = {'audit_template_uuid': audit_template_uuid} audit.update(kwargs) + if not audit['state']: + del audit['state'] return self._create_request('audits', audit) diff --git a/watcher_tempest_plugin/tests/api/admin/base.py b/watcher_tempest_plugin/tests/api/admin/base.py index cf137b836..c8c3c0e9d 100644 --- a/watcher_tempest_plugin/tests/api/admin/base.py +++ b/watcher_tempest_plugin/tests/api/admin/base.py @@ -16,8 +16,8 @@ import functools +from tempest.lib.common.utils import data_utils from tempest import test -from tempest_lib.common.utils import data_utils from watcher_tempest_plugin import infra_optim_clients as clients @@ -157,7 +157,7 @@ class BaseInfraOptimTest(test.BaseTestCase): @classmethod def create_audit(cls, audit_template_uuid, type='ONESHOT', - state='PENDING', deadline=None): + state=None, deadline=None): """Wrapper utility for creating a test audit :param audit_template_uuid: Audit Template UUID this audit will use diff --git a/watcher_tempest_plugin/tests/api/admin/test_audit.py b/watcher_tempest_plugin/tests/api/admin/test_audit.py index c956ea222..4a1bfef5b 100644 --- a/watcher_tempest_plugin/tests/api/admin/test_audit.py +++ b/watcher_tempest_plugin/tests/api/admin/test_audit.py @@ -16,7 +16,6 @@ from __future__ import unicode_literals -from tempest.lib import decorators from tempest.lib import exceptions from tempest import test @@ -72,7 +71,6 @@ class TestCreateUpdateDeleteAudit(base.BaseInfraOptimTest): self.assertRaises( exceptions.BadRequest, self.create_audit, **audit_params) - @decorators.skip_because(bug="1532843") @test.attr(type='smoke') def test_create_audit_with_invalid_state(self): _, audit_template = self.create_audit_template() @@ -85,7 +83,6 @@ class TestCreateUpdateDeleteAudit(base.BaseInfraOptimTest): self.assertRaises( exceptions.BadRequest, self.create_audit, **audit_params) - @decorators.skip_because(bug="1533210") @test.attr(type='smoke') def test_create_audit_with_no_state(self): _, audit_template = self.create_audit_template() diff --git a/watcher_tempest_plugin/tests/scenario/base.py b/watcher_tempest_plugin/tests/scenario/base.py index 89c573990..9a18fff50 100644 --- a/watcher_tempest_plugin/tests/scenario/base.py +++ b/watcher_tempest_plugin/tests/scenario/base.py @@ -24,8 +24,8 @@ from oslo_log import log from tempest import config from tempest import exceptions +from tempest.lib.common.utils import data_utils from tempest.scenario import manager -from tempest_lib.common.utils import data_utils from watcher_tempest_plugin import infra_optim_clients as clients @@ -114,7 +114,7 @@ class BaseInfraOptimScenarioTest(manager.ScenarioTest): # ### AUDITS ### # def create_audit(self, audit_template_uuid, type='ONESHOT', - state='PENDING', deadline=None): + state=None, deadline=None): """Wrapper utility for creating a test audit :param audit_template_uuid: Audit Template UUID this audit will use