Merge "Handle ambiguous physical resource IDs"

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')

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,

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.")

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)

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):

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]
 

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):

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'},

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()))