diff --git a/heat/api/aws/exception.py b/heat/api/aws/exception.py index a9ad6b53a5..22bf9a0278 100644 --- a/heat/api/aws/exception.py +++ b/heat/api/aws/exception.py @@ -311,6 +311,7 @@ def map_remote_error(ex): 'PropertyUnspecifiedError', 'NotSupported', 'InvalidBreakPointHook', + 'PhysicalResourceIDAmbiguity', ) denied_errors = ('Forbidden', 'NotAuthorized') already_exists_errors = ('StackExists') diff --git a/heat/api/middleware/fault.py b/heat/api/middleware/fault.py index 8fa839f0d7..c8cf3e8aab 100644 --- a/heat/api/middleware/fault.py +++ b/heat/api/middleware/fault.py @@ -63,6 +63,7 @@ class FaultWrapper(wsgi.Middleware): 'InvalidGlobalResource': webob.exc.HTTPInternalServerError, 'ResourceNotAvailable': webob.exc.HTTPNotFound, 'PhysicalResourceNameAmbiguity': webob.exc.HTTPBadRequest, + 'PhysicalResourceIDAmbiguity': webob.exc.HTTPBadRequest, 'InvalidTenant': webob.exc.HTTPForbidden, 'Forbidden': webob.exc.HTTPForbidden, 'StackExists': webob.exc.HTTPConflict, diff --git a/heat/common/exception.py b/heat/common/exception.py index 9247134627..ebe6cde712 100644 --- a/heat/common/exception.py +++ b/heat/common/exception.py @@ -177,6 +177,11 @@ class PhysicalResourceNameAmbiguity(HeatException): "Multiple physical resources were found with name (%(name)s).") +class PhysicalResourceIDAmbiguity(HeatException): + msg_fmt = _( + "Multiple resources were found with the physical ID (%(phys_id)s).") + + class InvalidTenant(HeatException): msg_fmt = _("Searching Tenant %(target)s " "from Tenant %(actual)s forbidden.") diff --git a/heat/db/api.py b/heat/db/api.py index 4daffcc094..d388681e9b 100644 --- a/heat/db/api.py +++ b/heat/db/api.py @@ -160,6 +160,11 @@ def resource_get_by_physical_resource_id(context, physical_resource_id): physical_resource_id) +def resource_get_all_by_physical_resource_id(context, physical_resource_id): + return IMPL.resource_get_all_by_physical_resource_id(context, + physical_resource_id) + + def stack_get(context, stack_id, show_deleted=False, eager_load=True): return IMPL.stack_get(context, stack_id, show_deleted=show_deleted, eager_load=eager_load) diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py index 753c35ca60..0714b69abc 100644 --- a/heat/db/sqlalchemy/api.py +++ b/heat/db/sqlalchemy/api.py @@ -204,7 +204,7 @@ def resource_get_by_name_and_stack(context, resource_name, stack_id): return result -def resource_get_by_physical_resource_id(context, physical_resource_id): +def resource_get_all_by_physical_resource_id(context, physical_resource_id): results = (context.session.query(models.Resource) .filter_by(physical_resource_id=physical_resource_id) .all()) @@ -212,8 +212,16 @@ def resource_get_by_physical_resource_id(context, physical_resource_id): for result in results: if context is None or context.tenant_id in ( result.stack.tenant, result.stack.stack_user_project_id): - return result - return None + yield result + + +def resource_get_by_physical_resource_id(context, physical_resource_id): + results = resource_get_all_by_physical_resource_id(context, + physical_resource_id) + try: + return next(results) + except StopIteration: + return None def resource_get_all(context): diff --git a/heat/engine/service.py b/heat/engine/service.py index 1936e3f685..e3e013eb36 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -1907,13 +1907,21 @@ class EngineService(service.ServiceBase): :param cnxt: RPC context. :param physical_resource_id: The physical resource ID to look up. """ - rs = resource_objects.Resource.get_by_physical_resource_id( + rsrcs = resource_objects.Resource.get_all_by_physical_resource_id( cnxt, physical_resource_id) - if not rs: + + if not rsrcs: raise exception.EntityNotFound(entity='Resource', name=physical_resource_id) + # This call is used only in the cfn API, which only cares about + # finding the stack anyway. So allow duplicate resource IDs within the + # same stack. + if len({rs.stack_id for rs in rsrcs}) > 1: + raise exception.PhysicalResourceIDAmbiguity( + phys_id=physical_resource_id) + rs = rsrcs[0] stack = parser.Stack.load(cnxt, stack_id=rs.stack_id) resource = stack[rs.name] diff --git a/heat/objects/resource.py b/heat/objects/resource.py index 0c9b34b03e..d49efbd8a7 100644 --- a/heat/objects/resource.py +++ b/heat/objects/resource.py @@ -202,11 +202,12 @@ class Resource( return cls._from_db_object(cls(context), context, resource_db) @classmethod - def get_by_physical_resource_id(cls, context, physical_resource_id): - resource_db = db_api.resource_get_by_physical_resource_id( + def get_all_by_physical_resource_id(cls, context, physical_resource_id): + matches = db_api.resource_get_all_by_physical_resource_id( context, physical_resource_id) - return cls._from_db_object(cls(context), context, resource_db) + return [cls._from_db_object(cls(context), context, resource_db) + for resource_db in matches] @classmethod def update_by_id(cls, context, resource_id, values): diff --git a/heat/tests/db/test_sqlalchemy_api.py b/heat/tests/db/test_sqlalchemy_api.py index 6425beb7d8..ff20417276 100644 --- a/heat/tests/db/test_sqlalchemy_api.py +++ b/heat/tests/db/test_sqlalchemy_api.py @@ -2318,6 +2318,20 @@ class DBAPIResourceTest(common.HeatTestCase): self.assertIsNone(db_api.resource_get_by_physical_resource_id(self.ctx, UUID2)) + def test_resource_get_all_by_physical_resource_id(self): + create_resource(self.ctx, self.stack) + create_resource(self.ctx, self.stack) + + ret_res = db_api.resource_get_all_by_physical_resource_id(self.ctx, + UUID1) + ret_list = list(ret_res) + self.assertTrue(ret_list) + for res in ret_list: + self.assertEqual(UUID1, res.physical_resource_id) + + mt = db_api.resource_get_all_by_physical_resource_id(self.ctx, UUID2) + self.assertFalse(list(mt)) + def test_resource_get_all(self): values = [ {'name': 'res1'}, diff --git a/heat/tests/engine/service/test_software_config.py b/heat/tests/engine/service/test_software_config.py index 6e27d31767..eb6c717a0c 100644 --- a/heat/tests/engine/service/test_software_config.py +++ b/heat/tests/engine/service/test_software_config.py @@ -223,10 +223,10 @@ class SoftwareConfigServiceTest(common.HeatTestCase): self.ctx, server_id=server.resource_id) self.assertEqual([deployment], deployments) - rs = resource_objects.Resource.get_by_physical_resource_id( + rsrcs = resource_objects.Resource.get_all_by_physical_resource_id( self.ctx, server_id) self.assertEqual(deployment['config_id'], - rs.rsrc_metadata.get('deployments')[0]['id']) + rsrcs[0].rsrc_metadata.get('deployments')[0]['id']) def test_metadata_software_deployments(self): stack_name = 'test_metadata_software_deployments' @@ -271,9 +271,9 @@ class SoftwareConfigServiceTest(common.HeatTestCase): # assert that metadata via metadata_software_deployments matches # metadata via server resource - rs = resource_objects.Resource.get_by_physical_resource_id( + rsrcs = resource_objects.Resource.get_all_by_physical_resource_id( self.ctx, server_id) - self.assertEqual(metadata, rs.rsrc_metadata.get('deployments')) + self.assertEqual(metadata, rsrcs[0].rsrc_metadata.get('deployments')) deployments = self.engine.metadata_software_deployments( self.ctx, server_id=str(uuid.uuid4()))