From f8602d1e9091b24ea5afd6ef6f7ef7feb4462940 Mon Sep 17 00:00:00 2001 From: Craig Vyvial Date: Thu, 15 Mar 2012 14:29:29 -0500 Subject: [PATCH] Adding validation of the api body * cleaning up issues with the load instance(s) * Moving over the validation from creating an instance * making more __name__ and less "strings" --- development/development_enviroment.sh | 5 +- reddwarf/common/auth.py | 2 +- reddwarf/common/exception.py | 5 + reddwarf/instance/models.py | 37 +++--- reddwarf/instance/service.py | 117 ++++++++++++++++++- reddwarf/openstack/common/wsgi.py | 10 +- reddwarf/tests/unit/test_database_models.py | 7 ++ reddwarf/tests/unit/test_database_service.py | 11 +- 8 files changed, 167 insertions(+), 27 deletions(-) diff --git a/development/development_enviroment.sh b/development/development_enviroment.sh index b0c39e2009..0c0ea4b292 100644 --- a/development/development_enviroment.sh +++ b/development/development_enviroment.sh @@ -33,9 +33,10 @@ keystone --endpoint http://localhost:35357/v2.0 --token be19c524ddc92109a224 use --user $REDDWARF_USER \ --role $REDDWARF_ROLE # These are the values -REDDWARF_TENANT=reddwarf +#REDDWARF_TENANT=reddwarf +REDDWARF_TENANT=`keystone --endpoint http://localhost:35357/v2.0 --token be19c524ddc92109a224 tenant-list| grep reddwarf | cut -d ' ' -f 2` echo $REDDWARF_TENANT -REDDWARF_USER=$(mysql keystone -e "select id from user where name='reddwarf';" | awk 'NR==2') +REDDWARF_USER=`keystone --endpoint http://localhost:35357/v2.0 --token be19c524ddc92109a224 user-list| grep reddwarf | cut -d ' ' -f 2` echo $REDDWARF_USER REDDWARF_TOKEN=$(curl -d '{"auth":{"passwordCredentials":{"username": "reddwarf", "password": "REDDWARF-PASS"},"tenantName":"reddwarf"}}' -H "Content-type: application/json" http://localhost:35357/v2.0/tokens | python -mjson.tool | grep id | tr -s ' ' | cut -d ' ' -f 3 | sed s/\"/''/g | awk 'NR==2' | cut -d ',' -f 1) echo $REDDWARF_TOKEN diff --git a/reddwarf/common/auth.py b/reddwarf/common/auth.py index 805060d786..c014def387 100644 --- a/reddwarf/common/auth.py +++ b/reddwarf/common/auth.py @@ -22,7 +22,7 @@ import webob.exc import wsgi -LOG = logging.getLogger("reddwarf.common.auth") +LOG = logging.getLogger(__name__) class AuthorizationMiddleware(wsgi.Middleware): diff --git a/reddwarf/common/exception.py b/reddwarf/common/exception.py index fe83d1fa1d..1b60e721ee 100644 --- a/reddwarf/common/exception.py +++ b/reddwarf/common/exception.py @@ -53,3 +53,8 @@ class GuestError(ReddwarfError): message = _("An error occurred communicating with the guest: " "%(original_message).") + + +class BadRequest(openstack_exception.MalformedRequestBody): + + message = _("Required element/key - %(key)s was not specified") diff --git a/reddwarf/instance/models.py b/reddwarf/instance/models.py index 5c19360ad8..0847410824 100644 --- a/reddwarf/instance/models.py +++ b/reddwarf/instance/models.py @@ -45,6 +45,7 @@ def load_server(client, uuid): try: server = client.servers.get(uuid) except nova_exceptions.NotFound, e: + #TODO(cp16net) would this be the wrong id to show the user? raise rd_exceptions.NotFound(uuid=uuid) except nova_exceptions.ClientException, e: raise rd_exceptions.ReddwarfError(str(e)) @@ -55,6 +56,7 @@ def delete_server(client, server_id): try: client.servers.delete(server_id) except nova_exceptions.NotFound, e: + #TODO(cp16net) would this be the wrong id to show the user? raise rd_exceptions.NotFound(uuid=server_id) except nova_exceptions.ClientException, e: raise rd_exceptions.ReddwarfError() @@ -173,20 +175,23 @@ class Instance(object): return links -class Instances(Instance): - - def __init__(self, context): - #TODO(hub-cap): Fix this, this just cant be right - client = create_nova_client(context) - self._data_object = client.servers.list() - - def __iter__(self): - for item in self._data_object: - yield item +class Instances(object): @staticmethod def load(context): - raise Exception("Implement this!") + if context is None: + raise TypeError("Argument context not defined.") + client = create_nova_client(context) + servers = client.servers.list() + db_infos = DBInstance.find_all() + ret = [] + for db in db_infos: + status = InstanceServiceStatus.find_by(instance_id=db.id) + for server in servers: + if server.id == db.compute_instance_id: + ret.append(Instance(context, db, server, status)) + break + return ret class DatabaseModelBase(ModelBase): @@ -220,7 +225,7 @@ class DatabaseModelBase(ModelBase): @classmethod def find_by(cls, **conditions): model = cls.get_by(**conditions) - if model == None: + if model is None: raise ModelNotFoundError(_("%s Not Found") % cls.__name__) return model @@ -228,6 +233,10 @@ class DatabaseModelBase(ModelBase): def get_by(cls, **kwargs): return db.db_api.find_by(cls, **cls._process_conditions(kwargs)) + @classmethod + def find_all(cls, **kwargs): + return db.db_query.find_all(cls, **cls._process_conditions(kwargs)) + @classmethod def _process_conditions(cls, raw_conditions): """Override in inheritors to format/modify any conditions.""" @@ -249,10 +258,10 @@ class DBInstance(DatabaseModelBase): self.set_task_status(task_status) def _validate(self, errors): - if self.task_status is None: - errors['task_status'] = "Cannot be none." if InstanceTask.from_code(self.task_id) is None: errors['task_id'] = "Not valid." + if self.task_status is None: + errors['task_status'] = "Cannot be none." def get_task_status(self): return InstanceTask.from_code(self.task_id) diff --git a/reddwarf/instance/service.py b/reddwarf/instance/service.py index a706915318..abaa82a559 100644 --- a/reddwarf/instance/service.py +++ b/reddwarf/instance/service.py @@ -25,6 +25,7 @@ from reddwarf.common import exception from reddwarf.common import utils from reddwarf.common import wsgi from reddwarf.instance import models, views +from reddwarf.common import exception as rd_exceptions CONFIG = config.Config LOG = logging.getLogger(__name__) @@ -39,6 +40,7 @@ class BaseController(wsgi.Controller): ], webob.exc.HTTPBadRequest: [ models.InvalidModelError, + exception.BadRequest, ], webob.exc.HTTPNotFound: [ exception.NotFound, @@ -58,6 +60,23 @@ class BaseController(wsgi.Controller): *self.exclude_attr)) +class api_validation: + """ api validation wrapper """ + def __init__(self, action=None): + self.action = action + + def __call__(self, f): + """ + Apply validation of the api body + """ + def wrapper(*args, **kwargs): + body = kwargs['body'] + if self.action == 'create': + InstanceController._validate(body) + return f(*args, **kwargs) + return wrapper + + class InstanceController(BaseController): """Controller for instance functionality""" @@ -65,6 +84,7 @@ class InstanceController(BaseController): """Return all instances.""" LOG.info("req : '%s'\n\n" % req) LOG.info("Detailing a database instance for tenant '%s'" % tenant_id) + #TODO(cp16net) return a detailed list instead of index return self.index(req, tenant_id) def index(self, req, tenant_id): @@ -94,6 +114,7 @@ class InstanceController(BaseController): except exception.ReddwarfError, e: # TODO(hub-cap): come up with a better way than # this to get the message + LOG.error(e) return wsgi.Result(str(e), 404) # TODO(cp16net): need to set the return code correctly return wsgi.Result(views.InstanceView(server).data(), 201) @@ -107,15 +128,14 @@ class InstanceController(BaseController): context = rd_context.ReddwarfContext( auth_tok=req.headers["X-Auth-Token"], tenant=tenant_id) - # TODO(cp16net) : need to handle exceptions here if the delete fails models.Instance.delete(context=context, uuid=id) - # TODO(cp16net): need to set the return code correctly return wsgi.Result(202) + @api_validation(action="create") def create(self, req, body, tenant_id): # find the service id (cant be done yet at startup due to - # inconsitencies w/ the load app paste + # inconsistencies w/ the load app paste # TODO(hub-cap): figure out how to get this to work in __init__ time # TODO(hub-cap): The problem with this in __init__ is that the paste # config is generated w/ the same config file as the db flags that @@ -141,6 +161,97 @@ class InstanceController(BaseController): #TODO(cp16net): need to set the return code correctly return wsgi.Result(views.InstanceView(instance).data(), 201) + @staticmethod + def _validate_empty_body(body): + """Check that the body is not empty""" + if not body: + msg = "The request contains an empty body" + raise rd_exceptions.ReddwarfError(msg) + + @staticmethod + def _validate_volume_size(size): + """Validate the various possible errors for volume size""" + try: + volume_size = float(size) + except (ValueError, TypeError) as err: + LOG.error(err) + msg = ("Required element/key - instance volume" + "'size' was not specified as a number") + raise rd_exceptions.ReddwarfError(msg) + if int(volume_size) != volume_size or int(volume_size) < 1: + msg = ("Volume 'size' needs to be a positive " + "integer value, %s cannot be accepted." + % volume_size) + raise rd_exceptions.ReddwarfError(msg) + #TODO(cp16net) add in the volume validation when volumes are supported +# max_size = FLAGS.reddwarf_max_accepted_volume_size +# if int(volume_size) > max_size: +# msg = ("Volume 'size' cannot exceed maximum " +# "of %d Gb, %s cannot be accepted." +# % (max_size, volume_size)) +# raise rd_exceptions.ReddwarfError(msg) + + @staticmethod + def _validate(body): + """Validate that the request has all the required parameters""" + InstanceController._validate_empty_body(body) + try: + body['instance'] + body['instance']['flavorRef'] + # TODO(cp16net) add in volume to the mix +# volume_size = body['instance']['volume']['size'] + except KeyError as e: + LOG.error("Create Instance Required field(s) - %s" % e) + raise rd_exceptions.ReddwarfError("Required element/key - %s " + "was not specified" % e) +# Instance._validate_volume_size(volume_size) + + @staticmethod + def _validate_resize_instance(body): + """ Validate that the resize body has the attributes for flavorRef """ + try: + body['resize'] + body['resize']['flavorRef'] + except KeyError as e: + LOG.error("Resize Instance Required field(s) - %s" % e) + raise rd_exceptions.ReddwarfError("Required element/key - %s " + "was not specified" % e) + + @staticmethod + def _validate_single_resize_in_body(body): + # Validate body resize does not have both volume and flavorRef + try: + resize = body['resize'] + if 'volume' in resize and 'flavorRef' in resize: + msg = ("Not allowed to resize volume " + "and flavor at the same time") + LOG.error(msg) + raise rd_exceptions.ReddwarfError(msg) + except KeyError as e: + LOG.error("Resize Instance Required field(s) - %s" % e) + raise rd_exceptions.ReddwarfError("Required element/key - %s " + "was not specified" % e) + + @staticmethod + def _validate_resize(body, old_volume_size): + """ + We are going to check that volume resizing data is present. + """ + InstanceController._validate_empty_body(body) + try: + body['resize'] + body['resize']['volume'] + new_volume_size = body['resize']['volume']['size'] + except KeyError as e: + LOG.error("Resize Instance Required field(s) - %s" % e) + raise rd_exceptions.ReddwarfError("Required element/key - %s " + "was not specified" % e) + Instance._validate_volume_size(new_volume_size) + if int(new_volume_size) <= old_volume_size: + raise rd_exceptions.ReddwarfError("The new volume 'size' cannot " + "be less than the current volume size " + "of '%s'" % old_volume_size) + class API(wsgi.Router): """API""" diff --git a/reddwarf/openstack/common/wsgi.py b/reddwarf/openstack/common/wsgi.py index 3d6731dbe8..f31df1c47e 100644 --- a/reddwarf/openstack/common/wsgi.py +++ b/reddwarf/openstack/common/wsgi.py @@ -578,7 +578,7 @@ class RequestDeserializer(object): def deserialize_body(self, request, action): if not len(request.body) > 0: LOG.debug(_("Empty body provided in request")) - return {} + return self._return_empty_body(action) try: content_type = request.get_content_type() @@ -588,7 +588,7 @@ class RequestDeserializer(object): if content_type is None: LOG.debug(_("No Content-Type provided in request")) - return {} + return self._return_empty_body(action) try: deserializer = self.get_body_deserializer(content_type) @@ -598,6 +598,12 @@ class RequestDeserializer(object): return deserializer.deserialize(request.body, action) + def _return_empty_body(self, action): + if action in ["create", "update", "action"]: + return {'body': None} + else: + return {} + def get_body_deserializer(self, content_type): try: return self.body_deserializers[content_type] diff --git a/reddwarf/tests/unit/test_database_models.py b/reddwarf/tests/unit/test_database_models.py index 50502fdb04..b56b83ed9d 100644 --- a/reddwarf/tests/unit/test_database_models.py +++ b/reddwarf/tests/unit/test_database_models.py @@ -19,6 +19,7 @@ import novaclient from reddwarf import tests from reddwarf.common import utils +from reddwarf.common import exception from reddwarf.instance import models from reddwarf.instance.tasks import InstanceTasks from reddwarf.tests.factories import models as factory_models @@ -88,3 +89,9 @@ class TestInstance(tests.BaseTest): self.assertEqual(instance['task_id'], InstanceTasks.BUILDING.code) self.assertEqual(instance['task_description'], InstanceTasks.BUILDING.db_text) + + def test_create_instance_data_without_flavorref(self): + #todo(cp16net) fix this to work with the factory + self.mock_out_client() + self.FAKE_SERVER.flavor = None + self.assertRaises(exception.BadRequest, factory_models.Instance()) diff --git a/reddwarf/tests/unit/test_database_service.py b/reddwarf/tests/unit/test_database_service.py index 72c4b34c06..0b3854f254 100644 --- a/reddwarf/tests/unit/test_database_service.py +++ b/reddwarf/tests/unit/test_database_service.py @@ -55,11 +55,12 @@ class TestInstanceController(ControllerTestBase): super(TestInstanceController, self).setUp() # TODO(hub-cap): Start testing the failure cases - # def test_show_broken(self): - # response = self.app.get("%s/%s" % (self.instances_path, - # self.DUMMY_INSTANCE_ID), - # headers={'X-Auth-Token': '123'}) - # self.assertEqual(response.status_int, 404) + def test_show_broken(self): + raise SkipTest() + response = self.app.get("%s/%s" % (self.instances_path, + self.DUMMY_INSTANCE_ID), + headers={'X-Auth-Token': '123'}) + self.assertEqual(response.status_int, 404) def test_show(self): raise SkipTest()