Просмотр исходного кода

Merge pull request #154 from Dany9966/api-keyerror

Format KeyError API messages
Nashwan Azhari 5 лет назад
Родитель
Сommit
128778a4e9

+ 18 - 31
coriolis/api/v1/endpoints.py

@@ -6,6 +6,7 @@ from webob import exc
 
 from coriolis import exception
 from coriolis.api.v1.views import endpoint_view
+from coriolis.api.v1 import utils as api_utils
 from coriolis.api import wsgi as api_wsgi
 from coriolis.endpoints import api
 from coriolis.policies import endpoints as endpoint_policies
@@ -33,24 +34,17 @@ class EndpointController(api_wsgi.Controller):
         return endpoint_view.collection(
             req, self._endpoint_api.get_endpoints(context))
 
+    @api_utils.format_keyerror_message(resource='endpoint', method='create')
     def _validate_create_body(self, body):
-        try:
-            endpoint = body["endpoint"]
-            name = endpoint["name"]
-            description = endpoint.get("description")
-            endpoint_type = endpoint["type"]
-            connection_info = endpoint["connection_info"]
-            mapped_regions = endpoint.get("mapped_regions", [])
-            return (
-                name, endpoint_type, description, connection_info,
-                mapped_regions)
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        endpoint = body["endpoint"]
+        name = endpoint["name"]
+        description = endpoint.get("description")
+        endpoint_type = endpoint["type"]
+        connection_info = endpoint["connection_info"]
+        mapped_regions = endpoint.get("mapped_regions", [])
+        return (
+            name, endpoint_type, description, connection_info,
+            mapped_regions)
 
     def create(self, req, body):
         context = req.environ["coriolis.context"]
@@ -61,21 +55,14 @@ class EndpointController(api_wsgi.Controller):
             context, name, endpoint_type, description, connection_info,
             mapped_regions))
 
+    @api_utils.format_keyerror_message(resource='endpoint', method='update')
     def _validate_update_body(self, body):
-        try:
-            endpoint = body["endpoint"]
-            return {
-                k: endpoint[k]
-                for k in endpoint.keys() & {
-                    "name", "description", "connection_info",
-                    "mapped_regions"}}
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        endpoint = body["endpoint"]
+        return {
+            k: endpoint[k]
+            for k in endpoint.keys() & {
+                "name", "description", "connection_info",
+                "mapped_regions"}}
 
     def update(self, req, id, body):
         context = req.environ["coriolis.context"]

+ 52 - 57
coriolis/api/v1/migrations.py

@@ -1,8 +1,6 @@
 # Copyright 2016 Cloudbase Solutions Srl
 # All Rights Reserved.
 
-import json
-
 from oslo_log import log as logging
 from webob import exc
 
@@ -53,60 +51,57 @@ class MigrationController(api_wsgi.Controller):
             req, self._migration_api.get_migrations(
                 context, include_tasks=True))
 
-    def _validate_migration_input(self, context, migration):
-        try:
-            origin_endpoint_id = migration["origin_endpoint_id"]
-            destination_endpoint_id = migration["destination_endpoint_id"]
-            origin_minion_pool_id = migration.get('origin_minion_pool_id')
-            destination_minion_pool_id = migration.get(
-                'destination_minion_pool_id')
-            instance_osmorphing_minion_pool_mappings = migration.get(
-                'instance_osmorphing_minion_pool_mappings', {})
-            destination_environment = migration.get(
-                "destination_environment", {})
-            instances = migration["instances"]
-            notes = migration.get("notes")
-            skip_os_morphing = migration.get("skip_os_morphing", False)
-            shutdown_instances = migration.get(
-                "shutdown_instances", False)
-            replication_count = int(migration.get("replication_count", 2))
-            if replication_count not in range(1, 11):
-                raise ValueError(
-                    "'replication_count' must be an integer between 1 and 10."
-                    " Got: %s" % replication_count)
-
-            source_environment = migration.get("source_environment", {})
-            self._endpoints_api.validate_source_environment(
-                context, origin_endpoint_id, source_environment)
-
-            network_map = migration.get("network_map", {})
-            api_utils.validate_network_map(network_map)
-            destination_environment['network_map'] = network_map
-
-            # NOTE(aznashwan): we validate the destination environment for the
-            # import provider before appending the 'storage_mappings' parameter
-            # for plugins with strict property name checks which do not yet
-            # support storage mapping features:
-            self._endpoints_api.validate_target_environment(
-                context, destination_endpoint_id, destination_environment)
-
-            # TODO(aznashwan): until the provider plugin interface is updated
-            # to have separate 'network_map' and 'storage_mappings' fields,
-            # we add them as part of the destination environment:
-            storage_mappings = migration.get("storage_mappings", {})
-            api_utils.validate_storage_mappings(storage_mappings)
-            destination_environment['storage_mappings'] = storage_mappings
-
-            return (origin_endpoint_id, destination_endpoint_id,
-                    origin_minion_pool_id, destination_minion_pool_id,
-                    instance_osmorphing_minion_pool_mappings, source_environment,
-                    destination_environment, instances, notes,
-                    skip_os_morphing, replication_count,
-                    shutdown_instances, network_map, storage_mappings)
-        except Exception as ex:
-            LOG.exception(ex)
-            msg = getattr(ex, "message", str(ex))
-            raise exception.InvalidInput(msg)
+    @api_utils.format_keyerror_message(resource='migration', method='create')
+    def _validate_migration_input(self, context, body):
+        migration = body["migration"]
+        origin_endpoint_id = migration["origin_endpoint_id"]
+        destination_endpoint_id = migration["destination_endpoint_id"]
+        origin_minion_pool_id = migration.get('origin_minion_pool_id')
+        destination_minion_pool_id = migration.get(
+            'destination_minion_pool_id')
+        instance_osmorphing_minion_pool_mappings = migration.get(
+            'instance_osmorphing_minion_pool_mappings', {})
+        destination_environment = migration.get(
+            "destination_environment", {})
+        instances = migration["instances"]
+        notes = migration.get("notes")
+        skip_os_morphing = migration.get("skip_os_morphing", False)
+        shutdown_instances = migration.get(
+            "shutdown_instances", False)
+        replication_count = int(migration.get("replication_count", 2))
+        if replication_count not in range(1, 11):
+            raise ValueError(
+                "'replication_count' must be an integer between 1 and 10."
+                " Got: %s" % replication_count)
+
+        source_environment = migration.get("source_environment", {})
+        self._endpoints_api.validate_source_environment(
+            context, origin_endpoint_id, source_environment)
+
+        network_map = migration.get("network_map", {})
+        api_utils.validate_network_map(network_map)
+        destination_environment['network_map'] = network_map
+
+        # NOTE(aznashwan): we validate the destination environment for the
+        # import provider before appending the 'storage_mappings' parameter
+        # for plugins with strict property name checks which do not yet
+        # support storage mapping features:
+        self._endpoints_api.validate_target_environment(
+            context, destination_endpoint_id, destination_environment)
+
+        # TODO(aznashwan): until the provider plugin interface is updated
+        # to have separate 'network_map' and 'storage_mappings' fields,
+        # we add them as part of the destination environment:
+        storage_mappings = migration.get("storage_mappings", {})
+        api_utils.validate_storage_mappings(storage_mappings)
+        destination_environment['storage_mappings'] = storage_mappings
+
+        return (origin_endpoint_id, destination_endpoint_id,
+                origin_minion_pool_id, destination_minion_pool_id,
+                instance_osmorphing_minion_pool_mappings, source_environment,
+                destination_environment, instances, notes,
+                skip_os_morphing, replication_count,
+                shutdown_instances, network_map, storage_mappings)
 
     def create(self, req, body):
         # TODO(alexpilotti): validate body
@@ -142,7 +137,7 @@ class MigrationController(api_wsgi.Controller):
              shutdown_instances,
              network_map,
              storage_mappings) = self._validate_migration_input(
-                 context, migration_body)
+                 context, body)
             migration = self._migration_api.migrate_instances(
                 context, origin_endpoint_id, destination_endpoint_id,
                 origin_minion_pool_id, destination_minion_pool_id,

+ 99 - 113
coriolis/api/v1/minion_pools.py

@@ -7,7 +7,7 @@ from webob import exc
 from coriolis import constants
 from coriolis import exception
 from coriolis.api.v1.views import minion_pool_view
-from coriolis.api.v1.views import minion_pool_tasks_execution_view
+from coriolis.api.v1 import utils as api_utils
 from coriolis.api import wsgi as api_wsgi
 from coriolis.endpoints import api as endpoints_api
 from coriolis.policies import minion_pools as pools_policies
@@ -73,64 +73,57 @@ class MinionPoolController(api_wsgi.Controller):
                     "'minion_max_idle_time' must be a strictly positive "
                     "integer. Got: %s" % maximum_minions)
 
+    @api_utils.format_keyerror_message(resource='minion_pool', method='create')
     def _validate_create_body(self, ctxt, body):
-        try:
-            minion_pool = body["minion_pool"]
-            name = minion_pool["pool_name"]
-            endpoint_id = minion_pool["endpoint_id"]
-            pool_os_type = minion_pool["pool_os_type"]
-            if pool_os_type not in constants.VALID_OS_TYPES:
-                raise Exception(
-                    "The provided pool OS type '%s' is invalid. Must be one "
-                    "of the following: %s" % (
-                        pool_os_type, constants.VALID_OS_TYPES))
-            pool_platform = minion_pool["pool_platform"]
-            supported_pool_platforms = [
-                constants.PROVIDER_PLATFORM_SOURCE,
-                constants.PROVIDER_PLATFORM_DESTINATION]
-            if pool_platform not in supported_pool_platforms:
-                raise Exception(
-                    "The provided pool platform ('%s') is invalid. Must be one"
-                    " of the following: %s" % (
-                        pool_platform, supported_pool_platforms))
-            if pool_platform == constants.PROVIDER_PLATFORM_SOURCE and (
-                    pool_os_type != constants.OS_TYPE_LINUX):
-                raise Exception(
-                    "Source Minion Pools are required to be of OS type "
-                    "'%s', not '%s'." % (
-                        constants.OS_TYPE_LINUX, pool_os_type))
-            environment_options = minion_pool["environment_options"]
-            if pool_platform == constants.PROVIDER_PLATFORM_SOURCE:
-                self._endpoints_api.validate_endpoint_source_minion_pool_options(
-                    ctxt, endpoint_id, environment_options)
-            elif pool_platform == constants.PROVIDER_PLATFORM_DESTINATION:
-                self._endpoints_api.validate_endpoint_destination_minion_pool_options(
-                    ctxt, endpoint_id, environment_options)
-
-            minimum_minions = minion_pool.get("minimum_minions", 1)
-            maximum_minions = minion_pool.get(
-                "maximum_minions", minimum_minions)
-            minion_max_idle_time = minion_pool.get(
-                "minion_max_idle_time", 1)
-            self._check_pool_numeric_values(
-                minimum_minions, maximum_minions, minion_max_idle_time)
-            minion_retention_strategy = minion_pool.get(
-                "minion_retention_strategy",
-                constants.MINION_POOL_MACHINE_RETENTION_STRATEGY_DELETE)
-            self._check_pool_retention_strategy(
-                minion_retention_strategy)
-            notes = minion_pool.get("notes")
-            return (
-                name, endpoint_id, pool_platform, pool_os_type,
-                environment_options, minimum_minions, maximum_minions,
-                minion_max_idle_time, minion_retention_strategy, notes)
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        minion_pool = body["minion_pool"]
+        name = minion_pool["pool_name"]
+        endpoint_id = minion_pool["endpoint_id"]
+        pool_os_type = minion_pool["pool_os_type"]
+        if pool_os_type not in constants.VALID_OS_TYPES:
+            raise Exception(
+                "The provided pool OS type '%s' is invalid. Must be one "
+                "of the following: %s" % (
+                    pool_os_type, constants.VALID_OS_TYPES))
+        pool_platform = minion_pool["pool_platform"]
+        supported_pool_platforms = [
+            constants.PROVIDER_PLATFORM_SOURCE,
+            constants.PROVIDER_PLATFORM_DESTINATION]
+        if pool_platform not in supported_pool_platforms:
+            raise Exception(
+                "The provided pool platform ('%s') is invalid. Must be one"
+                " of the following: %s" % (
+                    pool_platform, supported_pool_platforms))
+        if pool_platform == constants.PROVIDER_PLATFORM_SOURCE and (
+                pool_os_type != constants.OS_TYPE_LINUX):
+            raise Exception(
+                "Source Minion Pools are required to be of OS type "
+                "'%s', not '%s'." % (
+                    constants.OS_TYPE_LINUX, pool_os_type))
+        environment_options = minion_pool["environment_options"]
+        if pool_platform == constants.PROVIDER_PLATFORM_SOURCE:
+            self._endpoints_api.validate_endpoint_source_minion_pool_options(
+                ctxt, endpoint_id, environment_options)
+        elif pool_platform == constants.PROVIDER_PLATFORM_DESTINATION:
+            self._endpoints_api.validate_endpoint_destination_minion_pool_options(
+                ctxt, endpoint_id, environment_options)
+
+        minimum_minions = minion_pool.get("minimum_minions", 1)
+        maximum_minions = minion_pool.get(
+            "maximum_minions", minimum_minions)
+        minion_max_idle_time = minion_pool.get(
+            "minion_max_idle_time", 1)
+        self._check_pool_numeric_values(
+            minimum_minions, maximum_minions, minion_max_idle_time)
+        minion_retention_strategy = minion_pool.get(
+            "minion_retention_strategy",
+            constants.MINION_POOL_MACHINE_RETENTION_STRATEGY_DELETE)
+        self._check_pool_retention_strategy(
+            minion_retention_strategy)
+        notes = minion_pool.get("notes")
+        return (
+            name, endpoint_id, pool_platform, pool_os_type,
+            environment_options, minimum_minions, maximum_minions,
+            minion_max_idle_time, minion_retention_strategy, notes)
 
     def create(self, req, body):
         context = req.environ["coriolis.context"]
@@ -144,62 +137,55 @@ class MinionPoolController(api_wsgi.Controller):
             environment_options, minimum_minions, maximum_minions,
             minion_max_idle_time, minion_retention_strategy, notes=notes))
 
+    @api_utils.format_keyerror_message(resource='minion_pool', method='update')
     def _validate_update_body(self, id, context, body):
-        try:
-            minion_pool = body["minion_pool"]
-            if 'endpoint_id' in minion_pool:
-                raise Exception(
-                    "The 'endpoint_id' of a minion pool cannot be updated.")
-            if 'pool_platform' in minion_pool:
-                raise Exception(
-                    "The 'pool_platform' of a minion pool cannot be updated.")
-            vals = {k: minion_pool[k] for k in minion_pool.keys() &
-                    {"name", "environment_options", "minimum_minions",
-                     "maximum_minions", "minion_max_idle_time",
-                     "minion_retention_strategy", "notes", "pool_os_type"}}
-            if 'minion_retention_strategy' in vals:
-                self._check_pool_retention_strategy(
-                    vals['minion_retention_strategy'])
-            if any([
-                    f in vals for f in [
-                        'environment_options', 'minimum_minions',
-                        'maximum_minions', 'minion_max_idle_time']]):
-                minion_pool = self._minion_pool_api.get_minion_pool(
-                    context, id)
-                self._check_pool_numeric_values(
-                    vals.get(
-                        'minimum_minions', minion_pool['minimum_minions']),
-                    vals.get(
-                        'maximum_minions', minion_pool['maximum_minions']),
-                    vals.get('minion_max_idle_time'))
-
-                if 'environment_options' in vals:
-                    if minion_pool['pool_platform'] == (
-                            constants.PROVIDER_PLATFORM_SOURCE):
-                        self._endpoints_api.validate_endpoint_source_minion_pool_options(
-                            # TODO(aznashwan): remove endpoint ID fields redundancy
-                            # once DB models are overhauled:
-                            context, minion_pool['origin_endpoint_id'],
-                            vals['environment_options'])
-                    elif minion_pool['pool_platform'] == (
-                            constants.PROVIDER_PLATFORM_DESTINATION):
-                        self._endpoints_api.validate_endpoint_destination_minion_pool_options(
-                            # TODO(aznashwan): remove endpoint ID fields redundancy
-                            # once DB models are overhauled:
-                            context, minion_pool['origin_endpoint_id'],
-                            vals['environment_options'])
-                    else:
-                        raise Exception(
-                            "Unknown pool platform: %s" % minion_pool[
-                                'pool_platform'])
-            return vals
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        minion_pool = body["minion_pool"]
+        if 'endpoint_id' in minion_pool:
+            raise Exception(
+                "The 'endpoint_id' of a minion pool cannot be updated.")
+        if 'pool_platform' in minion_pool:
+            raise Exception(
+                "The 'pool_platform' of a minion pool cannot be updated.")
+        vals = {k: minion_pool[k] for k in minion_pool.keys() &
+                {"pool_name", "environment_options", "minimum_minions",
+                 "maximum_minions", "minion_max_idle_time",
+                 "minion_retention_strategy", "notes", "pool_os_type"}}
+        if 'minion_retention_strategy' in vals:
+            self._check_pool_retention_strategy(
+                vals['minion_retention_strategy'])
+        if any([
+                f in vals for f in [
+                    'environment_options', 'minimum_minions',
+                    'maximum_minions', 'minion_max_idle_time']]):
+            minion_pool = self._minion_pool_api.get_minion_pool(
+                context, id)
+            self._check_pool_numeric_values(
+                vals.get(
+                    'minimum_minions', minion_pool['minimum_minions']),
+                vals.get(
+                    'maximum_minions', minion_pool['maximum_minions']),
+                vals.get('minion_max_idle_time'))
+
+            if 'environment_options' in vals:
+                if minion_pool['pool_platform'] == (
+                        constants.PROVIDER_PLATFORM_SOURCE):
+                    self._endpoints_api.validate_endpoint_source_minion_pool_options(
+                        # TODO(aznashwan): remove endpoint ID fields redundancy
+                        # once DB models are overhauled:
+                        context, minion_pool['origin_endpoint_id'],
+                        vals['environment_options'])
+                elif minion_pool['pool_platform'] == (
+                        constants.PROVIDER_PLATFORM_DESTINATION):
+                    self._endpoints_api.validate_endpoint_destination_minion_pool_options(
+                        # TODO(aznashwan): remove endpoint ID fields redundancy
+                        # once DB models are overhauled:
+                        context, minion_pool['origin_endpoint_id'],
+                        vals['environment_options'])
+                else:
+                    raise Exception(
+                        "Unknown pool platform: %s" % minion_pool[
+                            'pool_platform'])
+        return vals
 
     def update(self, req, id, body):
         context = req.environ["coriolis.context"]

+ 11 - 24
coriolis/api/v1/regions.py

@@ -6,6 +6,7 @@ from webob import exc
 
 from coriolis import exception
 from coriolis.api.v1.views import region_view
+from coriolis.api.v1 import utils as api_utils
 from coriolis.api import wsgi as api_wsgi
 from coriolis.policies import regions as region_policies
 from coriolis.regions import api
@@ -33,20 +34,13 @@ class RegionController(api_wsgi.Controller):
         return region_view.collection(
             req, self._region_api.get_regions(context))
 
+    @api_utils.format_keyerror_message(resource='region', method='create')
     def _validate_create_body(self, body):
-        try:
-            region = body["region"]
-            name = region["name"]
-            description = region.get("description", "")
-            enabled = region.get("enabled", True)
-            return name, description, enabled
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        region = body["region"]
+        name = region["name"]
+        description = region.get("description", "")
+        enabled = region.get("enabled", True)
+        return name, description, enabled
 
     def create(self, req, body):
         context = req.environ["coriolis.context"]
@@ -56,18 +50,11 @@ class RegionController(api_wsgi.Controller):
             context, region_name=name, description=description,
             enabled=enabled))
 
+    @api_utils.format_keyerror_message(resource='region', method='update')
     def _validate_update_body(self, body):
-        try:
-            region = body["region"]
-            return {k: region[k] for k in region.keys() &
-                    {"name", "description", "enabled"}}
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        region = body["region"]
+        return {k: region[k] for k in region.keys() &
+                {"name", "description", "enabled"}}
 
     def update(self, req, id, body):
         context = req.environ["coriolis.context"]

+ 76 - 81
coriolis/api/v1/replicas.py

@@ -52,56 +52,52 @@ class ReplicaController(api_wsgi.Controller):
             req, self._replica_api.get_replicas(
                 context, include_tasks_executions=True))
 
+    @api_utils.format_keyerror_message(resource='replica', method='create')
     def _validate_create_body(self, context, body):
-        try:
-            replica = body["replica"]
-
-            origin_endpoint_id = replica["origin_endpoint_id"]
-            destination_endpoint_id = replica["destination_endpoint_id"]
-            destination_environment = replica.get(
-                "destination_environment", {})
-            instances = replica["instances"]
-            notes = replica.get("notes")
-
-            source_environment = replica.get("source_environment", {})
-            self._endpoints_api.validate_source_environment(
-                context, origin_endpoint_id, source_environment)
-
-            network_map = replica.get("network_map", {})
-            api_utils.validate_network_map(network_map)
-            destination_environment['network_map'] = network_map
-
-            origin_minion_pool_id = replica.get(
-                'origin_minion_pool_id')
-            destination_minion_pool_id = replica.get(
-                'destination_minion_pool_id')
-            instance_osmorphing_minion_pool_mappings = replica.get(
-                'instance_osmorphing_minion_pool_mappings', {})
-
-            # NOTE(aznashwan): we validate the destination environment for the
-            # import provider before appending the 'storage_mappings' parameter
-            # for plugins with strict property name checks which do not yet
-            # support storage mapping features:
-            self._endpoints_api.validate_target_environment(
-                context, destination_endpoint_id, destination_environment)
-
-            storage_mappings = replica.get("storage_mappings", {})
-            api_utils.validate_storage_mappings(storage_mappings)
-
-            # TODO(aznashwan): until the provider plugin interface is updated
-            # to have separate 'network_map' and 'storage_mappings' fields,
-            # we add them as part of the destination environment:
-            destination_environment['storage_mappings'] = storage_mappings
-
-            return (origin_endpoint_id, destination_endpoint_id,
-                    source_environment, destination_environment, instances,
-                    network_map, storage_mappings, notes,
-                    origin_minion_pool_id, destination_minion_pool_id,
-                    instance_osmorphing_minion_pool_mappings)
-        except Exception as ex:
-            LOG.exception(ex)
-            msg = getattr(ex, "message", str(ex))
-            raise exception.InvalidInput(msg)
+        replica = body["replica"]
+
+        origin_endpoint_id = replica["origin_endpoint_id"]
+        destination_endpoint_id = replica["destination_endpoint_id"]
+        destination_environment = replica.get(
+            "destination_environment", {})
+        instances = replica["instances"]
+        notes = replica.get("notes")
+
+        source_environment = replica.get("source_environment", {})
+        self._endpoints_api.validate_source_environment(
+            context, origin_endpoint_id, source_environment)
+
+        network_map = replica.get("network_map", {})
+        api_utils.validate_network_map(network_map)
+        destination_environment['network_map'] = network_map
+
+        origin_minion_pool_id = replica.get(
+            'origin_minion_pool_id')
+        destination_minion_pool_id = replica.get(
+            'destination_minion_pool_id')
+        instance_osmorphing_minion_pool_mappings = replica.get(
+            'instance_osmorphing_minion_pool_mappings', {})
+
+        # NOTE(aznashwan): we validate the destination environment for the
+        # import provider before appending the 'storage_mappings' parameter
+        # for plugins with strict property name checks which do not yet
+        # support storage mapping features:
+        self._endpoints_api.validate_target_environment(
+            context, destination_endpoint_id, destination_environment)
+
+        storage_mappings = replica.get("storage_mappings", {})
+        api_utils.validate_storage_mappings(storage_mappings)
+
+        # TODO(aznashwan): until the provider plugin interface is updated
+        # to have separate 'network_map' and 'storage_mappings' fields,
+        # we add them as part of the destination environment:
+        destination_environment['storage_mappings'] = storage_mappings
+
+        return (origin_endpoint_id, destination_endpoint_id,
+                source_environment, destination_environment, instances,
+                network_map, storage_mappings, notes,
+                origin_minion_pool_id, destination_minion_pool_id,
+                instance_osmorphing_minion_pool_mappings)
 
     def create(self, req, body):
         context = req.environ["coriolis.context"]
@@ -238,41 +234,13 @@ class ReplicaController(api_wsgi.Controller):
 
         return final_values
 
+    @api_utils.format_keyerror_message(resource='replica', method='update')
     def _validate_update_body(self, id, context, body):
 
         replica = self._replica_api.get_replica(context, id)
-        try:
-            merged_body = self._get_merged_replica_values(
-                replica, body['replica'])
-
-            origin_endpoint_id = replica["origin_endpoint_id"]
-            destination_endpoint_id = replica["destination_endpoint_id"]
-
-            self._endpoints_api.validate_source_environment(
-                context, origin_endpoint_id,
-                merged_body["source_environment"])
-
-            destination_environment = merged_body["destination_environment"]
-            self._endpoints_api.validate_target_environment(
-                context, destination_endpoint_id, destination_environment)
-
-            api_utils.validate_network_map(merged_body["network_map"])
-
-            api_utils.validate_storage_mappings(
-                merged_body["storage_mappings"])
-
-            return merged_body
-
-        except Exception as ex:
-            LOG.exception(ex)
-            raise exception.InvalidInput(
-                getattr(ex, "message", str(ex)))
-
-    def update(self, req, id, body):
-        context = req.environ["coriolis.context"]
-        context.can(replica_policies.get_replicas_policy_label("update"))
-        origin_endpoint_id = body['replica'].get('origin_endpoint_id', None)
-        destination_endpoint_id = body['replica'].get(
+        replica_body = body['replica']
+        origin_endpoint_id = replica_body.get('origin_endpoint_id', None)
+        destination_endpoint_id = replica_body.get(
             'destination_endpoint_id', None)
         instances = body['replica'].get('instances', None)
         if origin_endpoint_id or destination_endpoint_id:
@@ -287,6 +255,33 @@ class ReplicaController(api_wsgi.Controller):
                 explanation="The list of instances of a Replica cannot be "
                             "updated")
 
+        merged_body = self._get_merged_replica_values(
+            replica, replica_body)
+
+        replica_origin_endpoint_id = replica["origin_endpoint_id"]
+        replica_destination_endpoint_id = replica[
+            "destination_endpoint_id"]
+
+        self._endpoints_api.validate_source_environment(
+            context, replica_origin_endpoint_id,
+            merged_body["source_environment"])
+
+        destination_environment = merged_body["destination_environment"]
+        self._endpoints_api.validate_target_environment(
+            context, replica_destination_endpoint_id,
+            destination_environment)
+
+        api_utils.validate_network_map(merged_body["network_map"])
+
+        api_utils.validate_storage_mappings(
+            merged_body["storage_mappings"])
+
+        return merged_body
+
+    def update(self, req, id, body):
+        context = req.environ["coriolis.context"]
+        context.can(replica_policies.get_replicas_policy_label("update"))
+
         updated_values = self._validate_update_body(id, context, body)
         try:
             return replica_tasks_execution_view.single(

+ 13 - 26
coriolis/api/v1/services.py

@@ -6,6 +6,7 @@ from webob import exc
 
 from coriolis import exception
 from coriolis.api.v1.views import service_view
+from coriolis.api.v1 import utils as api_utils
 from coriolis.api import wsgi as api_wsgi
 from coriolis.policies import services as service_policies
 from coriolis.services import api
@@ -33,22 +34,15 @@ class ServiceController(api_wsgi.Controller):
         return service_view.collection(
             req, self._service_api.get_services(context))
 
+    @api_utils.format_keyerror_message(resource='service', method='create')
     def _validate_create_body(self, body):
-        try:
-            service = body["service"]
-            host = service["host"]
-            binary = service["binary"]
-            topic = service["topic"]
-            mapped_regions = service.get('mapped_regions', [])
-            enabled = service.get("enabled", True)
-            return host, binary, topic, mapped_regions, enabled
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        service = body["service"]
+        host = service["host"]
+        binary = service["binary"]
+        topic = service["topic"]
+        mapped_regions = service.get('mapped_regions', [])
+        enabled = service.get("enabled", True)
+        return host, binary, topic, mapped_regions, enabled
 
     def create(self, req, body):
         context = req.environ["coriolis.context"]
@@ -59,18 +53,11 @@ class ServiceController(api_wsgi.Controller):
             context, host=host, binary=binary, topic=topic,
             mapped_regions=mapped_regions, enabled=enabled))
 
+    @api_utils.format_keyerror_message(resource='service', method='update')
     def _validate_update_body(self, body):
-        try:
-            service = body["service"]
-            return {k: service[k] for k in service.keys() & {
-                "enabled", "mapped_regions"}}
-        except Exception as ex:
-            LOG.exception(ex)
-            if hasattr(ex, "message"):
-                msg = ex.message
-            else:
-                msg = str(ex)
-            raise exception.InvalidInput(msg)
+        service = body["service"]
+        return {k: service[k] for k in service.keys() & {
+            "enabled", "mapped_regions"}}
 
     def update(self, req, id, body):
         context = req.environ["coriolis.context"]

+ 40 - 0
coriolis/api/v1/utils.py

@@ -1,6 +1,7 @@
 # Copyright 2018 Cloudbase Solutions Srl
 # All Rights Reserved.
 
+import functools
 import json
 
 from oslo_log import log as logging
@@ -45,3 +46,42 @@ def validate_storage_mappings(storage_mappings):
     except exception.SchemaValidationException as ex:
         raise exc.HTTPBadRequest(
             explanation="Invalid storage_mappings: %s" % str(ex))
+
+
+def format_keyerror_message(resource='', method=''):
+    def _format_keyerror_message(func):
+        @functools.wraps(func)
+        def _wrapper(*args, **kwargs):
+            try:
+                return func(*args, **kwargs)
+            except KeyError as err:
+                LOG.exception(err)
+                if err.args:
+                    key = err.args[0]
+                    exc_message = _build_keyerror_message(
+                        resource, method, key)
+                else:
+                    exc_message = str(err)
+                raise exception.InvalidInput(exc_message)
+            except Exception as err:
+                LOG.exception(err)
+                msg = getattr(err, "message", str(err))
+                raise exception.InvalidInput(msg)
+        return _wrapper
+    return _format_keyerror_message
+
+
+def _build_keyerror_message(resource, method, key):
+    msg = ''
+    method_mapping = {
+        "create": "creation",
+        "update": "update", }
+
+    if resource == key:
+        msg = 'The %s %s body needs to be encased inside the "%s" key' % (
+            resource, method_mapping[method], key)
+    else:
+        msg = 'The %s %s body lacks a required attribute: "%s"' % (
+            resource, method_mapping[method], key)
+
+    return msg