Merge "Don't RESOLVE translate in resource init"

heat/engine/resource.py

@@ -192,7 +192,9 @@ class Resource(object):
         self.context = stack.context
         self.name = name
         self.t = definition
-        self.reparse()
+        # Only translate in cases where strict_validate is True
+        self.reparse(translate=self.stack.strict_validate,
+                     client_resolve=False)
         self.update_policy = self.t.update_policy(self.update_policy_schema,
                                                   self.context)
         self.attributes_schema.update(self.base_attributes_schema)
@@ -229,8 +231,6 @@ class Resource(object):
             resource = stack.db_resource_get(name)
             if resource:
                 self._load_data(resource)
-            else:
-                self.translate_properties(self.properties)
         else:
             self.action = stack.cache_data[name]['action']
             self.status = stack.cache_data[name]['status']
@@ -331,9 +331,17 @@ class Resource(object):
             {'status': self.COMPLETE, 'replaced_by': self.replaced_by})
         return new_rs.id
 
-    def reparse(self):
+    def reparse(self, translate=True, client_resolve=True):
+        """Reparse the resource properties.
+
+        Optional translate flag for property translation and
+        client_resolve flag for resolving properties by doing
+        client lookup.
+        """
         self.properties = self.t.properties(self.properties_schema,
                                             self.context)
+        if translate:
+            self.translate_properties(self.properties, client_resolve)
 
     def __eq__(self, other):
         """Allow == comparison of two resources."""
@@ -802,9 +810,10 @@ class Resource(object):
         # Re-resolve the template, since if the resource Ref's
         # the StackId pseudo parameter, it will change after
         # the parser.Stack is stored (which is after the resources
-        # are __init__'d, but before they are create()'d)
+        # are __init__'d, but before they are create()'d). We also
+        # do client lookups for RESOLVE translation rules here.
+
         self.reparse()
-        self.translate_properties(self.properties)
         self._update_stored_properties()
 
         def pause():
@@ -898,11 +907,17 @@ class Resource(object):
     def translation_rules(self, properties):
         """Return specified rules for resource."""
 
-    def translate_properties(self, properties):
+    def translate_properties(self, properties,
-        """Translates old properties to new ones."""
+                             client_resolve=True):
+        """Translates properties with resource specific rules.
+
+        The properties parameter is a properties object and the
+        optional client_resolve flag is to specify whether to
+        do 'RESOLVE' translation with client lookup.
+        """
         rules = self.translation_rules(properties) or []
         for rule in rules:
-            rule.execute_rule()
+            rule.execute_rule(client_resolve)
 
     def _get_resource_info(self, resource_data):
         if not resource_data:
@@ -1119,10 +1134,8 @@ class Resource(object):
                 yield self.action_handler_task(action,
                                                args=[after, tmpl_diff,
                                                      prop_diff])
-
                 self.t = after
                 self.reparse()
-                self.translate_properties(self.properties)
                 self._update_stored_properties()
 
         except exception.ResourceActionRestricted as ae:

heat/engine/resources/openstack/heat/none_resource.py

@@ -36,8 +36,10 @@ class NoneResource(resource.Resource):
                       prev_resource, check_init_complete=True):
         return False
 
-    def reparse(self):
+    def reparse(self, translate=True, client_resolve=True):
         self.properties = properties.Properties(schema={}, data={})
+        if translate:
+            self.translate_properties(self.properties, client_resolve)
 
     def handle_create(self):
         self.resource_id_set(six.text_type(uuid.uuid4()))

heat/engine/translation.py

@@ -101,7 +101,7 @@ class TranslationRule(object):
             raise ValueError(_('client_plugin and finder should be specified '
                                'for Resolve rule'))
 
-    def execute_rule(self):
+    def execute_rule(self, client_resolve=True):
         try:
             (source_key, source_data) = self._get_data_from_source_path(
                 self.source_path)
@@ -130,7 +130,7 @@ class TranslationRule(object):
         elif self.rule == TranslationRule.REPLACE:
             self._exec_replace(source_key, source_data,
                                value_key, value_data, value)
-        elif self.rule == TranslationRule.RESOLVE:
+        elif self.rule == TranslationRule.RESOLVE and client_resolve:
             self._exec_resolve(source_key, source_data)
         elif self.rule == TranslationRule.DELETE:
             self._exec_delete(source_key, source_data, value)

heat/tests/openstack/neutron/test_neutron_floating_ip.py

@@ -182,12 +182,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
             'abcd1234',
             cmd_resource=None,
         ).MultipleTimes().AndReturn('abcd1234')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'subnet',
-            'sub1234',
-            cmd_resource=None,
-        ).MultipleTimes().AndReturn('sub1234')
         neutronclient.Client.create_floatingip({
             'floatingip': {'floating_network_id': u'abcd1234'}
         }).AndReturn({'floatingip': {
@@ -247,18 +241,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
         self.m.VerifyAll()
 
     def test_FnGetRefId(self):
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'network',
-            'abcd1234',
-            cmd_resource=None,
-        ).MultipleTimes().AndReturn('abcd1234')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'subnet',
-            'sub1234',
-            cmd_resource=None,
-        ).MultipleTimes().AndReturn('sub1234')
         self.m.ReplayAll()
         t = template_format.parse(neutron_floating_template)
         stack = utils.parse_stack(t)
@@ -480,12 +462,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
         self.m.VerifyAll()
 
     def _test_floating_dependancy(self):
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'network',
-            'abcd1234',
-            cmd_resource=None,
-        ).MultipleTimes().AndReturn('abcd1234')
         neutronV20.find_resourceid_by_name_or_id(
             mox.IsA(neutronclient.Client),
             'router',
@@ -540,12 +516,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
             'abcd1234',
             cmd_resource=None,
         ).MultipleTimes().AndReturn('abcd1234')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'subnet',
-            'sub1234',
-            cmd_resource=None,
-        ).MultipleTimes().AndReturn('sub1234')
         self.stub_NetworkConstraint_validate()
         neutronclient.Client.create_floatingip({
             'floatingip': {'floating_network_id': u'abcd1234',

heat/tests/openstack/neutron/test_neutron_router.py

@@ -178,23 +178,7 @@ class NeutronRouterTest(common.HeatTestCase):
         self.assertEqual([u'792ff887-6c85-4a56-b518-23f24fa65581'],
                          rsrc.properties['l3_agent_ids'])
 
-    def _test_validate(self):
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'network',
-            'net1234',
-            cmd_resource=None,
-        ).MultipleTimes().AndReturn('net1234')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'subnet',
-            'sub1234',
-            cmd_resource=None,
-        ).MultipleTimes().AndReturn('sub1234')
-        self.m.ReplayAll()
-
     def test_router_validate_distribute_l3_agents(self):
-        self._test_validate()
         t = template_format.parse(neutron_template)
         props = t['resources']['router']['properties']
 
@@ -217,7 +201,6 @@ class NeutronRouterTest(common.HeatTestCase):
         self.m.VerifyAll()
 
     def test_router_validate_l3_agents(self):
-        self._test_validate()
         t = template_format.parse(neutron_template)
         props = t['resources']['router']['properties']
 
@@ -233,7 +216,6 @@ class NeutronRouterTest(common.HeatTestCase):
         self.m.VerifyAll()
 
     def test_router_validate_ha_distribute(self):
-        self._test_validate()
         t = template_format.parse(neutron_template)
         props = t['resources']['router']['properties']
 
@@ -249,7 +231,6 @@ class NeutronRouterTest(common.HeatTestCase):
         self.m.VerifyAll()
 
     def test_router_validate_ha_l3_agents(self):
-        self._test_validate()
         t = template_format.parse(neutron_template)
         props = t['resources']['router']['properties']
         # test non ha can not specify more than one l3 agent id
@@ -491,18 +472,6 @@ class NeutronRouterTest(common.HeatTestCase):
         self._test_router_interface(resolve_router=False)
 
     def _test_router_interface(self, resolve_router=True):
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'router',
-            '3e46229d-8fce-4733-819a-b5fe630550f8',
-            cmd_resource=None,
-        ).AndReturn('3e46229d-8fce-4733-819a-b5fe630550f8')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'subnet',
-            '91e47a57-7508-46fe-afc9-fc454e8580e1',
-            cmd_resource=None,
-        ).AndReturn('91e47a57-7508-46fe-afc9-fc454e8580e1')
         neutronclient.Client.add_interface_router(
             '3e46229d-8fce-4733-819a-b5fe630550f8',
             {'subnet_id': '91e47a57-7508-46fe-afc9-fc454e8580e1'}
@@ -764,18 +733,6 @@ class NeutronRouterTest(common.HeatTestCase):
             'public',
             cmd_resource=None,
         ).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'subnet',
-            'sub1234',
-            cmd_resource=None,
-        ).AndReturn('sub1234')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'network',
-            'fc68ea2c-b60b-4b4f-bd82-94ec81110766',
-            cmd_resource=None,
-        ).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
         neutronV20.find_resourceid_by_name_or_id(
             mox.IsA(neutronclient.Client),
             'subnet',
@@ -873,13 +830,6 @@ class NeutronRouterTest(common.HeatTestCase):
             'public',
             cmd_resource=None,
         ).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'network',
-            'fc68ea2c-b60b-4b4f-bd82-94ec81110766',
-            cmd_resource=None,
-        ).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
-
         neutronclient.Client.create_router({
             "router": {
                 "name": "Test Router",
@@ -960,7 +910,6 @@ class NeutronRouterTest(common.HeatTestCase):
             '91e47a57-7508-46fe-afc9-fc454e8580e1',
             cmd_resource=None,
         ).AndReturn('91e47a57-7508-46fe-afc9-fc454e8580e1')
-
         neutronclient.Client.update_router(
             '3e46229d-8fce-4733-819a-b5fe630550f8',
             {'router': {

heat/tests/openstack/neutron/test_neutron_subnet.py

@@ -644,12 +644,6 @@ class NeutronSubnetTest(common.HeatTestCase):
                          "supported for ipv4.", six.text_type(ex))
 
     def test_validate_both_subnetpool_cidr(self):
-        neutronV20.find_resourceid_by_name_or_id(
-            mox.IsA(neutronclient.Client),
-            'subnetpool',
-            'new_pool',
-            cmd_resource=None,
-        ).AndReturn('new_pool')
         neutronV20.find_resourceid_by_name_or_id(
             mox.IsA(neutronclient.Client),
             'subnetpool',

heat/tests/test_resource.py

@@ -43,6 +43,7 @@ from heat.engine import rsrc_defn
 from heat.engine import scheduler
 from heat.engine import stack as parser
 from heat.engine import template
+from heat.engine import translation
 from heat.objects import resource as resource_objects
 from heat.objects import resource_data as resource_data_object
 from heat.tests import common
@@ -163,11 +164,12 @@ class ResourceTest(common.HeatTestCase):
         self.assertEqual('Resource name may not contain "/"',
                          six.text_type(ex))
 
+    @mock.patch.object(translation.TranslationRule, '_exec_resolve')
     @mock.patch.object(parser.Stack, 'db_resource_get')
     @mock.patch.object(resource.Resource, '_load_data')
     @mock.patch.object(resource.Resource, 'translate_properties')
     def test_stack_resources(self, mock_translate, mock_load,
-                             mock_db_get):
+                             mock_db_get, mock_resolve):
         tpl = {'HeatTemplateFormatVersion': '2012-12-12',
                'Resources':
                    {'A': {'Type': 'ResourceWithPropsType',
@@ -183,11 +185,14 @@ class ResourceTest(common.HeatTestCase):
         self.assertEqual(0, mock_load.call_count)
 
         # set stack._resources = None to reload the resources
+        # and set strict_validate = False
         stack._resources = None
+        stack.strict_validate = False
         mock_db_get.return_value = mock.Mock()
         self.assertEqual(1, len(stack.resources))
         self.assertEqual(1, mock_translate.call_count)
         self.assertEqual(1, mock_load.call_count)
+        self.assertEqual(0, mock_resolve.call_count)
 
     def test_resource_new_stack_not_stored(self):
         snippet = rsrc_defn.ResourceDefinition('aresource',

heat/tests/test_translation_rule.py

@@ -614,6 +614,35 @@ class TestTranslationRule(common.HeatTestCase):
         rule.execute_rule()
         self.assertEqual(data, props.data)
 
+    def test_resolve_rule_other_with_get_attr(self):
+        client_plugin, schema = self._test_resolve_rule()
+
+        class DummyStack(dict):
+            pass
+
+        class rsrc(object):
+            pass
+
+        stack = DummyStack(another_res=rsrc())
+        attr_func = cfn_funcs.GetAtt(stack, 'Fn::GetAtt',
+                                     ['another_res', 'name'])
+        data = {'far': attr_func}
+        props = properties.Properties(schema, data)
+        rule = translation.TranslationRule(
+            props,
+            translation.TranslationRule.RESOLVE,
+            ['far'],
+            client_plugin=client_plugin,
+            finder='find_name_id')
+        rule.execute_rule(client_resolve=False)
+        self.assertEqual(data, props.data)
+
+        mock_getatt = self.patchobject(attr_func, 'result',
+                                       return_value='rose')
+        rule.execute_rule()
+        self.assertEqual('pink', props.get('far'))
+        self.assertEqual(1, mock_getatt.call_count)
+
     def test_resolve_rule_other_with_entity(self):
         client_plugin, schema = self._test_resolve_rule()
         data = {'far': 'one'}