From 81507cc09fd92c3cc9e0a5dc01e699354e563f14 Mon Sep 17 00:00:00 2001 From: Kanagaraj Manickam Date: Fri, 17 Apr 2015 14:31:50 +0530 Subject: [PATCH] Removes redundant validation of template (1) As part of stack create and preview, template is being validated twice, first at service._validate_new_stack() and second at stack.validate() by self.t.validate(). This patch fixes this issue. It also improves code readability by using proper indentation. NOTE: second part, will have dogpile cache backend integration Change-Id: I86649752cadcd145760d482d07313881183b02b0 Closes-bug: #1444316 --- heat/engine/template.py | 16 +++++++++ heat/tests/test_template.py | 70 +++++++++++++++++++++++++++++-------- 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/heat/engine/template.py b/heat/engine/template.py index 7a3ae839a..c890064a6 100644 --- a/heat/engine/template.py +++ b/heat/engine/template.py @@ -15,6 +15,7 @@ import abc import collections import copy import functools +import hashlib from oslo_log import log as logging import six @@ -115,8 +116,10 @@ class Template(collections.Mapping): self.files = files or {} self.maps = self[self.MAPPINGS] self.env = env or environment.Environment({}) + self.version = get_version(self.t, list(six.iterkeys(_template_classes))) + self.t_digest = None def __deepcopy__(self, memo): return Template(copy.deepcopy(self.t, memo), files=self.files, @@ -226,6 +229,18 @@ class Template(collections.Mapping): sections (e.g. parameters are check by parameters schema class). ''' + t_digest = hashlib.sha256(six.text_type(self.t)).hexdigest() + + # TODO(kanagaraj-manickam) currently t_digest is stored in self. which + # is used to check whether already template is validated or not. + # But it needs to be loaded from dogpile cache backend once its + # available in heat (http://specs.openstack.org/openstack/heat-specs/ + # specs/liberty/constraint-validation-cache.html). This is required + # as multiple heat-engines may process the same template at least + # in case of instance_group. And it fixes partially bug 1444316 + + if t_digest == self.t_digest: + return # check top-level sections for k in six.iterkeys(self.t): @@ -243,6 +258,7 @@ class Template(collections.Mapping): message = _('Resources must contain Resource. ' 'Found a [%s] instead') % type(res) raise exception.StackValidationFailed(message=message) + self.t_digest = t_digest @classmethod def create_empty_template(cls, diff --git a/heat/tests/test_template.py b/heat/tests/test_template.py index 59142a0e0..983ec52c1 100644 --- a/heat/tests/test_template.py +++ b/heat/tests/test_template.py @@ -12,6 +12,7 @@ # under the License. import copy +import hashlib import json import fixtures @@ -271,6 +272,27 @@ class ParserTest(common.HeatTestCase): class TestTemplateValidate(common.HeatTestCase): + def test_template_validate_cfn_check_t_digest(self): + t = { + 'AWSTemplateFormatVersion': '2010-09-09', + 'Description': 'foo', + 'Parameters': {}, + 'Mappings': {}, + 'Resources': { + 'server': { + 'Type': 'OS::Nova::Server' + } + }, + 'Outputs': {}, + } + + tmpl = template.Template(t) + self.assertIsNone(tmpl.t_digest) + tmpl.validate() + self.assertEqual(hashlib.sha256(six.text_type(t)).hexdigest(), + tmpl.t_digest, + 'invalid template digest') + def test_template_validate_cfn_good(self): t = { 'AWSTemplateFormatVersion': '2010-09-09', @@ -328,15 +350,35 @@ class TestTemplateValidate(common.HeatTestCase): def test_template_validate_cfn_empty(self): t = template_format.parse(''' -AWSTemplateFormatVersion: 2010-09-09 -Parameters: -Resources: -Outputs: -''') + AWSTemplateFormatVersion: 2010-09-09 + Parameters: + Resources: + Outputs: + ''') tmpl = template.Template(t) err = tmpl.validate() self.assertIsNone(err) + def test_template_validate_hot_check_t_digest(self): + t = { + 'heat_template_version': '2015-04-30', + 'description': 'foo', + 'parameters': {}, + 'resources': { + 'server': { + 'type': 'OS::Nova::Server' + } + }, + 'outputs': {}, + } + + tmpl = template.Template(t) + self.assertIsNone(tmpl.t_digest) + tmpl.validate() + self.assertEqual(hashlib.sha256(six.text_type(t)).hexdigest(), + tmpl.t_digest, + 'invalid template digest') + def test_template_validate_hot_good(self): t = { 'heat_template_version': '2013-05-23', @@ -507,16 +549,16 @@ class TemplateTest(common.HeatTestCase): def test_invalid_template(self): scanner_error = ''' -1 -Mappings: - ValidMapping: - TestKey: TestValue -''' + 1 + Mappings: + ValidMapping: + TestKey: TestValue + ''' parser_error = ''' -Mappings: - ValidMapping: - TestKey: {TestKey1: "Value1" TestKey2: "Value2"} -''' + Mappings: + ValidMapping: + TestKey: {TestKey1: "Value1" TestKey2: "Value2"} + ''' self.assertRaises(ValueError, template_format.parse, scanner_error) self.assertRaises(ValueError, template_format.parse, parser_error)